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

On Fri, Nov 08, 2013 at 05:18:02PM +0800, Fam Zheng wrote: > 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? It's used in vl.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. Thanks for the catch. > 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);