[libvirt] [PATCH] qemu: Provide default LUN=0 for iSCSI if not provided

[Peter Krempa]

On Wed, Sep 06, 2017 at 12:05:32 -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1477880
> 
> If the "/#" is missing from the provided iSCSI path, then we need
> to provide the default LUN of /0; otherwise, QEMU will fail to parse
> the URL causing a failure to either create the guest or hotplug
> attach the storage.
> 
> Alter the command lines generated appropriately. This change does not
> effect the XML input files.
> 
> One 'side effect' of this for the argv2xml is that if a "/0" is found
> on the command line, then the generated XML would need to add the "/0"
> to the output XML since it wouldn't be known whether it existed or not
> at qemu process creation time.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  NB: Investigated the history quite a bit between QEMU and libiscsi and
>  it seems as though this probably never worked and of course never "tested"
>  in a real environment. Ironically, in qapi/block-core.json, it is claimed
>  that the LUN would default to 0, but the block/iscsi.c code that calls
>  iscsi_parse_full_url would never (AFAICT) have supported not supplying
>  some LUN on the command line.
> 
>  src/qemu/qemu_command.c                              | 20 ++++++++++++++++----
>  .../qemuargv2xml-disk-drive-network-iscsi.args       |  2 +-
>  .../qemuargv2xml-disk-drive-network-iscsi.xml        |  2 +-
>  .../qemuxml2argv-disk-drive-network-iscsi-lun.args   |  2 +-
>  .../qemuxml2argv-disk-drive-network-iscsi.args       |  2 +-
>  .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args         |  2 +-
>  .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args      |  2 +-
>  7 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ed8cb6e..cff84de 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -830,10 +830,22 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
>                              src->volume, src->path) < 0)
>                  goto cleanup;
>          } else {
> -            if (virAsprintf(&uri->path, "%s%s",
> -                            src->path[0] == '/' ? "" : "/",
> -                            src->path) < 0)
> -                goto cleanup;
> +            if (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
> +                !strchr(src->path, '/')) {
> +
> +                /* The details of iqn is defined by RFC 3720 and 3721, but
> +                 * we just need to make sure there's a lun provided. If not
> +                 * provided, then default to zero. If ISCSI LUN is provided
> +                 * by /dev/disk/by-path/... , then that path will have the
> +                 * specific lun requested. */
> +                if (virAsprintf(&uri->path, "/%s/0", src->path) < 0)
> +                    goto cleanup;
> +            } else {
> +                if (virAsprintf(&uri->path, "%s%s",
> +                                src->path[0] == '/' ? "" : "/",
> +                                src->path) < 0)
> +                    goto cleanup;
> +            }
>          }
>      }

The command line generator is not really the correct place to add
defaults. We have the post parse callbacks for this, and the LUN clearly
should be marked in the XML
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170911/94874fa8/attachment-0001.sig>