Re: [libvirt] [Qemu-devel] [PATCH v2] qmp: access the local QemuOptsLists for drive option

Looks good to me. Two small comments below. On Wed, 11/06 13:16, Amos Kong wrote: > Currently we have three QemuOptsList (qemu_common_drive_opts, > qemu_legacy_drive_opts, and qemu_drive_opts), only qemu_drive_opts > is added to vm_config_groups[]. > > This patch changes query-command-line-options to access three local > QemuOptsLists for drive option, and merge the description items > together. > > Signed-off-by: Amos Kong <akong(a)redhat.com&gt; > --- > V2: access local QemuOptsLists for drive (kevin) > --- > blockdev.c | 1 - > include/qemu/config-file.h | 1 + > include/sysemu/sysemu.h | 2 ++ > util/qemu-config.c | 77 +++++++++++++++++++++++++++++++++++++++++++++- > vl.c | 3 ++ > 5 files changed, 82 insertions(+), 2 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index b260477..f09f991 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -47,7 +47,6 @@ > #include "sysemu/arch_init.h" > > static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives); > -extern QemuOptsList qemu_common_drive_opts; > > static const char *const if_name[IF_COUNT] = { > [IF_NONE] = "none", > diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h > index ad4a9e5..508428f 100644 > --- a/include/qemu/config-file.h > +++ b/include/qemu/config-file.h > @@ -8,6 +8,7 @@ > QemuOptsList *qemu_find_opts(const char *group); > QemuOptsList *qemu_find_opts_err(const char *group, Error **errp); > void qemu_add_opts(QemuOptsList *list); > +void qemu_add_drive_opts(QemuOptsList *list); This can be static. Do you plan to use it outside qemu-config.c?

> int qemu_set_option(const char *str); > int qemu_global_option(const char *str); > void qemu_add_globals(void); > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index cd5791e..495dae8 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -193,6 +193,8 @@ QemuOpts *qemu_get_machine_opts(void); > > bool usb_enabled(bool default_usb); > > +extern QemuOptsList qemu_legacy_drive_opts; > +extern QemuOptsList qemu_common_drive_opts; > extern QemuOptsList qemu_drive_opts; > extern QemuOptsList qemu_chardev_opts; > extern QemuOptsList qemu_device_opts; > diff --git a/util/qemu-config.c b/util/qemu-config.c > index a59568d..867fb4b 100644 > --- a/util/qemu-config.c > +++ b/util/qemu-config.c > @@ -8,6 +8,7 @@ > #include "qmp-commands.h" > > static QemuOptsList *vm_config_groups[32]; > +static QemuOptsList *drive_config_groups[4]; > > static QemuOptsList *find_list(QemuOptsList **lists, const char *group, > Error **errp) > @@ -77,6 +78,59 @@ static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc *desc) > return param_list; > } > > +/* remove repeated entry from the info list */ > +static void cleanup_infolist(CommandLineParameterInfoList *head) > +{ > + CommandLineParameterInfoList *pre_entry, *cur, *del_entry; > + > + cur = head; > + while (cur->next) { > + pre_entry = head; > + while (pre_entry != cur->next) { > + if (!strcmp(pre_entry->value->name, cur->next->value->name)) { > + del_entry = cur->next; > + cur->next = cur->next->next; > + g_free(del_entry); > + break; > + } > + pre_entry = pre_entry->next; > + } > + cur = cur->next; > + } > +} > + > +/* merge the description items of two parameter infolists */ > +static void connect_infolist(CommandLineParameterInfoList *head, > + CommandLineParameterInfoList *new) > +{ > + CommandLineParameterInfoList *cur; > + > + cur = head; > + while (cur->next) { > + cur = cur->next; > + } > + cur->next = new; > +} > + > +/* access all the local QemuOptsLists for drive option */ > +static CommandLineParameterInfoList *get_drive_infolist(void) > +{ > + CommandLineParameterInfoList *head = NULL, *cur; > + int i; > + > + for (i = 0; drive_config_groups[i] != NULL; i++) { > + if (!head) { > + head = query_option_descs(drive_config_groups[i]->desc); > + } else { > + cur = query_option_descs(drive_config_groups[i]->desc); > + connect_infolist(head, cur); > + } > + } > + cleanup_infolist(head); > + > + return head; > +} > + > CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, > const char *option, > Error **errp) > @@ -89,7 +143,12 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, > if (!has_option || !strcmp(option, vm_config_groups[i]->name)) { > info = g_malloc0(sizeof(*info)); > info->option = g_strdup(vm_config_groups[i]->name); > - info->parameters = query_option_descs(vm_config_groups[i]->desc); > + if (!strcmp("drive", vm_config_groups[i]->name)) { > + info->parameters = get_drive_infolist(); > + } else { Misaligned line.

Fam > + info->parameters = > + query_option_descs(vm_config_groups[i]->desc); > + } > entry = g_malloc0(sizeof(*entry)); > entry->value = info; > entry->next = conf_list; > @@ -109,6 +168,22 @@ QemuOptsList *qemu_find_opts_err(const char *group, Error **errp) > return find_list(vm_config_groups, group, errp); > } > > +void qemu_add_drive_opts(QemuOptsList *list) > +{ > + int entries, i; > + > + entries = ARRAY_SIZE(drive_config_groups); > + entries--; /* keep list NULL terminated */ > + for (i = 0; i < entries; i++) { > + if (drive_config_groups[i] == NULL) { > + drive_config_groups[i] = list; > + return; > + } > + } > + fprintf(stderr, "ran out of space in drive_config_groups"); > + abort(); > +} > + > void qemu_add_opts(QemuOptsList *list) > { > int entries, i; > diff --git a/vl.c b/vl.c > index efbff65..341d7b5 100644 > --- a/vl.c > +++ b/vl.c > @@ -2869,6 +2869,9 @@ int main(int argc, char **argv, char **envp) > module_call_init(MODULE_INIT_QOM); > > qemu_add_opts(&qemu_drive_opts); > + qemu_add_drive_opts(&qemu_legacy_drive_opts); > + qemu_add_drive_opts(&qemu_common_drive_opts); > + qemu_add_drive_opts(&qemu_drive_opts);

> qemu_add_opts(&qemu_chardev_opts); > qemu_add_opts(&qemu_device_opts); > qemu_add_opts(&qemu_netdev_opts);