|
From: | Paolo Bonzini |
Subject: | Re: [PATCH v2 3/6] qemu-option: restrict qemu_opts_set to merge-lists QemuOpts |
Date: | Mon, 9 Nov 2020 19:58:10 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |