[stgt] [BUG] [tgt-setup-lun] - adding inaccurate LUN to existing target will remove the target itself

[Max Benenson]

> > > Description of problem:
> > > =======================
> > > If you try to add a new LUN (and pass not existing device and the
> > > target name that already exists) the operation will fail since the
> > > target with the same name already exists and then the utility will
> > > suggest to the user to add this LUN to this existing target and if
> > > the user will select to proceed it will fail since the device does
> > > not exist and as the result will remove the target itself (a kind
> > > of rollback).
> > >
> > > Steps to reproduce:
> > > ===================
> > > 1. lvcreate -L 10G -n /dev/vg_autofiler01/max_lv01
> > > 2. tgt-setup-lun -n target1 -d /dev/vg_autofiler01/max_lv01 -t
> > > iscsi
> > > 3. tgt-setup-lun -n target1 -d /dev/vg_autofiler01/max_lv02 -t
> > > iscsi
> > >
> > > the last line will produce the following output:
> > > Using transport: iscsi
> > > Error: target named iqn.2001-04.com.auto-filer01-target1 already
> > > exists
> > > Add a new lun to the existing target? (yes/NO): yes
> > > Adding a logical unit (/dev/vg_autofiler01/max_lv02) to target,
> > > tid=1
> > > tgtadm: invalid request
> > > Error: failed to add a logical unit (/dev/vg_autofiler01/max_lv02)
> > > to target, tid=1
> > > Deleting new target, tid=1
> > >
> > > Actual result:
> > > ==============
> > > The target with id 1 was removed from the targets.conf file.
> > >
> > > Expected result:
> > > ================
> > > The rollback action should consider the presence of existing
> > > target and should NOT remove it.
> > >
> > > Environment details:
> > > ====================
> > > [root at auto-filer01 tgt]# cat /etc/redhat-release
> > > Red Hat Enterprise Linux Server release 6.0 (Santiago)
> > > [root at auto-filer01 tgt]# rpm -qa | grep kernel
> > > dracut-kernel-004-33.2.el6_0.noarch
> > > abrt-addon-kerneloops-1.1.13-4.el6.x86_64
> > > kernel-firmware-2.6.32-71.14.1.el6.noarch
> > > kernel-2.6.32-71.14.1.el6.x86_64
> > > kernel-2.6.32-71.el6.x86_64
> > > [root at auto-filer01 tgt]# rpm -qa | grep lvm
> > > lvm2-2.02.72-8.el6_0.4.x86_64
> > > lvm2-libs-2.02.72-8.el6_0.4.x86_64
> > > [root at auto-filer01 tgt]# less /var/log/messages
> > > Jan 31 17:43:09 auto-filer01 tgtd: backed_file_open(92) Could not
> > > open /dev/vg_autofiler01/max_lv02, No such file or directory
> > >
> > > Resolution:
> > > ===========
> > > IMHO the fix should be in err_exit() method which should verify
> > > (using additional function)
> > > LUN presence in the target before performing "tgtadm --lld
> > > $lld_name --op delete --mode target --tid $tid".
> > > Something like (I am using Python syntax):
> > > if not hasAttachedLUN($tid):
> > >     tgtadm --lld $lld_name --op delete --mode target --tid $tid
> >
> > Your proposal looks ok to me but in the first place, why does the
> > script checks if the target already exists before adding the lu?
> >
> > This is a logic in this script that was written by
> > erezz at voltaire.com
> > I am agree that the script should add the device to the existing
> > target
> > and create the new one in case the target doesn't exist.
> 
> I think that the change is ok since sounds it doesn't break the
> existing configuration.
> 
> Can you send a patch?
> 
> 
> Thanks,

I don't have a patch currently (I am Red Hat employee and I have a very tight schedule right now, sorry).
I think there are more than one fix:
1. Add new function that validates if a target is empty
2. Fix err_exit() function to use the function from 1 above before remove any target
3. Remove the validation if target already exists during 'add new LUN' sequence

I am going to report this issue in Bugzilla under scsi-target-utils component.
In case there is no specific maintainer of tgt-setup-lun utility and no one will
provide a patch for a while I will try to prepare a patch by myself.


Regards,
Max
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html