* [PATCH 0/3] Minor cleanups: eliminate redundant code in QOM, block-backend and vl.c
@ 2026-04-29 6:20 Bin Guo
2026-04-29 6:20 ` [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child Bin Guo
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Bin Guo @ 2026-04-29 6:20 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, kwolf, hreitz, qemu-block
This series contains three independent cleanups that remove redundant
code and improve consistency across different subsystems.
Patch 1 merges a double hash-table traversal in object_property_del_child()
into a single pass, halving the number of hash table operations in the
common case.
Patch 2 makes blk_co_preadv() a one-liner wrapper around
blk_co_preadv_part(), matching the existing pattern already used by
the write side (blk_co_pwritev / blk_co_pwritev_part).
Patch 3 inlines qemu_opts_parse_noisily() return-value checks directly
into if-conditions for options that do not use the returned pointer,
matching the style already used by QEMU_OPTION_action.
All three patches are independent and can be applied in any order.
Bin Guo (3):
qom/object: merge double hash table traversal in
object_property_del_child
block/block-backend: delegate blk_co_preadv to blk_co_preadv_part
system/vl.c: inline qemu_opts_parse_noisily() result checks
block/block-backend.c | 8 +-------
qom/object.c | 7 -------
system/vl.c | 37 +++++++++++++++----------------------
3 files changed, 16 insertions(+), 36 deletions(-)
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child 2026-04-29 6:20 [PATCH 0/3] Minor cleanups: eliminate redundant code in QOM, block-backend and vl.c Bin Guo @ 2026-04-29 6:20 ` Bin Guo 2026-04-29 21:07 ` Richard Henderson 2026-04-29 6:20 ` [PATCH 2/3] block/block-backend: delegate blk_co_preadv to blk_co_preadv_part Bin Guo 2026-04-29 6:20 ` [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks Bin Guo 2 siblings, 1 reply; 12+ messages in thread From: Bin Guo @ 2026-04-29 6:20 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, berrange, kwolf, hreitz, qemu-block object_property_del_child() previously performed two full iterations over obj->properties: the first to find the matching child property and call its release callback, the second to find the same property again and remove it from the hash table. Merge the two loops into one: when the matching property is found, call the release callback and immediately remove the entry via g_hash_table_iter_remove(), then break. This halves the number of hash table operations in the common case and avoids the redundant second scan. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> --- qom/object.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/qom/object.c b/qom/object.c index f981e27044..9c0e8dfd02 100644 --- a/qom/object.c +++ b/qom/object.c @@ -629,13 +629,6 @@ static void object_property_del_child(Object *obj, Object *child) prop->release(obj, prop->name, prop->opaque); prop->release = NULL; } - break; - } - } - g_hash_table_iter_init(&iter, obj->properties); - while (g_hash_table_iter_next(&iter, &key, &value)) { - prop = value; - if (object_property_is_child(prop) && prop->opaque == child) { g_hash_table_iter_remove(&iter); break; } -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child 2026-04-29 6:20 ` [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child Bin Guo @ 2026-04-29 21:07 ` Richard Henderson 2026-04-30 9:24 ` Daniel P. Berrangé 0 siblings, 1 reply; 12+ messages in thread From: Richard Henderson @ 2026-04-29 21:07 UTC (permalink / raw) To: qemu-devel On 4/29/26 16:20, Bin Guo wrote: > object_property_del_child() previously performed two full iterations > over obj->properties: the first to find the matching child property and > call its release callback, the second to find the same property again > and remove it from the hash table. > > Merge the two loops into one: when the matching property is found, call > the release callback and immediately remove the entry via > g_hash_table_iter_remove(), then break. This halves the number of hash > table operations in the common case and avoids the redundant second scan. > > Signed-off-by: Bin Guo <guobin@linux.alibaba.com> > --- > qom/object.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index f981e27044..9c0e8dfd02 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -629,13 +629,6 @@ static void object_property_del_child(Object *obj, Object *child) > prop->release(obj, prop->name, prop->opaque); > prop->release = NULL; > } > - break; > - } > - } > - g_hash_table_iter_init(&iter, obj->properties); > - while (g_hash_table_iter_next(&iter, &key, &value)) { > - prop = value; > - if (object_property_is_child(prop) && prop->opaque == child) { > g_hash_table_iter_remove(&iter); > break; > } Fixes: b604a854e84 ("qom: Replace object property list with GHashTable") Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child 2026-04-29 21:07 ` Richard Henderson @ 2026-04-30 9:24 ` Daniel P. Berrangé 2026-04-30 13:36 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 12+ messages in thread From: Daniel P. Berrangé @ 2026-04-30 9:24 UTC (permalink / raw) To: Richard Henderson; +Cc: qemu-devel On Thu, Apr 30, 2026 at 07:07:03AM +1000, Richard Henderson wrote: > On 4/29/26 16:20, Bin Guo wrote: > > object_property_del_child() previously performed two full iterations > > over obj->properties: the first to find the matching child property and > > call its release callback, the second to find the same property again > > and remove it from the hash table. > > > > Merge the two loops into one: when the matching property is found, call > > the release callback and immediately remove the entry via > > g_hash_table_iter_remove(), then break. This halves the number of hash > > table operations in the common case and avoids the redundant second scan. > > > > Signed-off-by: Bin Guo <guobin@linux.alibaba.com> > > --- > > qom/object.c | 7 ------- > > 1 file changed, 7 deletions(-) > > > > diff --git a/qom/object.c b/qom/object.c > > index f981e27044..9c0e8dfd02 100644 > > --- a/qom/object.c > > +++ b/qom/object.c > > @@ -629,13 +629,6 @@ static void object_property_del_child(Object *obj, Object *child) > > prop->release(obj, prop->name, prop->opaque); > > prop->release = NULL; > > } > > - break; > > - } > > - } > > - g_hash_table_iter_init(&iter, obj->properties); > > - while (g_hash_table_iter_next(&iter, &key, &value)) { > > - prop = value; > > - if (object_property_is_child(prop) && prop->opaque == child) { > > g_hash_table_iter_remove(&iter); > > break; > > } > > Fixes: b604a854e84 ("qom: Replace object property list with GHashTable") On the contrary, this proposed patch breaks that commit. The original proposal for that old commit had a single iteration, but it was discovered that this broke cleanup for certain devices, as prop->release can have side-effects that trigger assertions in g_hash_table_iter_remove. This requires a two phase iteration to be safe. A test for that problem was added in 8c4d156c187c84b574d287bd4b9ddf9a6975de7c and this new patch proposal duly breaks the check-qom-proplist test. With regards, Daniel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child 2026-04-30 9:24 ` Daniel P. Berrangé @ 2026-04-30 13:36 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2026-04-30 13:36 UTC (permalink / raw) To: Daniel P. Berrangé, Richard Henderson; +Cc: qemu-devel On 30/4/26 11:24, Daniel P. Berrangé wrote: > On Thu, Apr 30, 2026 at 07:07:03AM +1000, Richard Henderson wrote: >> On 4/29/26 16:20, Bin Guo wrote: >>> object_property_del_child() previously performed two full iterations >>> over obj->properties: the first to find the matching child property and >>> call its release callback, the second to find the same property again >>> and remove it from the hash table. >>> >>> Merge the two loops into one: when the matching property is found, call >>> the release callback and immediately remove the entry via >>> g_hash_table_iter_remove(), then break. This halves the number of hash >>> table operations in the common case and avoids the redundant second scan. >>> >>> Signed-off-by: Bin Guo <guobin@linux.alibaba.com> >>> --- >>> qom/object.c | 7 ------- >>> 1 file changed, 7 deletions(-) >>> >>> diff --git a/qom/object.c b/qom/object.c >>> index f981e27044..9c0e8dfd02 100644 >>> --- a/qom/object.c >>> +++ b/qom/object.c >>> @@ -629,13 +629,6 @@ static void object_property_del_child(Object *obj, Object *child) >>> prop->release(obj, prop->name, prop->opaque); >>> prop->release = NULL; >>> } >>> - break; >>> - } >>> - } >>> - g_hash_table_iter_init(&iter, obj->properties); >>> - while (g_hash_table_iter_next(&iter, &key, &value)) { >>> - prop = value; >>> - if (object_property_is_child(prop) && prop->opaque == child) { >>> g_hash_table_iter_remove(&iter); >>> break; >>> } >> >> Fixes: b604a854e84 ("qom: Replace object property list with GHashTable") > > On the contrary, this proposed patch breaks that commit. > > The original proposal for that old commit had a single iteration, > but it was discovered that this broke cleanup for certain devices, > as prop->release can have side-effects that trigger assertions in > g_hash_table_iter_remove. This requires a two phase iteration > to be safe. Worth a comment in the code :) > A test for that problem was added in 8c4d156c187c84b574d287bd4b9ddf9a6975de7c > and this new patch proposal duly breaks the check-qom-proplist test. > > With regards, > Daniel > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] block/block-backend: delegate blk_co_preadv to blk_co_preadv_part 2026-04-29 6:20 [PATCH 0/3] Minor cleanups: eliminate redundant code in QOM, block-backend and vl.c Bin Guo 2026-04-29 6:20 ` [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child Bin Guo @ 2026-04-29 6:20 ` Bin Guo 2026-04-29 21:07 ` Richard Henderson 2026-05-12 12:58 ` Kevin Wolf 2026-04-29 6:20 ` [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks Bin Guo 2 siblings, 2 replies; 12+ messages in thread From: Bin Guo @ 2026-04-29 6:20 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, berrange, kwolf, hreitz, qemu-block blk_co_preadv() and blk_co_preadv_part() share identical bodies except that blk_co_preadv() always passes qiov_offset=0. blk_co_pwritev() already uses this pattern and simply calls blk_co_pwritev_part() with qiov_offset=0. Apply the same simplification to the read side so that both pairs are consistent and the shared logic lives in a single place. Before this change blk_co_preadv() duplicated the blk_inc_in_flight / blk_co_do_preadv_part / blk_dec_in_flight sequence. After this change it is a one-liner wrapper, matching the write side. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> --- block/block-backend.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 9944657120..490b149bf8 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1376,14 +1376,8 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags) { - int ret; IO_OR_GS_CODE(); - - blk_inc_in_flight(blk); - ret = blk_co_do_preadv_part(blk, offset, bytes, qiov, 0, flags); - blk_dec_in_flight(blk); - - return ret; + return blk_co_preadv_part(blk, offset, bytes, qiov, 0, flags); } int coroutine_fn blk_co_preadv_part(BlockBackend *blk, int64_t offset, -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] block/block-backend: delegate blk_co_preadv to blk_co_preadv_part 2026-04-29 6:20 ` [PATCH 2/3] block/block-backend: delegate blk_co_preadv to blk_co_preadv_part Bin Guo @ 2026-04-29 21:07 ` Richard Henderson 2026-05-12 12:58 ` Kevin Wolf 1 sibling, 0 replies; 12+ messages in thread From: Richard Henderson @ 2026-04-29 21:07 UTC (permalink / raw) To: qemu-devel On 4/29/26 16:20, Bin Guo wrote: > blk_co_preadv() and blk_co_preadv_part() share identical bodies except > that blk_co_preadv() always passes qiov_offset=0. blk_co_pwritev() > already uses this pattern and simply calls blk_co_pwritev_part() with > qiov_offset=0. Apply the same simplification to the read side so that > both pairs are consistent and the shared logic lives in a single place. > > Before this change blk_co_preadv() duplicated the > blk_inc_in_flight / blk_co_do_preadv_part / blk_dec_in_flight > sequence. After this change it is a one-liner wrapper, matching the > write side. > > Signed-off-by: Bin Guo <guobin@linux.alibaba.com> > --- > block/block-backend.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/block/block-backend.c b/block/block-backend.c > index 9944657120..490b149bf8 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -1376,14 +1376,8 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset, > int64_t bytes, QEMUIOVector *qiov, > BdrvRequestFlags flags) > { > - int ret; > IO_OR_GS_CODE(); > - > - blk_inc_in_flight(blk); > - ret = blk_co_do_preadv_part(blk, offset, bytes, qiov, 0, flags); > - blk_dec_in_flight(blk); > - > - return ret; > + return blk_co_preadv_part(blk, offset, bytes, qiov, 0, flags); > } > > int coroutine_fn blk_co_preadv_part(BlockBackend *blk, int64_t offset, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] block/block-backend: delegate blk_co_preadv to blk_co_preadv_part 2026-04-29 6:20 ` [PATCH 2/3] block/block-backend: delegate blk_co_preadv to blk_co_preadv_part Bin Guo 2026-04-29 21:07 ` Richard Henderson @ 2026-05-12 12:58 ` Kevin Wolf 1 sibling, 0 replies; 12+ messages in thread From: Kevin Wolf @ 2026-05-12 12:58 UTC (permalink / raw) To: Bin Guo; +Cc: qemu-devel, pbonzini, berrange, hreitz, qemu-block Am 29.04.2026 um 08:20 hat Bin Guo geschrieben: > blk_co_preadv() and blk_co_preadv_part() share identical bodies except > that blk_co_preadv() always passes qiov_offset=0. blk_co_pwritev() > already uses this pattern and simply calls blk_co_pwritev_part() with > qiov_offset=0. Apply the same simplification to the read side so that > both pairs are consistent and the shared logic lives in a single place. > > Before this change blk_co_preadv() duplicated the > blk_inc_in_flight / blk_co_do_preadv_part / blk_dec_in_flight > sequence. After this change it is a one-liner wrapper, matching the > write side. > > Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks 2026-04-29 6:20 [PATCH 0/3] Minor cleanups: eliminate redundant code in QOM, block-backend and vl.c Bin Guo 2026-04-29 6:20 ` [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child Bin Guo 2026-04-29 6:20 ` [PATCH 2/3] block/block-backend: delegate blk_co_preadv to blk_co_preadv_part Bin Guo @ 2026-04-29 6:20 ` Bin Guo 2026-04-29 8:04 ` Philippe Mathieu-Daudé 2 siblings, 1 reply; 12+ messages in thread From: Bin Guo @ 2026-04-29 6:20 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, berrange, kwolf, hreitz, qemu-block In qemu_init()'s option parsing switch, several cases assigned the return value of qemu_opts_parse_noisily() to the shared 'opts' variable solely to check for NULL, without using the pointer afterwards. Inline the call directly into the if-condition, matching the style already used by QEMU_OPTION_action. This affects the following options: -drive, -numa, -iscsi, -m, -mon, -chardev, -fsdev, -fwcfg Cases where the returned QemuOpts* is subsequently used (e.g. -acpitable, -smbios, -virtfs) are left unchanged. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> --- system/vl.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/system/vl.c b/system/vl.c index 0e1fc217b4..0e0d3cb761 100644 --- a/system/vl.c +++ b/system/vl.c @@ -2961,9 +2961,8 @@ void qemu_init(int argc, char **argv) break; } case QEMU_OPTION_drive: - opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), - optarg, false); - if (opts == NULL) { + if (!qemu_opts_parse_noisily(qemu_find_opts("drive"), + optarg, false)) { exit(1); } break; @@ -2988,9 +2987,8 @@ void qemu_init(int argc, char **argv) replay_add_blocker("-snapshot"); break; case QEMU_OPTION_numa: - opts = qemu_opts_parse_noisily(qemu_find_opts("numa"), - optarg, true); - if (!opts) { + if (!qemu_opts_parse_noisily(qemu_find_opts("numa"), + optarg, true)) { exit(1); } break; @@ -3049,9 +3047,8 @@ void qemu_init(int argc, char **argv) break; #ifdef CONFIG_LIBISCSI case QEMU_OPTION_iscsi: - opts = qemu_opts_parse_noisily(qemu_find_opts("iscsi"), - optarg, false); - if (!opts) { + if (!qemu_opts_parse_noisily(qemu_find_opts("iscsi"), + optarg, false)) { exit(1); } break; @@ -3104,8 +3101,8 @@ void qemu_init(int argc, char **argv) exit(0); break; case QEMU_OPTION_m: - opts = qemu_opts_parse_noisily(qemu_find_opts("memory"), optarg, true); - if (opts == NULL) { + if (!qemu_opts_parse_noisily(qemu_find_opts("memory"), + optarg, true)) { exit(1); } break; @@ -3226,17 +3223,15 @@ void qemu_init(int argc, char **argv) default_monitor = 0; break; case QEMU_OPTION_mon: - opts = qemu_opts_parse_noisily(qemu_find_opts("mon"), optarg, - true); - if (!opts) { + if (!qemu_opts_parse_noisily(qemu_find_opts("mon"), optarg, + true)) { exit(1); } default_monitor = 0; break; case QEMU_OPTION_chardev: - opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"), - optarg, true); - if (!opts) { + if (!qemu_opts_parse_noisily(qemu_find_opts("chardev"), + optarg, true)) { exit(1); } break; @@ -3246,8 +3241,7 @@ void qemu_init(int argc, char **argv) error_report("fsdev support is disabled"); exit(1); } - opts = qemu_opts_parse_noisily(olist, optarg, true); - if (!opts) { + if (!qemu_opts_parse_noisily(olist, optarg, true)) { exit(1); } break; @@ -3386,9 +3380,8 @@ void qemu_init(int argc, char **argv) smbios_entry_add(opts, &error_fatal); break; case QEMU_OPTION_fwcfg: - opts = qemu_opts_parse_noisily(qemu_find_opts("fw_cfg"), - optarg, true); - if (opts == NULL) { + if (!qemu_opts_parse_noisily(qemu_find_opts("fw_cfg"), + optarg, true)) { exit(1); } break; -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks 2026-04-29 6:20 ` [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks Bin Guo @ 2026-04-29 8:04 ` Philippe Mathieu-Daudé 2026-04-29 21:03 ` Richard Henderson 0 siblings, 1 reply; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2026-04-29 8:04 UTC (permalink / raw) To: Bin Guo, qemu-devel; +Cc: pbonzini, berrange, kwolf, hreitz, qemu-block On 29/4/26 08:20, Bin Guo wrote: > In qemu_init()'s option parsing switch, several cases assigned the > return value of qemu_opts_parse_noisily() to the shared 'opts' > variable solely to check for NULL, without using the pointer > afterwards. Inline the call directly into the if-condition, matching > the style already used by QEMU_OPTION_action. > > This affects the following options: > -drive, -numa, -iscsi, -m, -mon, -chardev, -fsdev, -fwcfg > > Cases where the returned QemuOpts* is subsequently used (e.g. > -acpitable, -smbios, -virtfs) are left unchanged. > > Signed-off-by: Bin Guo <guobin@linux.alibaba.com> > --- > system/vl.c | 37 +++++++++++++++---------------------- > 1 file changed, 15 insertions(+), 22 deletions(-) > > diff --git a/system/vl.c b/system/vl.c > index 0e1fc217b4..0e0d3cb761 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -2961,9 +2961,8 @@ void qemu_init(int argc, char **argv) > break; > } > case QEMU_OPTION_drive: > - opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), > - optarg, false); > - if (opts == NULL) { > + if (!qemu_opts_parse_noisily(qemu_find_opts("drive"), > + optarg, false)) { > exit(1); > } > break; For clarity I'd also squash in this patch: -- >8 -- diff --git a/system/vl.c b/system/vl.c index cab8106984a..237334236aa 100644 --- a/system/vl.c +++ b/system/vl.c @@ -2842,3 +2842,2 @@ void qemu_init(int argc, char **argv) { - QemuOpts *opts; QemuOpts *icount_opts = NULL, *accel_opts = NULL; @@ -2921,2 +2920,4 @@ void qemu_init(int argc, char **argv) for(;;) { + QemuOpts *opts; + if (optind >= argc) --- Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks 2026-04-29 8:04 ` Philippe Mathieu-Daudé @ 2026-04-29 21:03 ` Richard Henderson 2026-04-30 6:07 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 12+ messages in thread From: Richard Henderson @ 2026-04-29 21:03 UTC (permalink / raw) To: qemu-devel On 4/29/26 18:04, Philippe Mathieu-Daudé wrote: > On 29/4/26 08:20, Bin Guo wrote: >> In qemu_init()'s option parsing switch, several cases assigned the >> return value of qemu_opts_parse_noisily() to the shared 'opts' >> variable solely to check for NULL, without using the pointer >> afterwards. Inline the call directly into the if-condition, matching >> the style already used by QEMU_OPTION_action. >> >> This affects the following options: >> -drive, -numa, -iscsi, -m, -mon, -chardev, -fsdev, -fwcfg >> >> Cases where the returned QemuOpts* is subsequently used (e.g. >> -acpitable, -smbios, -virtfs) are left unchanged. >> >> Signed-off-by: Bin Guo <guobin@linux.alibaba.com> >> --- >> system/vl.c | 37 +++++++++++++++---------------------- >> 1 file changed, 15 insertions(+), 22 deletions(-) >> >> diff --git a/system/vl.c b/system/vl.c >> index 0e1fc217b4..0e0d3cb761 100644 >> --- a/system/vl.c >> +++ b/system/vl.c >> @@ -2961,9 +2961,8 @@ void qemu_init(int argc, char **argv) >> break; >> } >> case QEMU_OPTION_drive: >> - opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), >> - optarg, false); >> - if (opts == NULL) { >> + if (!qemu_opts_parse_noisily(qemu_find_opts("drive"), >> + optarg, false)) { >> exit(1); >> } >> break; > > For clarity I'd also squash in this patch: > > -- >8 -- > diff --git a/system/vl.c b/system/vl.c > index cab8106984a..237334236aa 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -2842,3 +2842,2 @@ void qemu_init(int argc, char **argv) > { > - QemuOpts *opts; > QemuOpts *icount_opts = NULL, *accel_opts = NULL; > @@ -2921,2 +2920,4 @@ void qemu_init(int argc, char **argv) > for(;;) { > + QemuOpts *opts; > + > if (optind >= argc) > --- > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> If you're going to move the decl, the innermost block is just below with } else { const QEMUOption *popt; + QemuOpts *opts; popt = lookup_opt(argc, argv, &optarg, &optind); r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks 2026-04-29 21:03 ` Richard Henderson @ 2026-04-30 6:07 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2026-04-30 6:07 UTC (permalink / raw) To: Richard Henderson, qemu-devel On 29/4/26 23:03, Richard Henderson wrote: > On 4/29/26 18:04, Philippe Mathieu-Daudé wrote: >> On 29/4/26 08:20, Bin Guo wrote: >>> In qemu_init()'s option parsing switch, several cases assigned the >>> return value of qemu_opts_parse_noisily() to the shared 'opts' >>> variable solely to check for NULL, without using the pointer >>> afterwards. Inline the call directly into the if-condition, matching >>> the style already used by QEMU_OPTION_action. >>> >>> This affects the following options: >>> -drive, -numa, -iscsi, -m, -mon, -chardev, -fsdev, -fwcfg >>> >>> Cases where the returned QemuOpts* is subsequently used (e.g. >>> -acpitable, -smbios, -virtfs) are left unchanged. >>> >>> Signed-off-by: Bin Guo <guobin@linux.alibaba.com> >>> --- >>> system/vl.c | 37 +++++++++++++++---------------------- >>> 1 file changed, 15 insertions(+), 22 deletions(-) >>> >>> diff --git a/system/vl.c b/system/vl.c >>> index 0e1fc217b4..0e0d3cb761 100644 >>> --- a/system/vl.c >>> +++ b/system/vl.c >>> @@ -2961,9 +2961,8 @@ void qemu_init(int argc, char **argv) >>> break; >>> } >>> case QEMU_OPTION_drive: >>> - opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), >>> - optarg, false); >>> - if (opts == NULL) { >>> + if (!qemu_opts_parse_noisily(qemu_find_opts("drive"), >>> + optarg, false)) { >>> exit(1); >>> } >>> break; >> >> For clarity I'd also squash in this patch: >> >> -- >8 -- >> diff --git a/system/vl.c b/system/vl.c >> index cab8106984a..237334236aa 100644 >> --- a/system/vl.c >> +++ b/system/vl.c >> @@ -2842,3 +2842,2 @@ void qemu_init(int argc, char **argv) >> { >> - QemuOpts *opts; >> QemuOpts *icount_opts = NULL, *accel_opts = NULL; >> @@ -2921,2 +2920,4 @@ void qemu_init(int argc, char **argv) >> for(;;) { >> + QemuOpts *opts; >> + >> if (optind >= argc) >> --- >> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > If you're going to move the decl, the innermost block is just below with > > } else { > const QEMUOption *popt; > + QemuOpts *opts; > > popt = lookup_opt(argc, argv, &optarg, &optind); Good catch. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-05-12 12:59 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-29 6:20 [PATCH 0/3] Minor cleanups: eliminate redundant code in QOM, block-backend and vl.c Bin Guo 2026-04-29 6:20 ` [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child Bin Guo 2026-04-29 21:07 ` Richard Henderson 2026-04-30 9:24 ` Daniel P. Berrangé 2026-04-30 13:36 ` Philippe Mathieu-Daudé 2026-04-29 6:20 ` [PATCH 2/3] block/block-backend: delegate blk_co_preadv to blk_co_preadv_part Bin Guo 2026-04-29 21:07 ` Richard Henderson 2026-05-12 12:58 ` Kevin Wolf 2026-04-29 6:20 ` [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks Bin Guo 2026-04-29 8:04 ` Philippe Mathieu-Daudé 2026-04-29 21:03 ` Richard Henderson 2026-04-30 6:07 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox