Patchwork Add an error_report when failing to open due to block-drv-whitelist

login
register
mail settings
Submitter Ryan Harper
Date June 9, 2010, 7:28 p.m.
Message ID <20100609192809.GX5280@us.ibm.com>
Download mbox | patch
Permalink /patch/55121/
State New
Headers show

Comments

Ryan Harper - June 9, 2010, 7:28 p.m.
When configure qemu with --block-drv-whitelist we don't report when we are
blocked by the white list and the resulting error message is misleading:

./configure --target-list=x86_64-softmmu \
            --block-drv-whitelist=qcow2,raw,host_device,host_cdrom

x86_64-softmmu/qemu-system-x86_64 -L pc-bios -m 512 -drive \
        file=fedora9_32_20G.qcow2,if=ide -monitor stdio
qemu: could not open disk image fedora9_32_20G.qcow2: Inappropriate ioctl for device

Which might lead one to look at the bdrv probe functions for floppy/cdrom
because we indeed will get an ioctl failure stored in errno and we report this
in vl.c when we get a non-zero return value from bdrv_open().

This patch adds an error report when we fail the whitelist and changes the errno
value to ENOPROTOOPT which was the closest thing I could think of that matched
the actual error.

Now we get the following output on whitelist failure:

x86_64-softmmu/qemu-system-x86_64 -L pc-bios -m 512 -drive \
        file=fedora9_32_20G.qcow2,if=ide -monitor stdio
qemu-system-x86_64: -drive file=fedora9_32_20G.qcow2,if=ide: block-drv-whitelist prevents using format 'file'
qemu: could not open disk image fedora9_32_20G.qcow2: Protocol not supported

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 block.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Anthony Liguori - June 9, 2010, 7:35 p.m.
On 06/09/2010 02:28 PM, Ryan Harper wrote:
> When configure qemu with --block-drv-whitelist we don't report when we are
> blocked by the white list and the resulting error message is misleading:
>
> ./configure --target-list=x86_64-softmmu \
>              --block-drv-whitelist=qcow2,raw,host_device,host_cdrom
>
> x86_64-softmmu/qemu-system-x86_64 -L pc-bios -m 512 -drive \
>          file=fedora9_32_20G.qcow2,if=ide -monitor stdio
> qemu: could not open disk image fedora9_32_20G.qcow2: Inappropriate ioctl for device
>
> Which might lead one to look at the bdrv probe functions for floppy/cdrom
> because we indeed will get an ioctl failure stored in errno and we report this
> in vl.c when we get a non-zero return value from bdrv_open().
>
> This patch adds an error report when we fail the whitelist and changes the errno
> value to ENOPROTOOPT which was the closest thing I could think of that matched
> the actual error.
>
> Now we get the following output on whitelist failure:
>
> x86_64-softmmu/qemu-system-x86_64 -L pc-bios -m 512 -drive \
>          file=fedora9_32_20G.qcow2,if=ide -monitor stdio
> qemu-system-x86_64: -drive file=fedora9_32_20G.qcow2,if=ide: block-drv-whitelist prevents using format 'file'
> qemu: could not open disk image fedora9_32_20G.qcow2: Protocol not supported
>    

Do you really have to say raw and file in the whitelist?  Is that what 
the source of the problem is?

Regards,

Anthony Liguori

> Signed-off-by: Ryan Harper<ryanh@us.ibm.com>
> ---
>   block.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/block.c b/block.c
> index 39724c1..ffcf7f2 100644
> --- a/block.c
> +++ b/block.c
> @@ -403,6 +403,9 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
>       pstrcpy(bs->filename, sizeof(bs->filename), filename);
>
>       if (use_bdrv_whitelist&&  !bdrv_is_whitelisted(drv)) {
> +        error_report("block-drv-whitelist prevents using format '%s'", drv->format_name);
> +        /* reset errno since we're failing because of whitelist restrictions */
> +        errno = EPROTONOSUPPORT;
>           return -ENOTSUP;
>       }
>
>
Ryan Harper - June 9, 2010, 7:37 p.m.
* Anthony Liguori <anthony@codemonkey.ws> [2010-06-09 14:36]:
> On 06/09/2010 02:28 PM, Ryan Harper wrote:
> >When configure qemu with --block-drv-whitelist we don't report when we are
> >blocked by the white list and the resulting error message is misleading:
> >
> >./configure --target-list=x86_64-softmmu \
> >             --block-drv-whitelist=qcow2,raw,host_device,host_cdrom
> >
> >x86_64-softmmu/qemu-system-x86_64 -L pc-bios -m 512 -drive \
> >         file=fedora9_32_20G.qcow2,if=ide -monitor stdio
> >qemu: could not open disk image fedora9_32_20G.qcow2: Inappropriate ioctl 
> >for device
> >
> >Which might lead one to look at the bdrv probe functions for floppy/cdrom
> >because we indeed will get an ioctl failure stored in errno and we report 
> >this
> >in vl.c when we get a non-zero return value from bdrv_open().
> >
> >This patch adds an error report when we fail the whitelist and changes the 
> >errno
> >value to ENOPROTOOPT which was the closest thing I could think of that 
> >matched
> >the actual error.
> >
> >Now we get the following output on whitelist failure:
> >
> >x86_64-softmmu/qemu-system-x86_64 -L pc-bios -m 512 -drive \
> >         file=fedora9_32_20G.qcow2,if=ide -monitor stdio
> >qemu-system-x86_64: -drive file=fedora9_32_20G.qcow2,if=ide: 
> >block-drv-whitelist prevents using format 'file'
> >qemu: could not open disk image fedora9_32_20G.qcow2: Protocol not 
> >supported
> >   
> 
> Do you really have to say raw and file in the whitelist?  Is that what 
> the source of the problem is?

