[Qemu-devel] [PATCH] help: add id suboption to -iscsi

[Qemu-devel] [PATCH] help: add id suboption to -iscsi

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-options.hx |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 8b94264..7f12f02 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2089,7 +2089,7 @@ ETEXI
 DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
     "-iscsi [user=user][,password=password]\n"
     "       [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
-    "       [,initiator-name=iqn]\n"
+    "       [,initiator-name=initiator-iqn][,id=target-iqn]\n"
     "                iSCSI session parameters\n", QEMU_ARCH_ALL)
 STEXI
 
-- 
1.7.1

Re: [Qemu-devel] [Qemu-trivial] [PATCH] help: add id suboption to -iscsi

[Michael Tokarev]

06.12.2013 19:08, Paolo Bonzini wrote:
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2089,7 +2089,7 @@ ETEXI
>  DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
>      "-iscsi [user=user][,password=password]\n"
>      "       [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
> -    "       [,initiator-name=iqn]\n"
> +    "       [,initiator-name=initiator-iqn][,id=target-iqn]\n"
>      "                iSCSI session parameters\n", QEMU_ARCH_ALL)
>  STEXI

I'm not sure this is clearer, to me it looks more confusing
than before.

The thing is that "id" suboption is automatic and is handled
in the option parsing code explicitly.

As far as I can see, "id" suboption is actually never used by
iscsi code, only initiator-name is.

What this change is for?

Thanks,

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH] help: add id suboption to -iscsi

[Paolo Bonzini]

Il 06/12/2013 20:04, Michael Tokarev ha scritto:
> 06.12.2013 19:08, Paolo Bonzini wrote:
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2089,7 +2089,7 @@ ETEXI
>>  DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
>>      "-iscsi [user=user][,password=password]\n"
>>      "       [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
>> -    "       [,initiator-name=iqn]\n"
>> +    "       [,initiator-name=initiator-iqn][,id=target-iqn]\n"
>>      "                iSCSI session parameters\n", QEMU_ARCH_ALL)
>>  STEXI
> 
> I'm not sure this is clearer, to me it looks more confusing
> than before.
> 
> The thing is that "id" suboption is automatic and is handled
> in the option parsing code explicitly.

But it has a particular meaning in the case of -iscsi.  Usually it is
referred to in other options, or by monitor commands.  For -iscsi, it is
the target IQN.

Also, it is documented in qemu-doc.texi (albeit in -readconfig format):

Setting the target name allows different options for different targets
@example
[iscsi "iqn.target.name"]
  user = "CHAP username"
  password = "CHAP password"
  initiator-name = "iqn.qemu.test:my-initiator"
  # header digest is one of CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
  header-digest = "CRC32C"
@end example


> As far as I can see, "id" suboption is actually never used by
> iscsi code, only initiator-name is.

It's used:

    list = qemu_find_opts("iscsi");
    if (list) {
        opts = qemu_opts_find(list, target);
        if (!opts) {
            opts = QTAILQ_FIRST(&list->head);
        }
        if (opts) {
            name = qemu_opt_get(opts, "initiator-name");
            if (name) {
                return g_strdup(name);
            }
        }
    }

qemu_opts_find(list, target) returns the option set whose id is equal to
target.

> What this change is for?

Better documentation. :)

Paolo