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@xxxxxxxxxx>
> ---
> 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);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list