[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 for-2.6] block: convert iscsi target to a val
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v2 for-2.6] block: convert iscsi target to a valid ID for -iscsi arg lookup |
Date: |
Thu, 14 Apr 2016 09:25:42 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, Apr 13, 2016 at 09:41:54PM -0400, John Ferlan wrote:
>
>
> On 04/13/2016 12:17 PM, Daniel P. Berrange wrote:
> > The iSCSI block driver has a very strange approach whereby it
> > does not accept options directly as part of the -drive arg,
> > but instead takes them indirectly from a -iscsi arg. To make
> > up -driver and -iscsi args, it takes the iSCSI target name
> > and uses that as an ID value for the -iscsi arg lookup.
> >
> > For example, given a -drive arg that looks like
> >
> > -drive
> > file=iscsi://192.168.122.1:3260/iqn.2013-12.com.example%3Aiscsi-chap-netpool/1,format=raw,if=none,id=drive-virtio-disk0
> >
> > The iSCSI driver will try to find the -iscsi arg with an
> > id of "iqn.2013-12.com.example:iscsi-chap-netpool" eg it
> > expects
> >
> > -iscsi
> > id=iqn.2013-12.com.example:iscsi-chap-netpool,user=fred,password-secret=secret0
> >
> > Unfortunately this is impossible to actually do in practice
> > because almost all iSCSI target names contain a ':' which
> > is not a valid ID character for QEMU:
> >
> > $ qemu-system-x86_64: -iscsi
> > id=iqn.2013-12.com.example:iscsi-chap-netpool,user=redhat,password=123456:
> > Parameter 'id' expects an identifier
> > Identifiers consist of letters, digits, '-', '.', '_', starting with a
> > letter.
> >
> > So for this to work we need to remove the invalid characters
> > in some manner. This patch takes the simplest possible approach
> > of just converting all invalid characters into underscores. eg
> > you can now use
> >
> > $ qemu-system-x86_64: -iscsi
> > id=iqn.2013-12.com.example_iscsi-chap-netpool,user=redhat,password=123456:
> > Parameter 'id' expects an identifier
> >
> > There is theoretically the possibility for collison with this
> > approach if there were 2 iSCSI luns attached to the same VM
> > with target names that clash on the escaped char: eg
> >
> > iqn.2013-12.com.example:iscsi-chap-netpool
> > iqn.2013-12.com.example_iscsi-chap-netpool
> >
> > but in reality this will never happen. In addition in QEMU 2.7
> > the need to use the -iscsi arg will be removed by allowing all
> > the desired options to be listed directly with -drive. IOW this
> > quick stripping of invalid characters is just a short term fix
> > that will ultimately go away. For this reason it is not thought
> > worthwhile to invent a full collision-free escaping syntax for
> > iSCSI target IDs.
> >
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> >
>
> Figured I'd chime in since I tripped across this today...
>
> I think the one thing that concerns me about this '_' approach is we'd
> be "stuck" with it. Eventually if 'initiator-name' is added to the
> -drive command (as well as being able to parse the 'user=' and
> 'password-secret='), then needing to add -iscsi wouldn't be required for
> libvirt. Whether this patch would be required after -drive is modified
> was the other question rattling around. So I figured I'd see if I can
> get things to work without it...
>
> Using the v1 of this patch series did work for libvirt if I passed the
> "id=" shown above using the '_' instead of ':'; however, taking the Pino
> Toscano's series in mind, I can also start a domain using the
> "initiator-name=" and "id=" parameters for '-iscsi' as follows:
>
> ...
> -object
> secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-3-jaf/master-key.aes
>
> ...
> -iscsi
> id=iscsi-chap-netpool,initiator-name=iqn.2013-12.com.example,user=redhat,password-secret=virtio-disk1-ivKey0
> -drive
> file=iscsi://192.168.122.1:3260/iqn.2013-12.com.example%3Aiscsi-chap-netpool/1,format=raw,if=none,id=drive-virtio-disk1
> -device
> virtio-blk-pci,scsi=off,bus=pci.0,addr=0x9,drive=drive-virtio-disk1,id=virtio-disk1,bootindex=1
>
I don't believe that is doing what you think it is doing.
QEMU will still be using the "iqn.2013-12.com.example:iscsi-chap-netpool"
string as an ID to lookup the corresponding -iscsi arg. It will not
find it because your arg uses ID of "iscsi-chap-netpool". So, the code
will now be falling backk to just using the /first/ -iscsi arg in the
list.
IOW, if you add multiple iSCSI disks to the VM, they will all be using
the first -iscsi arg, which is certainly not what we want
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|