[Qemu-devel] [PATCH] qemu_opt_get_bool_helper: Fix option handling

[Qemu-devel] [PATCH] qemu_opt_get_bool_helper: Fix option handling

[Michal Privoznik]

Well, after 49d2e648e8087 the options to -machine parameter no longer
has .desc nor .desc->type. That's mainly because the options are
dynamically added while .desc is allocated statically. Anyway, if user
tries to run:

   qemu-system-x86_64 -machine pc-i440fx-2.2,accel=kvm,usb=off

the arguments evaluation fails with:

   qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type ==
   QEMU_OPT_BOOL' failed.

Fix this by dropping the assert() which is useless after the mentioned
commit anyway.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 util/qemu-option.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..478420f 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -384,7 +384,6 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
         }
         return ret;
     }
-    assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL);
     ret = opt->value.boolean;
     if (del) {
         qemu_opt_del_all(opts, name);
-- 
2.0.5

Re: [Qemu-devel] [PATCH] qemu_opt_get_bool_helper: Fix option handling

[Eric Blake]

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

On 01/08/2015 09:09 AM, Michal Privoznik wrote:
> Well, after 49d2e648e8087 the options to -machine parameter no longer
> has .desc nor .desc->type. That's mainly because the options are
> dynamically added while .desc is allocated statically. Anyway, if user
> tries to run:
> 
>    qemu-system-x86_64 -machine pc-i440fx-2.2,accel=kvm,usb=off
> 
> the arguments evaluation fails with:
> 
>    qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type ==
>    QEMU_OPT_BOOL' failed.
> 
> Fix this by dropping the assert() which is useless after the mentioned
> commit anyway.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  util/qemu-option.c | 1 -
>  1 file changed, 1 deletion(-)

There are several other open threads about this same issue.  The
consensus so far appears to be that the assert is catching a real bug,
and should remain, and that we are instead working on the patches to fix
the real bug.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH] qemu_opt_get_bool_helper: Fix option handling

[Marcel Apfelbaum]

On 01/08/2015 06:09 PM, Michal Privoznik wrote:
> Well, after 49d2e648e8087 the options to -machine parameter no longer
> has .desc nor .desc->type. That's mainly because the options are
> dynamically added while .desc is allocated statically. Anyway, if user
> tries to run:
>
>     qemu-system-x86_64 -machine pc-i440fx-2.2,accel=kvm,usb=off
>
> the arguments evaluation fails with:
>
>     qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type ==
>     QEMU_OPT_BOOL' failed.
>
> Fix this by dropping the assert() which is useless after the mentioned
> commit anyway.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>   util/qemu-option.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index a708241..478420f 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -384,7 +384,6 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
>           }
>           return ret;
>       }
> -    assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL);
>       ret = opt->value.boolean;
>       if (del) {
>           qemu_opt_del_all(opts, name);
>

Hi Michal,
Thank you for the patch, but there is already a PULL request for a fix.

You can follow the details in this mail thread
https://www.mail-archive.com/qemu-devel@nongnu.org/msg272607.html

Thank you,
Marcel

Re: [Qemu-devel] [PATCH] qemu_opt_get_bool_helper: Fix option handling

[Paolo Bonzini]

On 08/01/2015 17:09, Michal Privoznik wrote:
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index a708241..478420f 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -384,7 +384,6 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
>          }
>          return ret;
>      }
> -    assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL);
>      ret = opt->value.boolean;
>      if (del) {
>          qemu_opt_del_all(opts, name);
> 

opt->value.boolean is not initialized correctly if opt->desc is NULL.
See how it is assigned in qemu_opt_parse.

Paolo