I have to add 'file' to the whitelist string to get the above command
line to work.


> 
> Regards,
> 
> Anthony Liguori
> 
> >Signed-off-by: Ryan Harper<ryanh@us.ibm.com>
> >---
> >  block.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> >diff --git a/block.c b/block.c
> >index 39724c1..ffcf7f2 100644
> >--- a/block.c
> >+++ b/block.c
> >@@ -403,6 +403,9 @@ static int bdrv_open_common(BlockDriverState *bs, 
> >const char *filename,
> >      pstrcpy(bs->filename, sizeof(bs->filename), filename);
> >
> >      if (use_bdrv_whitelist&&  !bdrv_is_whitelisted(drv)) {
> >+        error_report("block-drv-whitelist prevents using format '%s'", 
> >drv->format_name);
> >+        /* reset errno since we're failing because of whitelist 
> >restrictions */
> >+        errno = EPROTONOSUPPORT;
> >          return -ENOTSUP;
> >      }
> >
> >
Kevin Wolf - June 10, 2010, 9:25 a.m.
Am 09.06.2010 21:28, schrieb Ryan Harper:
> When configure qemu with --block-drv-whitelist we don't report when we are
> blocked by the white list and the resulting error message is misleading:
> 
> ./configure --target-list=x86_64-softmmu \
>             --block-drv-whitelist=qcow2,raw,host_device,host_cdrom
> 
> x86_64-softmmu/qemu-system-x86_64 -L pc-bios -m 512 -drive \
>         file=fedora9_32_20G.qcow2,if=ide -monitor stdio
> qemu: could not open disk image fedora9_32_20G.qcow2: Inappropriate ioctl for device
> 
> Which might lead one to look at the bdrv probe functions for floppy/cdrom
> because we indeed will get an ioctl failure stored in errno and we report this
> in vl.c when we get a non-zero return value from bdrv_open().
> 
> This patch adds an error report when we fail the whitelist and changes the errno
> value to ENOPROTOOPT which was the closest thing I could think of that matched
> the actual error.
> 
> Now we get the following output on whitelist failure:
> 
> x86_64-softmmu/qemu-system-x86_64 -L pc-bios -m 512 -drive \
>         file=fedora9_32_20G.qcow2,if=ide -monitor stdio
> qemu-system-x86_64: -drive file=fedora9_32_20G.qcow2,if=ide: block-drv-whitelist prevents using format 'file'
> qemu: could not open disk image fedora9_32_20G.qcow2: Protocol not supported
> 
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> ---
>  block.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 39724c1..ffcf7f2 100644
> --- a/block.c
> +++ b/block.c
> @@ -403,6 +403,9 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
>      pstrcpy(bs->filename, sizeof(bs->filename), filename);
>  
>      if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
> +        error_report("block-drv-whitelist prevents using format '%s'", drv->format_name);
> +        /* reset errno since we're failing because of whitelist restrictions */
> +        errno = EPROTONOSUPPORT;

Any code that relies on this errno is broken. errno isn't part of the
bdrv_open interface. In fact, last week I have sent a patch to fix the
error message to use the return value instead, Anthony just needs to pull.

The error_report may be a good idea, though.

Kevin

Patch

diff --git a/block.c b/block.c
index 39724c1..ffcf7f2 100644
--- a/block.c
+++ b/block.c
@@ -403,6 +403,9 @@  static int bdrv_open_common(BlockDriverState *bs, const char *filename,
     pstrcpy(bs->filename, sizeof(bs->filename), filename);
 
     if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
+        error_report("block-drv-whitelist prevents using format '%s'", drv->format_name);
+        /* reset errno since we're failing because of whitelist restrictions */
+        errno = EPROTONOSUPPORT;
         return -ENOTSUP;
     }