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 <ak...@redhat.com>
> > ---
> > 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);
--
Amos.
