| Message ID | 1431105726-3682-22-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/08/2015 11:21 AM, Kevin Wolf wrote: > The code already special-cased "node-name", which is currently the only > option passed in the QDict that isn't driver-specific. Generalise the > code to take all general block layer options into consideration. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > > for (entry = qdict_first(bs->options); entry; > entry = qdict_next(bs->options, entry)) > { > - /* Only take options for this level and exclude all non-driver-specific > - * options */ > - if (!strchr(qdict_entry_key(entry), '.') && > - strcmp(qdict_entry_key(entry), "node-name")) > - { > - qobject_incref(qdict_entry_value(entry)); > - qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry)); > - found_any = true; > + /* Only take options for this level */ > + if (strchr(qdict_entry_key(entry), '.')) { > + continue; > } > + > + /* And exclude all non-driver-specific options */ > + for (desc = bdrv_runtime_opts.desc; desc->name; desc++) { > + if (!strcmp(qdict_entry_key(entry), desc->name)) { > + break; > + } > + } > + if (desc->name) { > + continue; If only C had Java's "continue label" notation, which makes it cleaner to jump out of nested loops :) At least you didn't do a backwards "goto". Looks like it will be O(n^2), but since our set of recognized option names is still rather small, I don't think the performance hit will reach a point where scaling matters. If it does, we could switch to binary search for O(n log n) or hash table lookups for O(n) amortized cost, at the cost of code complexity, at a future date. Reviewed-by: Eric Blake <eblake@redhat.com>
On 08.05.2015 19:21, Kevin Wolf wrote: > The code already special-cased "node-name", which is currently the only > option passed in the QDict that isn't driver-specific. Generalise the > code to take all general block layer options into consideration. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) Reviewed-by: Max Reitz <mreitz@redhat.com>
diff --git a/block.c b/block.c index 561cefd..e329a47 100644 --- a/block.c +++ b/block.c @@ -4036,20 +4036,30 @@ out: static bool append_open_options(QDict *d, BlockDriverState *bs) { const QDictEntry *entry; + QemuOptDesc *desc; bool found_any = false; for (entry = qdict_first(bs->options); entry; entry = qdict_next(bs->options, entry)) { - /* Only take options for this level and exclude all non-driver-specific - * options */ - if (!strchr(qdict_entry_key(entry), '.') && - strcmp(qdict_entry_key(entry), "node-name")) - { - qobject_incref(qdict_entry_value(entry)); - qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry)); - found_any = true; + /* Only take options for this level */ + if (strchr(qdict_entry_key(entry), '.')) { + continue; } + + /* And exclude all non-driver-specific options */ + for (desc = bdrv_runtime_opts.desc; desc->name; desc++) { + if (!strcmp(qdict_entry_key(entry), desc->name)) { + break; + } + } + if (desc->name) { + continue; + } + + qobject_incref(qdict_entry_value(entry)); + qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry)); + found_any = true; } return found_any;
The code already special-cased "node-name", which is currently the only option passed in the QDict that isn't driver-specific. Generalise the code to take all general block layer options into consideration. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)