[Pkg-iscsi-maintainers] Bug#756843: open-iscsi: 7e1ae42 patch only mounting first target disk, then always failing in the end even on success

[Torben Frey]

Hi Ritesh,

oh yes, the fs variable is initialized before the loops are starting, together with the MOUNT_RESULT:

        # Now let's mount
        log_daemon_msg "Mounting network filesystems"
        MOUNT_RESULT=1
        fs=

And I guess functionality is good after removing the “break” command. I just don’t exactly know how the MOUNT_RESULT could summarize the result of all single mounts. Setting it to 0 after the first successful mount will just not catch if a following mount fails since the value would still be 0 (and I guess the exit code of the script should be an error code if one single mount is failing, right?). Maybe it could be done by touching a temporary file on the first failed mount and if we have that in the end setting MOUNT_RESULT to 1 and removing the temporary file again before calling log_end_msg? 

Else it would be necessary to get rid of the variable scope problem :)

Best regards,
Torben


On Aug 2, 2014, at 3:45 PM, Ritesh Raj Sarraf <rrs at researchut.com> wrote:

> Thank you for the bug report Torben.
> 
> Sigh!! I think that whole patch was buggy. I don't see the $fs variable ever having been initialized. Was it ?? My bad. :-(
> 
> @Turbo: Do you have an opinion here? I am inclined to reverting that patch completely. Let me know.
> 
> 
> On 08/02/2014 06:23 PM, Torben Frey wrote:
>> Package: open-iscsi
>> Version: 2.0.873+git0.3b4b4500-2
>> Severity: important
>> 
>> 
>> Dear Ritesh and Turbo,
>> 
>> this new patch is causing trouble for two reasons. Here are the relevant lines from patch 7e1ae42:
>> 
>> +	    while read fs; do
>> +		set -- $(eval echo "$fs" | sed 's@:@ @')
>> +		case "$1" in
>> +		    swap)
>> +			swapon $2
>> +			;;
>> +		    *)
>> +			fsck -a "$2"
>> +
>> +			if mount "$2" >/dev/null 2>&1; then
>> +			    MOUNT_RESULT=0                   <----- this does NOT change the value for the last line
>> +			    break                            <----- this is the break line I removed
>> +			fi
>> +			;;
>> +		esac
>> +	    done
>> 
>>        log_end_msg $MOUNT_RESULT                             <----- this will stay on 1 from inital setting
>> 
>> 1) The “break" is exiting the while loop after mounting the first found target disk successfully, ignoring all further disks which might still be in the loop value list. I fixed this behaviour for myself by just removing the break line. This should be the correct fix.
>> 2) "MOUNT_RESULT=0" is NOT changing the value to 0 because MOUNT_RESULT was initially set to 1 outside the nested loops/pipes. So the function/script will always call log_end_msg with the initial value of 1, displaying a “failed” after the init script finishes. I fixed this for myself by just explicitely setting the value of MOUNT_RESULT to 0 before the log_end_msg line. Of course, this is not representing the correct result of the mount calls - but currently it is not doing that as well. This is only a workaround.
>> 
>> Best regards,
>> Torben
>> 
>> 
>> 
>> -- System Information:
>> Debian Release: jessie/sid
>>   APT prefers testing
>>   APT policy: (500, 'testing')
>> Architecture: amd64 (x86_64)
>> 
>> Kernel: Linux 3.14-1-amd64 (SMP w/4 CPU cores)
>> Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
>> Shell: /bin/sh linked to /bin/dash
>> 
>> Versions of packages open-iscsi depends on:
>> ii  libc6  2.19-7
>> ii  udev   208-6
>> 
>> open-iscsi recommends no packages.
>> 
>> open-iscsi suggests no packages.
>> 
>> -- Configuration Files:
>> /etc/init.d/open-iscsi changed [not included]
>> /etc/iscsi/initiatorname.iscsi changed [not included]
>> /etc/iscsi/iscsid.conf changed [not included]
>> 
>> -- no debconf information
>> 
>> 
> 
> 
> -- 
> Ritesh Raj Sarraf
> RESEARCHUT - 
> http://www.researchut.com
> 
> "Necessity is the mother of invention."
>