From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35290) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Veznn-0008Ay-Tm for qemu-devel@nongnu.org; Fri, 08 Nov 2013 23:05:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Veznh-0001Ml-K5 for qemu-devel@nongnu.org; Fri, 08 Nov 2013 23:05:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:12290) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Veznh-0001Me-BA for qemu-devel@nongnu.org; Fri, 08 Nov 2013 23:05:33 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rA945WaA012881 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 8 Nov 2013 23:05:32 -0500 Message-ID: <527DB486.50904@redhat.com> Date: Sat, 09 Nov 2013 12:05:26 +0800 From: Fam Zheng MIME-Version: 1.0 References: <20131105163417.GA26314@amosk.info> <1383715014-30499-1-git-send-email-akong@redhat.com> <20131108091802.GA26775@T430s.nay.redhat.com> <20131109031950.GA2450@amosk.info> In-Reply-To: <20131109031950.GA2450@amosk.info> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] qmp: access the local QemuOptsLists for drive option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong Cc: libvir-list@redhat.com, kwolf@redhat.com, qemu-devel@nongnu.org, jyang@redhat.com, lcapitulino@redhat.com On 11/09/2013 11:19 AM, Amos Kong wrote: > 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 >>> --- >>> 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 > Ah yes, I missed it. Fam >>> 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); >