On Wednesday 13 April 2016 15:18:20 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.
Maybe it would be worth as workaround for 2.6, although ... > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > > Note this problem was previously raised: > > http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg06501.html > > and discussed the following month: > > http://lists.nongnu.org/archive/html/qemu-devel/2015-12/msg00112.html > > block/iscsi.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 302baf8..7475cc9 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1070,6 +1070,22 @@ retry: > return 0; > } > > + > +static char *convert_target_to_id(const char *target) > +{ > + char *id = g_strdup(target); > + size_t i; > + > + for (i = 1; id[i]; i++) { > + if (!qemu_isalnum(id[i]) && !strchr("-._", id[i])) { > + id[i] = '_'; > + } > + } > + > + return id; > +} > + > + > static void parse_chap(struct iscsi_context *iscsi, const char *target, > Error **errp) > { > @@ -1079,13 +1095,16 @@ static void parse_chap(struct iscsi_context *iscsi, > const char *target, > const char *password = NULL; > const char *secretid; > char *secret = NULL; > + char *targetid = NULL; > > list = qemu_find_opts("iscsi"); > if (!list) { > return; > } > > - opts = qemu_opts_find(list, target); > + targetid = convert_target_to_id(target); > + opts = qemu_opts_find(list, targetid); > + g_free(targetid); > if (opts == NULL) { > opts = QTAILQ_FIRST(&list->head); > if (!opts) { ... the commit message seems to suggest that it applies to all the iSCSI parameter, but it actually works on the authentication-related ones. IMHO a better way would be using convert_target_to_id directly in iscsi_open on iscsi_url->target (right after the url parsing) passing the converted id to parse_initiator_name, iscsi_set_targetname, parse_chap, and parse_header_digest: this way it can apply to all the parameters. Thanks, -- Pino Toscano
signature.asc
Description: This is a digitally signed message part.
