block: Bugfix 'format' and 'snapshot' used in drive option

When use -drive file='xxx',format=qcow2,snapshot=on the error
message "Can't use snapshot=on with driver-specific options"
can be show, and fail to start the qemu.

This should not be happened, and there is no file.driver option
in qemu command line.

It is because the commit 74fe54f2a1b5c4f4498a8fe521e1dfc936656cd4,
it puts 'driver' option if the command line use 'format' option.

This patch is to solve this bug.

Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
---
 blockdev.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Hi all

This 'bug' also happens use -snapshot,

But as newer to qemu,
I'm not sure about what am I understanding.

But I think snapshot should works at least in qcow2.

My understanding about the driver option is mainly for
'nbd' and 'http/https/ftp', and so in commit:
c2ad1b0c465a9ea8375eaff14bbd85705c673f73
So when 'driver' option used, the snapshot can't
be using.

As this understanding, my first patch is try to
get the the value of key==driver, and if the
value in 'nbd' and 'http/https/ftp', then
give out that error messages.

So give me some comments and sugguestions, pls.

Thanks
Mike

2013/8/8 22:45, Mike Qiu wrote:
> When use -drive file='xxx',format=qcow2,snapshot=on the error
> message "Can't use snapshot=on with driver-specific options"
> can be show, and fail to start the qemu.
>
> This should not be happened, and there is no file.driver option
> in qemu command line.
>
> It is because the commit 74fe54f2a1b5c4f4498a8fe521e1dfc936656cd4,
> it puts 'driver' option if the command line use 'format' option.
>
> This patch is to solve this bug.
>
> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
> ---
>  blockdev.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 41b0a49..e174b7d 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -340,6 +340,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
>      QDict *bs_opts;
>      const char *id;
>      bool has_driver_specific_opts;
> +    BlockDriver *drv = NULL;
>
>      translation = BIOS_ATA_TRANSLATION_AUTO;
>      media = MEDIA_DISK;
> @@ -485,7 +486,11 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
>              return NULL;
>          }
>
> -        qdict_put(bs_opts, "driver", qstring_from_str(buf));
> +        drv = bdrv_find_whitelisted_format(buf, ro);
> +        if (!drv) {
> +            error_report("'%s' invalid format", buf);
> +            return NULL;
> +        }
>      }
>
>      /* disk I/O throttling */
> @@ -700,12 +705,13 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
>      }
>
>      QINCREF(bs_opts);
> -    ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, NULL);
> +    ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv);
>
>      if (ret < 0) {
>          if (ret == -EMEDIUMTYPE) {
>              error_report("could not open disk image %s: not in %s format",
> -                         file ?: dinfo->id, qdict_get_str(bs_opts, "driver"));
> +                         file ?: dinfo->id, drv ? drv->format_name :
> +                         qdict_get_str(bs_opts, "driver"));
>          } else {
>              error_report("could not open disk image %s: %s",
>                           file ?: dinfo->id, strerror(-ret));

Am 08.08.2013 um 16:45 hat Mike Qiu geschrieben:
> When use -drive file='xxx',format=qcow2,snapshot=on the error
> message "Can't use snapshot=on with driver-specific options"
> can be show, and fail to start the qemu.
> 
> This should not be happened, and there is no file.driver option
> in qemu command line.
> 
> It is because the commit 74fe54f2a1b5c4f4498a8fe521e1dfc936656cd4,
> it puts 'driver' option if the command line use 'format' option.
> 
> This patch is to solve this bug.
> 
> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>

Thanks, applied to the block branch.

This is quite obviously not the proper long-term solution, but I've had
a look at the code and considering that we'll only have one more release
candidate for 1.6, it seems to be too risky to do anything more involved
than this.

I'm going to revert this patch again for 1.7 and replace it by proper
snapshot=on support with driver-specific options.

Kevin