[Qemu-devel] [PATCH for-2.9] iscsi: Fix iscsi_create

[Qemu-devel] [PATCH for-2.9] iscsi: Fix iscsi_create

[Fam Zheng]

Since d5895fcb (iscsi: Split URL into individual options), creating
qcow2 image on an iscsi LUN fails:

    qemu-img create -f qcow2 iscsi://$SERVER/$IQN/0 1G
    qemu-img: iscsi://$SERVER/$IQN/0: Could not create image: Invalid
        argument

The problem is iscsi_open now expects that transport_name, portal and
target are already parsed into structured options by
iscsi_parse_filename, but it is not called in iscsi_create.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/iscsi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 716e74a..07596b5 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2092,6 +2092,7 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
     BlockDriverState *bs;
     IscsiLun *iscsilun = NULL;
     QDict *bs_options;
+    Error *local_err = NULL;
 
     bs = bdrv_new();
 
@@ -2103,7 +2104,13 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
 
     bs_options = qdict_new();
     qdict_put(bs_options, "filename", qstring_from_str(filename));
-    ret = iscsi_open(bs, bs_options, 0, NULL);
+    iscsi_parse_filename(filename, bs_options, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+    } else {
+        ret = iscsi_open(bs, bs_options, 0, NULL);
+    }
     QDECREF(bs_options);
 
     if (ret != 0) {
-- 
2.9.3

Re: [Qemu-devel] [PATCH for-2.9] iscsi: Fix iscsi_create

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On 04/10/2017 02:54 AM, Fam Zheng wrote:
> Since d5895fcb (iscsi: Split URL into individual options), creating
> qcow2 image on an iscsi LUN fails:
> 
>     qemu-img create -f qcow2 iscsi://$SERVER/$IQN/0 1G
>     qemu-img: iscsi://$SERVER/$IQN/0: Could not create image: Invalid
>         argument
> 
> The problem is iscsi_open now expects that transport_name, portal and
> target are already parsed into structured options by
> iscsi_parse_filename, but it is not called in iscsi_create.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/iscsi.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH for-2.9] iscsi: Fix iscsi_create

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]

On 10.04.2017 09:54, Fam Zheng wrote:
> Since d5895fcb (iscsi: Split URL into individual options), creating
> qcow2 image on an iscsi LUN fails:
> 
>     qemu-img create -f qcow2 iscsi://$SERVER/$IQN/0 1G
>     qemu-img: iscsi://$SERVER/$IQN/0: Could not create image: Invalid
>         argument
> 
> The problem is iscsi_open now expects that transport_name, portal and
> target are already parsed into structured options by
> iscsi_parse_filename, but it is not called in iscsi_create.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/iscsi.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 716e74a..07596b5 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -2092,6 +2092,7 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
>      BlockDriverState *bs;
>      IscsiLun *iscsilun = NULL;
>      QDict *bs_options;
> +    Error *local_err = NULL;
>  
>      bs = bdrv_new();
>  
> @@ -2103,7 +2104,13 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
>  
>      bs_options = qdict_new();
>      qdict_put(bs_options, "filename", qstring_from_str(filename));

Doesn't the below change make this qdict_put() superfluous?

Max

> -    ret = iscsi_open(bs, bs_options, 0, NULL);
> +    iscsi_parse_filename(filename, bs_options, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        ret = -EINVAL;
> +    } else {
> +        ret = iscsi_open(bs, bs_options, 0, NULL);
> +    }
>      QDECREF(bs_options);
>  
>      if (ret != 0) {
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [Qemu-devel] [PATCH for-2.9] iscsi: Fix iscsi_create

[Fam Zheng]

On Mon, 04/10 17:17, Max Reitz wrote:
> >      bs_options = qdict_new();
> >      qdict_put(bs_options, "filename", qstring_from_str(filename));
> 
> Doesn't the below change make this qdict_put() superfluous?

You are right, bdrv_iscsi.bdrv_needs_filename is false.

Fam

Re: [Qemu-devel] [PATCH for-2.9] iscsi: Fix iscsi_create

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

On 10.04.2017 09:54, Fam Zheng wrote:
> Since d5895fcb (iscsi: Split URL into individual options), creating
> qcow2 image on an iscsi LUN fails:
> 
>     qemu-img create -f qcow2 iscsi://$SERVER/$IQN/0 1G
>     qemu-img: iscsi://$SERVER/$IQN/0: Could not create image: Invalid
>         argument
> 
> The problem is iscsi_open now expects that transport_name, portal and
> target are already parsed into structured options by
> iscsi_parse_filename, but it is not called in iscsi_create.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/iscsi.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Thanks, dropped the qdict_put() for "filename" and applied to my block
branch:

https://github.com/XanClic/qemu/commits/block

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]