Re: [PATCH v2 3/6] qemu-option: restrict qemu_opts_set to merge-lists QemuOpts

[Paolo Bonzini <pbonzini@redhat.com>]

On 09/11/20 19:52, Markus Armbruster wrote:
> Do you think working (some of) this into commit message would be worth
> your while?

Easy and does not require a respin, so yes.

>> Improve the testcase, though I should have mentioned it in the commit
>> message.  Basically emulating "-kernel bc -kernel def".
> Worth testing.  But the case "-kernel bc" is also worth testing.
> test_qemu_opts_get() tests both:
> 
>      /* haven't set anything to str2 yet */
>      opt = qemu_opt_get(opts, "str2");
>      g_assert(opt == NULL);
> 
>      qemu_opt_set(opts, "str2", "value", &error_abort);
> 
>      /* now we have set str2, should know about it */
>      opt = qemu_opt_get(opts, "str2");
>      g_assert_cmpstr(opt, ==, "value");
> 
>      qemu_opt_set(opts, "str2", "value2", &error_abort);
> 
>      /* having reset the value, the returned should be the reset one */
>      opt = qemu_opt_get(opts, "str2");
>      g_assert_cmpstr(opt, ==, "value2");

Note opt_set vs. opts_set though.

> I'm okay with not improving the test in this patch, or with strictly
> extending coverage, preferably in a separate patch that goes before this
> one.
> 

Ok, I'll just drop the testcase change for now.

Paolo