[libvirt] [PATCH RFC] blockdev: copy legacy and common opts to qemu_drive_opts

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[]. We query commandline options by checking information in vm_config_groups[], so we can only get a NULL parameter list now. This patch copied desc items of qemu_legacy_drive_opts and qemu_common_drive_opts to qemu_drive_opts. Signed-off-by: Amos Kong <akong@redhat.com> --- blockdev.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 164 insertions(+), 4 deletions(-) diff --git a/blockdev.c b/blockdev.c index b260477..28f3078 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2345,10 +2345,170 @@ QemuOptsList qemu_drive_opts = { .name = "drive", .head = QTAILQ_HEAD_INITIALIZER(qemu_drive_opts.head), .desc = { - /* - * no elements => accept any params - * validation will happen later - */ + /* qemu_legacy_drive_opts */ + { + .name = "bus", + .type = QEMU_OPT_NUMBER, + .help = "bus number", + },{ + .name = "unit", + .type = QEMU_OPT_NUMBER, + .help = "unit number (i.e. lun for scsi)", + },{ + .name = "index", + .type = QEMU_OPT_NUMBER, + .help = "index number", + },{ + .name = "media", + .type = QEMU_OPT_STRING, + .help = "media type (disk, cdrom)", + },{ + .name = "if", + .type = QEMU_OPT_STRING, + .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", + },{ + .name = "cyls", + .type = QEMU_OPT_NUMBER, + .help = "number of cylinders (ide disk geometry)", + },{ + .name = "heads", + .type = QEMU_OPT_NUMBER, + .help = "number of heads (ide disk geometry)", + },{ + .name = "secs", + .type = QEMU_OPT_NUMBER, + .help = "number of sectors (ide disk geometry)", + },{ + .name = "trans", + .type = QEMU_OPT_STRING, + .help = "chs translation (auto, lba, none)", + },{ + .name = "boot", + .type = QEMU_OPT_BOOL, + .help = "(deprecated, ignored)", + },{ + .name = "addr", + .type = QEMU_OPT_STRING, + .help = "pci address (virtio only)", + }, + + /* Options that are passed on, but have special semantics with -drive */ + { + .name = "read-only", + .type = QEMU_OPT_BOOL, + .help = "open drive file as read-only", + },{ + .name = "copy-on-read", + .type = QEMU_OPT_BOOL, + .help = "copy read data from backing file into image file", + }, + + /* qemu_common_drive_opts */ + { + .name = "snapshot", + .type = QEMU_OPT_BOOL, + .help = "enable/disable snapshot mode", + },{ + .name = "file", + .type = QEMU_OPT_STRING, + .help = "disk image", + },{ + .name = "discard", + .type = QEMU_OPT_STRING, + .help = "discard operation (ignore/off, unmap/on)", + },{ + .name = "cache.writeback", + .type = QEMU_OPT_BOOL, + .help = "enables writeback mode for any caches", + },{ + .name = "cache.direct", + .type = QEMU_OPT_BOOL, + .help = "enables use of O_DIRECT (bypass the host page cache)", + },{ + .name = "cache.no-flush", + .type = QEMU_OPT_BOOL, + .help = "ignore any flush requests for the device", + },{ + .name = "aio", + .type = QEMU_OPT_STRING, + .help = "host AIO implementation (threads, native)", + },{ + .name = "format", + .type = QEMU_OPT_STRING, + .help = "disk format (raw, qcow2, ...)", + },{ + .name = "serial", + .type = QEMU_OPT_STRING, + .help = "disk serial number", + },{ + .name = "rerror", + .type = QEMU_OPT_STRING, + .help = "read error action", + },{ + .name = "werror", + .type = QEMU_OPT_STRING, + .help = "write error action", + },{ + .name = "read-only", + .type = QEMU_OPT_BOOL, + .help = "open drive file as read-only", + },{ + .name = "throttling.iops-total", + .type = QEMU_OPT_NUMBER, + .help = "limit total I/O operations per second", + },{ + .name = "throttling.iops-read", + .type = QEMU_OPT_NUMBER, + .help = "limit read operations per second", + },{ + .name = "throttling.iops-write", + .type = QEMU_OPT_NUMBER, + .help = "limit write operations per second", + },{ + .name = "throttling.bps-total", + .type = QEMU_OPT_NUMBER, + .help = "limit total bytes per second", + },{ + .name = "throttling.bps-read", + .type = QEMU_OPT_NUMBER, + .help = "limit read bytes per second", + },{ + .name = "throttling.bps-write", + .type = QEMU_OPT_NUMBER, + .help = "limit write bytes per second", + },{ + .name = "throttling.iops-total-max", + .type = QEMU_OPT_NUMBER, + .help = "I/O operations burst", + },{ + .name = "throttling.iops-read-max", + .type = QEMU_OPT_NUMBER, + .help = "I/O operations read burst", + },{ + .name = "throttling.iops-write-max", + .type = QEMU_OPT_NUMBER, + .help = "I/O operations write burst", + },{ + .name = "throttling.bps-total-max", + .type = QEMU_OPT_NUMBER, + .help = "total bytes burst", + },{ + .name = "throttling.bps-read-max", + .type = QEMU_OPT_NUMBER, + .help = "total bytes read burst", + },{ + .name = "throttling.bps-write-max", + .type = QEMU_OPT_NUMBER, + .help = "total bytes write burst", + },{ + .name = "throttling.iops-size", + .type = QEMU_OPT_NUMBER, + .help = "when limiting by iops max size of an I/O in bytes", + },{ + .name = "copy-on-read", + .type = QEMU_OPT_BOOL, + .help = "copy read data from backing file into image file", + }, { /* end of list */ } }, }; -- 1.8.3.1

Am 04.11.2013 um 08:01 hat Amos Kong geschrieben:
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[].
We query commandline options by checking information in vm_config_groups[], so we can only get a NULL parameter list now.
This patch copied desc items of qemu_legacy_drive_opts and qemu_common_drive_opts to qemu_drive_opts.
Signed-off-by: Amos Kong <akong@redhat.com>
This breaks driver-specific options because they aren't (and cannot be) listed in the QemuOptsList. For example: $ x86_64-softmmu/qemu-system-x86_64 -drive file.driver=nbd,file.host=localhost qemu-system-x86_64: -drive file.driver=nbd,file.host=localhost: Invalid parameter 'file.driver' query-command-line-options isn't an appropriate API to query the -drive capabilities in the blockdev-add world. You really want to have introspection for that. For compatibility, we might want to at least expose part of the provided options there. In this case you should modify the monitor command to access the local QemuOptsLists of drive_init() and blockdev_init() for option="drive". Kevin

Hi Kevin, On Mon, Nov 04, 2013 at 12:27:10PM +0100, Kevin Wolf wrote:
Am 04.11.2013 um 08:01 hat Amos Kong geschrieben:
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[].
We query commandline options by checking information in vm_config_groups[], so we can only get a NULL parameter list now.
This patch copied desc items of qemu_legacy_drive_opts and qemu_common_drive_opts to qemu_drive_opts.
Signed-off-by: Amos Kong <akong@redhat.com>
This breaks driver-specific options because they aren't (and cannot be) listed in the QemuOptsList.
For example:
$ x86_64-softmmu/qemu-system-x86_64 -drive file.driver=nbd,file.host=localhost qemu-system-x86_64: -drive file.driver=nbd,file.host=localhost: Invalid parameter 'file.driver'
query-command-line-options isn't an appropriate API to query the -drive capabilities in the blockdev-add world. You really want to have introspection for that.
For compatibility, we might want to at least expose part of the provided options there. In this case you should modify the monitor command to access the local QemuOptsLists of drive_init() and blockdev_init() for option="drive".
It's to access the local QemuOptsLists of drive_init() and blockdev_init() for option="drive". I saw there are two repeat items ("copy-on-read", "read-only") I will merge the two desc lists together, remove the repeat items, and output once. Attached my draft patch, I can use it to compile qemu binary, but touched following error, any note? ------------------------ [amos@amosk qemu]$ make CC util/qemu-config.o AR libqemuutil.a LINK qemu-ga LINK qemu-nbd LINK qemu-img LINK qemu-io libqemuutil.a(qemu-config.o): In function `qmp_query_command_line_options': /home/devel/qemu/util/qemu-config.c:141: undefined reference to `qemu_legacy_drive_opts' /home/devel/qemu/util/qemu-config.c:142: undefined reference to `qemu_common_drive_opts' /home/devel/qemu/util/qemu-config.c:144: undefined reference to `qemu_drive_opts' collect2: error: ld returned 1 exit status make: *** [qemu-nbd] Error 1 make: *** Waiting for unfinished jobs.... libqemuutil.a(qemu-config.o): In function `qmp_query_command_line_options': /home/devel/qemu/util/qemu-config.c:141: undefined reference to `qemu_legacy_drive_opts' /home/devel/qemu/util/qemu-config.c:142: undefined reference to `qemu_common_drive_opts' /home/devel/qemu/util/qemu-config.c:144: undefined reference to `qemu_drive_opts' collect2: error: ld returned 1 exit status make: *** [qemu-img] Error 1 libqemuutil.a(qemu-config.o): In function `qmp_query_command_line_options': /home/devel/qemu/util/qemu-config.c:141: undefined reference to `qemu_legacy_drive_opts' /home/devel/qemu/util/qemu-config.c:142: undefined reference to `qemu_common_drive_opts' /home/devel/qemu/util/qemu-config.c:144: undefined reference to `qemu_drive_opts' collect2: error: ld returned 1 exit status make: *** [qemu-io] Error 1 LINK x86_64-softmmu/qemu-system-x86_64 -- Amos.

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@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); 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 { + 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); -- 1.8.3.1

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@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?
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);

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@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.

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 <akong@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
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);

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@redhat.com> --- V2: access local QemuOptsLists for drive (kevin) V3: fix indent (fam) Signed-off-by: Amos Kong <akong@redhat.com> --- 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); 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..04da942 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 { + 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); -- 1.8.3.1

Am 09.11.2013 um 05:15 hat Amos Kong geschrieben:
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@redhat.com>
Thanks, applied to the block branch. Kevin
participants (3)
-
Amos Kong
-
Fam Zheng
-
Kevin Wolf