Date: Mon, 8 Oct 2007 14:07:20 GMT From: Javier Martín Rueda <jmrueda@diatel.upm.es> To: freebsd-gnats-submit@FreeBSD.org Subject: ports/117015: iscsi-target fails when exporting a disk device Message-ID: <200710081407.l98E7KpO079585@www.freebsd.org> Resent-Message-ID: <200710081410.l98EA0p7095982@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 117015
>Category: ports
>Synopsis: iscsi-target fails when exporting a disk device
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-ports-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Mon Oct 08 14:10:00 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator: Javier Martín Rueda
>Release: FreeBSD 7.0-CURRENT-200708
>Organization:
Universidad Politécnica de Madrid
>Environment:
FreeBSD 7.0-CURRENT-200708 FreeBSD 7.0-CURRENT-200708 #0: Fri Aug 17 07:47:18 UTC 2007 root@logan.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC i386
>Description:
This report tackles two bugs at the same time, but I report them at the same time because the fix is common (actually, I found bug number two when trying to correct bug number one).
Number one:
If you define an extent based on a disk device, such as /dev/da0, /dev/da0s1 or similar, the iscsi-target daemon complains that it cannot allocate space for the target and then aborts.
Number two:
If you define and extent based on an existing disk file of the exact size, the initialization code will append one byte of rubbish at the end of it, although everything will work fine appart from that.
See the detailed examples in "how to repeat the problem".
>How-To-Repeat:
Number one:
Create the following config file in /usr/local/etc/iscsi/targets:
extent0 /dev/da0 0 100MB
target0 rw extent0 0.0.0.0/0
Then start the daemon and you will get the error message specified in the full description of the problem report:
# /usr/local/etc/rc.d/iscsi_target start
Starting iscsi_target.
Reading configuration from `/usr/local/etc/iscsi/targets'
target0:rw:0.0.0.0/0
extent0:/dev/da0:0:104857600
DISK: 1 logical unit (204800 blocks, 512 bytes/block), type iscsi fs
DISK: LUN 0: pid 4623:disk.c:770: ***ERROR*** error reading "target0"pid 4623:disk.c:911: ***ERROR*** error allocating space for "target0"pid 4623:target.c:1496: ***ERROR*** device_init() failed
pid 4623:iscsi-target.c:145: ***ERROR*** target_init() failed
Number two:
Create the following config file in /usr/local/etc/iscsi/targets:
extent0 /my_example 0 100MB
target0 rw extent0 0.0.0.0/0
Now execute the following:
# dd if=/dev/zero of=/my_example bs=1m count=100
# ls -l /my_example
-rw-r--r-- 1 root wheel 104857600 Oct 8 15:43 /my_example
# /usr/local/etc/rc.d/iscsi_target start
Starting iscsi_target.
Reading configuration from `/usr/local/etc/iscsi/targets'
target0:rw:0.0.0.0/0
extent0:/my_example:0:104857600
DISK: 1 logical unit (204800 blocks, 512 bytes/block), type iscsi fs
DISK: LUN 0: 100 MB disk storage for "target0"
TARGET: TargetName is iqn.1994-04.org.netbsd.iscsi-target
# ls -l /my_example
-rw-r--r-- 1 root wheel 104857601 Oct 8 15:44 /my_example
As you can see, now the file is one byte longer than before.
>Fix:
The initialization code attempts to see if the extent file exists by lseeking to its size minus 1, and then reads and writes 1 byte. The write is done to create the file if it did not exist previously. That provokes two bugs:
Number one:
If the extent file is a disk device, you cannot just read 1 byte, because the kernel will reply with EINVAL. You have to read a multiple of the sector size, and I think it has to be aligned with the sector size too. My proposed fix is to lseek to the extent size minus 512 and then read and write 512 bytes. This accomplish the same effect as the original code, but does not fail when the extent file is a disk device. Note: instead of 512, maybe I should use some predefined constant. Maybe S_BLKSIZE in /usr/include/sys/stat.h?
Number two:
If the extent file exists and its size is at least as big as what you specified in the iscsi_target config file, the read operation will advance the file pointer to the end, and the subsequent write will append an extra superfluos byte at the end, or overwrite one extra byte out of range if the file was larger. Obviously, the code was written thinking of the case in which the file does not exist or is smaller than the exported size, because in such case the read does not advance the file pointer, and everything works fine.
My proposed fix basically is to write only if the read did not get anything.
I enclose a patch file for the port. Just drop it in /usr/ports/net/iscsi-target/files, recompile and reinstall.
Patch attached with submission follows:
--- disk.c.orig 2007-09-06 22:11:15.000000000 +0200
+++ disk.c 2007-10-07 09:30:39.000000000 +0200
@@ -759,18 +759,19 @@
de_allocate(disc_de_t *de, char *filename)
{
off_t size;
- char ch;
+ char ch[512];
+ ssize_t nb;
size = de_getsize(de);
- if (de_lseek(de, size - 1, SEEK_SET) == -1) {
+ if (de_lseek(de, size - 512, SEEK_SET) == -1) {
iscsi_trace_error(__FILE__, __LINE__, "error seeking \"%s\"\n", filename);
return 0;
}
- if (de_read(de, &ch, 1) == -1) {
+ if ((nb = de_read(de, &ch, 512)) == -1) {
iscsi_trace_error(__FILE__, __LINE__, "error reading \"%s\"", filename);
return 0;
}
- if (de_write(de, &ch, 1) == -1) {
+ if (de_write(de, &ch, 512 - nb) == -1) {
iscsi_trace_error(__FILE__, __LINE__, "error writing \"%s\"", filename);
return 0;
}
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200710081407.l98E7KpO079585>