[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu
From: |
Shannon Zhao |
Subject: |
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper() |
Date: |
Tue, 6 Jan 2015 14:20:36 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
On 2015/1/6 10:37, Chen, Tiejun wrote:
> On 2015/1/5 20:14, Marcel Apfelbaum wrote:
>> On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
>>> On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka <address@hidden>
>>> wrote:
>>>> On 2015-01-05 12:22, Stefan Hajnoczi wrote:
>>>>> Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 ("machine: remove
>>>>> qemu_machine_opts global list") removed option descriptions from the
>>>>> -machine QemuOptsList to avoid repeating MachineState's QOM properties.
>>>>>
>>>>> This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
>>>>> be used on QemuOptsList without option descriptions since QemuOpts
>>>>> doesn't know the type and therefore left an unparsed string value.
>>>>>
>>>>> This patch avoids calling qemu_opt_get_bool() to fix the assertion
>>>>> failure:
>>>>>
>>>>> $ qemu-system-x86_64 -usb
>>>>> qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type
>>>>> == QEMU_OPT_BOOL' failed.
>>>>>
>>>>> Test the presence of -usb using qemu_opt_find() but use the
>>>>> MachineState->usb field instead of qemu_opt_get_bool().
>>>>>
>>>>> Cc: Marcel Apfelbaum <address@hidden>
>>>>> Cc: Tiejun Chen <address@hidden>
>>>>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>>>>> ---
>>>>> vl.c | 7 +++++--
>>>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/vl.c b/vl.c
>>>>> index bea9656..6e8889c 100644
>>>>> --- a/vl.c
>>>>> +++ b/vl.c
>>>>> @@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
>>>>> *opaque)
>>>>>
>>>>> bool usb_enabled(bool default_usb)
>>>>> {
>>>>> - return qemu_opt_get_bool(qemu_get_machine_opts(), "usb",
>>>>> - has_defaults && default_usb);
>>>>> + if (qemu_opt_find(qemu_get_machine_opts(), "usb")) {
>>>>> + return current_machine->usb;
>>>>> + } else {
>>>>> + return has_defaults && default_usb;
>>>>> + }
>>>>> }
>>>>>
>>>>> #ifndef _WIN32
>>>>>
>>>>
>>>> That still leaves the other boolean machine options broken. A generic
>>>> fix would be good. Or revert the original commit until we have one.
>>>
>>> I think we should revert the original commit.
>>>
>>> qemu_option_get_*() callers need to be converted to query MachineState
>>> fields instead of using QemuOpts functions.
>
> Can this work out currently?
>
Hi Tiejun,
I apply your following patch and it works. At least it doesn't crash.
Thanks,
Shannon
> ---
> util/qemu-option.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index a708241..933b885 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
> }
>
> opt = qemu_opt_find(opts, name);
> - if (!opt) {
> + if (!opt || !opt->desc) {
> const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
> if (desc && desc->def_value_str) {
> return desc->def_value_str;
> @@ -341,7 +341,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
> }
>
> opt = qemu_opt_find(opts, name);
> - if (!opt) {
> + if (!opt || !opt->desc) {
> desc = find_desc_by_name(opts->list->desc, name);
> if (desc && desc->def_value_str) {
> str = g_strdup(desc->def_value_str);
> @@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts,
> const char *name,
> }
>
> opt = qemu_opt_find(opts, name);
> - if (opt == NULL) {
> + if (!opt || !opt->desc) {
> const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
> if (desc && desc->def_value_str) {
> parse_option_bool(name, desc->def_value_str, &ret, &error_abort);
> @@ -413,7 +413,7 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts
> *opts, const char *name,
> }
>
> opt = qemu_opt_find(opts, name);
> - if (opt == NULL) {
> + if (!opt || !opt->desc) {
> const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
> if (desc && desc->def_value_str) {
> parse_option_number(name, desc->def_value_str, &ret,
> &error_abort);
> @@ -450,7 +450,7 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts *opts,
> const char *name,
> }
>
> opt = qemu_opt_find(opts, name);
> - if (opt == NULL) {
> + if (!opt || !opt->desc) {
> const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
> if (desc && desc->def_value_str) {
> parse_option_size(name, desc->def_value_str, &ret, &error_abort);
- [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Stefan Hajnoczi, 2015/01/05
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Jan Kiszka, 2015/01/05
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Stefan Hajnoczi, 2015/01/05
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Marcel Apfelbaum, 2015/01/05
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Laszlo Ersek, 2015/01/05
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Chen, Tiejun, 2015/01/05
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(),
Shannon Zhao <=
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Chen, Tiejun, 2015/01/06
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Marcel Apfelbaum, 2015/01/06
- Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Stefan Hajnoczi, 2015/01/06
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper(), Jan Kiszka, 2015/01/31