[Qemu-devel] [PATCH 0/10] QemuOpts+qdev patches.

[Qemu-devel] [PATCH 0/10] QemuOpts+qdev patches.

[Gerd Hoffmann]

  Hi,

Here is a respin of the QemuOpts patch series.
What is in there?

The first three patches are bugfixes.  Same as yesterday.  These
should be applied even if the following patches need more discussion.

Next in the series are patches implementing a generic -set switch
instead of -drive-set.  Works for everything which uses QemuOpts, which
is -drive (upstream alredy) and -device (switched over by one of the
patches) at the moment.

The last patches in the series are new and they finish the qdev support
for virtio-blk-pci, i.e. creating virtio block devices using -device
works then.  Posted as part of this series because they depend on the
QemuOpts fixes and improvements.

Check the individual patch descriptions for details.

cheers,
  Gerd

[Qemu-devel] [PATCH 01/10] QemuOpts: add some functions

[Gerd Hoffmann]

qemu_opt_foreach: loop over all QemuOpts entries.
qemu_opts_id: return QemuOpts id.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-option.c |   19 +++++++++++++++++++
 qemu-option.h |    4 ++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 591d178..7164ee8 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -607,6 +607,20 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
     return 0;
 }
 
+int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
+                     int abort_on_failure)
+{
+    QemuOpt *opt;
+    int rc = 0;
+
+    TAILQ_FOREACH(opt, &opts->head, next) {
+        rc = func(opt->name, opt->str, opaque);
+        if (abort_on_failure  &&  rc != 0)
+            break;
+    }
+    return rc;
+}
+
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id)
 {
     QemuOpts *opts;
@@ -663,6 +677,11 @@ int qemu_opts_set(QemuOptsList *list, const char *id,
     return qemu_opt_set(opts, name, value);
 }
 
+const char *qemu_opts_id(QemuOpts *opts)
+{
+    return opts->id;
+}
+
 void qemu_opts_del(QemuOpts *opts)
 {
     QemuOpt *opt;
diff --git a/qemu-option.h b/qemu-option.h
index 428c947..56c7eac 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -104,11 +104,15 @@ int qemu_opt_get_bool(QemuOpts *opts, const char *name, int defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
+typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque);
+int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
+                     int abort_on_failure);
 
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists);
 int qemu_opts_set(QemuOptsList *list, const char *id,
                   const char *name, const char *value);
+const char *qemu_opts_id(QemuOpts *opts);
 void qemu_opts_del(QemuOpts *opts);
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *firstname);
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 02/10] QemuOpts: qemu_opts_parse: fix id= parsing

[Gerd Hoffmann]

We can't use get_param_value(), it can't handle parameters without
'=' in there.  Examples not working because of that:

  -device foo,id=bar
  -device file=/path/image,format=qcow2,snapshot,id=disk0

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-option.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 7164ee8..61141e0 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -715,8 +715,13 @@ QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *fi
     QemuOpts *opts;
     const char *p,*pe,*pc;
 
-    if (get_param_value(value, sizeof(value), "id", params))
+    if (strncmp(params, "id=", 3) == 0) {
+        get_opt_value(value, sizeof(value), params+3);
         id = qemu_strdup(value);
+    } else if ((p = strstr(params, ",id=")) != NULL) {
+        get_opt_value(value, sizeof(value), p+4);
+        id = qemu_strdup(value);
+    }
     opts = qemu_opts_create(list, id, 1);
     if (opts == NULL)
         return NULL;
-- 
1.6.2.5

[Qemu-devel] [PATCH 03/10] QemuOpts: make the drive id actually show up in "info block".

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index fdd4f03..48331d4 100644
--- a/vl.c
+++ b/vl.c
@@ -2213,7 +2213,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
     /* init */
 
     dinfo = qemu_mallocz(sizeof(*dinfo));
-    if ((buf = qemu_opt_get(opts, "id")) != NULL) {
+    if ((buf = qemu_opts_id(opts)) != NULL) {
         dinfo->id = qemu_strdup(buf);
     } else {
         /* no id supplied -> create one */
-- 
1.6.2.5

[Qemu-devel] [PATCH 04/10] QemuOpts: create qemu-config.h

[Gerd Hoffmann]

Move drive option description there.
Rename it, give it a qemu_ prefix.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.target |    2 +-
 qemu-config.c   |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-config.h   |    1 +
 vl.c            |   76 +++----------------------------------------------------
 4 files changed, 79 insertions(+), 73 deletions(-)
 create mode 100644 qemu-config.c
 create mode 100644 qemu-config.h

diff --git a/Makefile.target b/Makefile.target
index 49ba08d..b611193 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -255,7 +255,7 @@ endif #CONFIG_BSD_USER
 ifndef CONFIG_USER_ONLY
 
 obj-y = vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o \
-        gdbstub.o gdbstub-xml.o msix.o ioport.o
+        gdbstub.o gdbstub-xml.o msix.o ioport.o qemu-config.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
 obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
diff --git a/qemu-config.c b/qemu-config.c
new file mode 100644
index 0000000..786f055
--- /dev/null
+++ b/qemu-config.c
@@ -0,0 +1,73 @@
+#include "qemu-common.h"
+#include "qemu-option.h"
+#include "qemu-config.h"
+
+QemuOptsList qemu_drive_opts = {
+    .name = "drive",
+    .head = TAILQ_HEAD_INITIALIZER(qemu_drive_opts.head),
+    .desc = {
+        {
+            .name = "bus",
+            .type = QEMU_OPT_NUMBER,
+            .help = "bus number",
+        },{
+            .name = "unit",
+            .type = QEMU_OPT_NUMBER,
+            .help = "unit number (i.e. lun for scsi)",
+        },{
+            .name = "if",
+            .type = QEMU_OPT_STRING,
+            .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
+        },{
+            .name = "index",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .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 = "media",
+            .type = QEMU_OPT_STRING,
+            .help = "media type (disk, cdrom)",
+        },{
+            .name = "snapshot",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "file",
+            .type = QEMU_OPT_STRING,
+            .help = "disk image",
+        },{
+            .name = "cache",
+            .type = QEMU_OPT_STRING,
+            .help = "host cache usage (none, writeback, writethrough)",
+        },{
+            .name = "format",
+            .type = QEMU_OPT_STRING,
+            .help = "disk format (raw, qcow2, ...)",
+        },{
+            .name = "serial",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "werror",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "addr",
+            .type = QEMU_OPT_STRING,
+            .help = "pci address (virtio only)",
+        },
+        { /* end if list */ }
+    },
+};
+
diff --git a/qemu-config.h b/qemu-config.h
new file mode 100644
index 0000000..3ada418
--- /dev/null
+++ b/qemu-config.h
@@ -0,0 +1 @@
+extern QemuOptsList qemu_drive_opts;
diff --git a/vl.c b/vl.c
index 48331d4..5a9ff70 100644
--- a/vl.c
+++ b/vl.c
@@ -158,6 +158,7 @@ int main(int argc, char **argv)
 #include "kvm.h"
 #include "balloon.h"
 #include "qemu-option.h"
+#include "qemu-config.h"
 
 #include "disas.h"
 
@@ -1797,75 +1798,6 @@ static int bt_parse(const char *opt)
 #define MTD_ALIAS "if=mtd"
 #define SD_ALIAS "index=0,if=sd"
 
-static QemuOptsList drive_opt_list = {
-    .name = "drive",
-    .head = TAILQ_HEAD_INITIALIZER(drive_opt_list.head),
-    .desc = {
-        {
-            .name = "bus",
-            .type = QEMU_OPT_NUMBER,
-            .help = "bus number",
-        },{
-            .name = "unit",
-            .type = QEMU_OPT_NUMBER,
-            .help = "unit number (i.e. lun for scsi)",
-        },{
-            .name = "if",
-            .type = QEMU_OPT_STRING,
-            .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
-        },{
-            .name = "index",
-            .type = QEMU_OPT_NUMBER,
-        },{
-            .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 = "media",
-            .type = QEMU_OPT_STRING,
-            .help = "media type (disk, cdrom)",
-        },{
-            .name = "snapshot",
-            .type = QEMU_OPT_BOOL,
-        },{
-            .name = "file",
-            .type = QEMU_OPT_STRING,
-            .help = "disk image",
-        },{
-            .name = "cache",
-            .type = QEMU_OPT_STRING,
-            .help = "host cache usage (none, writeback, writethrough)",
-        },{
-            .name = "format",
-            .type = QEMU_OPT_STRING,
-            .help = "disk format (raw, qcow2, ...)",
-        },{
-            .name = "serial",
-            .type = QEMU_OPT_STRING,
-        },{
-            .name = "werror",
-            .type = QEMU_OPT_STRING,
-        },{
-            .name = "addr",
-            .type = QEMU_OPT_STRING,
-            .help = "pci address (virtio only)",
-        },
-        { /* end if list */ }
-    },
-};
-
 QemuOpts *drive_add(const char *file, const char *fmt, ...)
 {
     va_list ap;
@@ -1876,7 +1808,7 @@ QemuOpts *drive_add(const char *file, const char *fmt, ...)
     vsnprintf(optstr, sizeof(optstr), fmt, ap);
     va_end(ap);
 
-    opts = qemu_opts_parse(&drive_opt_list, optstr, NULL);
+    opts = qemu_opts_parse(&qemu_drive_opts, optstr, NULL);
     if (!opts) {
         fprintf(stderr, "%s: huh? duplicate? (%s)\n",
                 __FUNCTION__, optstr);
@@ -5829,8 +5761,8 @@ int main(int argc, char **argv, char **envp)
 
     /* open the virtual block devices */
     if (snapshot)
-        qemu_opts_foreach(&drive_opt_list, drive_enable_snapshot, NULL, 0);
-    if (qemu_opts_foreach(&drive_opt_list, drive_init_func, machine, 1) != 0)
+        qemu_opts_foreach(&qemu_drive_opts, drive_enable_snapshot, NULL, 0);
+    if (qemu_opts_foreach(&qemu_drive_opts, drive_init_func, machine, 1) != 0)
         exit(1);
 
     register_savevm("timer", 0, 2, timer_save, timer_load, NULL);
-- 
1.6.2.5

[Qemu-devel] [PATCH 05/10] QemuOpts: add -set option

[Gerd Hoffmann]

One use case will be file for drives (no filename quoting issues), i.e.

	-drive id=test,if=virtio
	-set drive.test.file=/vmdisk/test-virtio.img

It will work for any other option (assuming handled by QemuOpts) though.
Except for id= for obvious reasons ;).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-config.c   |   41 +++++++++++++++++++++++++++++++++++++++++
 qemu-config.h   |    2 ++
 qemu-options.hx |    6 +++++-
 vl.c            |    4 ++++
 4 files changed, 52 insertions(+), 1 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 786f055..bcfb6eb 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -71,3 +71,44 @@ QemuOptsList qemu_drive_opts = {
     },
 };
 
+static QemuOptsList *lists[] = {
+    &qemu_drive_opts,
+    NULL,
+};
+
+int qemu_set_option(const char *str)
+{
+    char group[64], id[64], arg[64];
+    QemuOpts *opts;
+    int i, rc, offset;
+
+    rc = sscanf(str, "%63[^.].%63[^.].%63[^=]%n", group, id, arg, &offset);
+    if (rc < 3 || str[offset] != '=') {
+        fprintf(stderr, "can't parse: \"%s\"\n", str);
+        return -1;
+    }
+
+    for (i = 0; lists[i] != NULL; i++) {
+        if (strcmp(lists[i]->name, group) == 0)
+            break;
+    }
+    if (lists[i] == NULL) {
+        fprintf(stderr, "there is no option group \"%s\"\n", group);
+        return -1;
+    }
+
+    opts = qemu_opts_find(lists[i], id);
+    if (!opts) {
+        fprintf(stderr, "there is no %s \"%s\" defined\n",
+                lists[i]->name, id);
+        return -1;
+    }
+
+    if (-1 == qemu_opt_set(opts, arg, str+offset+1)) {
+        fprintf(stderr, "failed to set \"%s\" for %s \"%s\"\n",
+                arg, lists[i]->name, id);
+        return -1;
+    }
+    return 0;
+}
+
diff --git a/qemu-config.h b/qemu-config.h
index 3ada418..7faec9b 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -1 +1,3 @@
 extern QemuOptsList qemu_drive_opts;
+
+int qemu_set_option(const char *str);
diff --git a/qemu-options.hx b/qemu-options.hx
index 1b420a3..38989f1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -95,8 +95,12 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
     "       [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
-    "       [,addr=A]\n"
+    "       [,addr=A][,id=name]\n"
     "                use 'file' as a drive image\n")
+DEF("set", HAS_ARG, QEMU_OPTION_set,
+    "-set group.id.arg=value\n"
+    "                set <arg> parameter for item <id> of type <group>\n"
+    "                i.e. -set drive.$id.file=/path/to/image\n")
 STEXI
 @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
 
diff --git a/vl.c b/vl.c
index 5a9ff70..698182e 100644
--- a/vl.c
+++ b/vl.c
@@ -4963,6 +4963,10 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_drive:
                 drive_add(NULL, "%s", optarg);
 	        break;
+            case QEMU_OPTION_set:
+                if (qemu_set_option(optarg) != 0)
+                    exit(1);
+	        break;
             case QEMU_OPTION_mtdblock:
                 drive_add(optarg, MTD_ALIAS);
                 break;
-- 
1.6.2.5

[Qemu-devel] [PATCH 06/10] QemuOpts: switch over -device.

[Gerd Hoffmann]

Make -device switch use the QemuOpts framework.
Everything should continue to work like it did before.

New: "-set device.$id.$property=$value" works.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c     |   78 +++++++++++++++++++++++++++-----------------------------
 hw/qdev.h     |    5 ++-
 qemu-config.c |   14 ++++++++++
 qemu-config.h |    1 +
 vl.c          |   31 ++++++++++++----------
 5 files changed, 73 insertions(+), 56 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 9488dba..eae442f 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -120,18 +120,33 @@ static int qdev_print_devinfo(DeviceInfo *info, char *dest, int len)
     return pos;
 }
 
-DeviceState *qdev_device_add(const char *cmdline)
+static int set_property(const char *name, const char *value, void *opaque)
 {
+    DeviceState *dev = opaque;
+
+    if (strcmp(name, "driver") == 0)
+        return 0;
+    if (strcmp(name, "bus") == 0)
+        return 0;
+
+    if (-1 == qdev_prop_parse(dev, name, value)) {
+        fprintf(stderr, "can't set property \"%s\" to \"%s\" for \"%s\"\n",
+                name, value, dev->info->name);
+        return -1;
+    }
+    return 0;
+}
+
+DeviceState *qdev_device_add(QemuOpts *opts)
+{
+    const char *driver, *path, *id;
     DeviceInfo *info;
     DeviceState *qdev;
     BusState *bus;
-    char driver[32], path[128] = "";
-    char tag[32], value[256];
-    const char *params = NULL;
-    int n = 0;
 
-    if (1 != sscanf(cmdline, "%32[^,],%n", driver, &n)) {
-        fprintf(stderr, "device parse error: \"%s\"\n", cmdline);
+    driver = qemu_opt_get(opts, "driver");
+    if (!driver) {
+        fprintf(stderr, "-device: no driver specified\n");
         return NULL;
     }
     if (strcmp(driver, "?") == 0) {
@@ -142,10 +157,8 @@ DeviceState *qdev_device_add(const char *cmdline)
         }
         return NULL;
     }
-    if (n) {
-        params = cmdline + n;
-        get_param_value(path, sizeof(path), "bus",  params);
-    }
+
+    /* find driver */
     info = qdev_find_info(NULL, driver);
     if (!info) {
         fprintf(stderr, "Device \"%s\" not found.  Try -device '?' for a list.\n",
@@ -158,40 +171,26 @@ DeviceState *qdev_device_add(const char *cmdline)
         return NULL;
     }
 
-    if (strlen(path)) {
+    /* find bus */
+    path = qemu_opt_get(opts, "bus");
+    if (path != NULL) {
         bus = qbus_find(path);
-        if (!bus)
-            return NULL;
-        qdev = qdev_create(bus, driver);
     } else {
         bus = qbus_find_recursive(main_system_bus, NULL, info->bus_info);
-        if (!bus)
-            return NULL;
-        qdev = qdev_create(bus, driver);
     }
+    if (!bus)
+        return NULL;
 
-    if (params) {
-        while (params[0]) {
-            if (2 != sscanf(params, "%31[^=]=%255[^,]%n", tag, value, &n)) {
-                fprintf(stderr, "parse error at \"%s\"\n", params);
-                break;
-            }
-            params += n;
-            if (params[0] == ',')
-                params++;
-            if (strcmp(tag, "bus") == 0)
-                continue;
-            if (strcmp(tag, "id") == 0) {
-                qdev->id = qemu_strdup(value);
-                continue;
-            }
-            if (-1 == qdev_prop_parse(qdev, tag, value)) {
-                fprintf(stderr, "can't set property \"%s\" to \"%s\" for \"%s\"\n",
-                        tag, value, driver);
-            }
-        }
+    /* create device, set properties */
+    qdev = qdev_create(bus, driver);
+    id = qemu_opts_id(opts);
+    if (id) {
+        qdev->id = id;
+    }
+    if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
+        qdev_free(qdev);
+        return NULL;
     }
-
     qdev_init(qdev);
     return qdev;
 }
@@ -208,7 +207,6 @@ void qdev_init(DeviceState *dev)
 void qdev_free(DeviceState *dev)
 {
     LIST_REMOVE(dev, sibling);
-    qemu_free(dev->id);
     qemu_free(dev);
 }
 
diff --git a/hw/qdev.h b/hw/qdev.h
index b342afb..0f0acb1 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -3,6 +3,7 @@
 
 #include "hw.h"
 #include "sys-queue.h"
+#include "qemu-option.h"
 
 typedef struct Property Property;
 
@@ -19,7 +20,7 @@ typedef struct BusInfo BusInfo;
 /* This structure should not be accessed directly.  We declare it here
    so that it can be embedded in individual device state structures.  */
 struct DeviceState {
-    char *id;
+    const char *id;
     DeviceInfo *info;
     BusState *parent_bus;
     int num_gpio_out;
@@ -82,7 +83,7 @@ struct CompatProperty {
 /*** Board API.  This should go away once we have a machine config file.  ***/
 
 DeviceState *qdev_create(BusState *bus, const char *name);
-DeviceState *qdev_device_add(const char *cmdline);
+DeviceState *qdev_device_add(QemuOpts *opts);
 void qdev_init(DeviceState *dev);
 void qdev_free(DeviceState *dev);
 
diff --git a/qemu-config.c b/qemu-config.c
index bcfb6eb..3dd473a 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -71,8 +71,22 @@ QemuOptsList qemu_drive_opts = {
     },
 };
 
+QemuOptsList qemu_device_opts = {
+    .name = "device",
+    .head = TAILQ_HEAD_INITIALIZER(qemu_device_opts.head),
+    .desc = {
+        /*
+         * no elements => accept any
+         * sanity checking will happen later
+         * when setting device properties
+         */
+        { /* end if list */ }
+    },
+};
+
 static QemuOptsList *lists[] = {
     &qemu_drive_opts,
+    &qemu_device_opts,
     NULL,
 };
 
diff --git a/qemu-config.h b/qemu-config.h
index 7faec9b..08629de 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -1,3 +1,4 @@
 extern QemuOptsList qemu_drive_opts;
+extern QemuOptsList qemu_device_opts;
 
 int qemu_set_option(const char *str);
diff --git a/vl.c b/vl.c
index 698182e..1a47497 100644
--- a/vl.c
+++ b/vl.c
@@ -4713,9 +4713,18 @@ char *qemu_find_file(int type, const char *name)
     return buf;
 }
 
+static int device_init_func(QemuOpts *opts, void *opaque)
+{
+    DeviceState *dev;
+
+    dev = qdev_device_add(opts);
+    if (!dev)
+        return -1;
+    return 0;
+}
+
 struct device_config {
     enum {
-        DEV_GENERIC,   /* -device      */
         DEV_USB,       /* -usbdevice   */
         DEV_BT,        /* -bt          */
     } type;
@@ -4749,16 +4758,6 @@ static int foreach_device_config(int type, int (*func)(const char *cmdline))
     return 0;
 }
 
-static int generic_parse(const char *cmdline)
-{
-    DeviceState *dev;
-
-    dev = qdev_device_add(cmdline);
-    if (!dev)
-        return -1;
-    return 0;
-}
-
 int main(int argc, char **argv, char **envp)
 {
     const char *gdbstub_dev = NULL;
@@ -4773,7 +4772,7 @@ int main(int argc, char **argv, char **envp)
     int cyls, heads, secs, translation;
     const char *net_clients[MAX_NET_CLIENTS];
     int nb_net_clients;
-    QemuOpts *hda_opts = NULL;
+    QemuOpts *hda_opts = NULL, *opts;
     int optind;
     const char *r, *optarg;
     CharDriverState *monitor_hd = NULL;
@@ -5383,7 +5382,11 @@ int main(int argc, char **argv, char **envp)
                 add_device_config(DEV_USB, optarg);
                 break;
             case QEMU_OPTION_device:
-                add_device_config(DEV_GENERIC, optarg);
+                opts = qemu_opts_parse(&qemu_device_opts, optarg, "driver");
+                if (!opts) {
+                    fprintf(stderr, "parse error: %s\n", optarg);
+                    exit(1);
+                }
                 break;
             case QEMU_OPTION_smp:
             {
@@ -5919,7 +5922,7 @@ int main(int argc, char **argv, char **envp)
     }
 
     /* init generic devices */
-    if (foreach_device_config(DEV_GENERIC, generic_parse))
+    if (qemu_opts_foreach(&qemu_device_opts, device_init_func, NULL, 1) != 0)
         exit(1);
 
     if (!display_state)
-- 
1.6.2.5

[Qemu-devel] [PATCH 07/10] constify drive_get_by_id arg

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 sysemu.h |    2 +-
 vl.c     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sysemu.h b/sysemu.h
index 6af88d8..21e132c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -183,7 +183,7 @@ extern TAILQ_HEAD(drivelist, DriveInfo) drives;
 extern TAILQ_HEAD(driveoptlist, DriveOpt) driveopts;
 
 extern DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
-extern DriveInfo *drive_get_by_id(char *id);
+extern DriveInfo *drive_get_by_id(const char *id);
 extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(BlockDriverState *bdrv);
 extern const char *drive_get_serial(BlockDriverState *bdrv);
diff --git a/vl.c b/vl.c
index 1a47497..12423b2 100644
--- a/vl.c
+++ b/vl.c
@@ -1835,7 +1835,7 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit)
     return NULL;
 }
 
-DriveInfo *drive_get_by_id(char *id)
+DriveInfo *drive_get_by_id(const char *id)
 {
     DriveInfo *dinfo;
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 08/10] add -drive if=none

[Gerd Hoffmann]

This adds a host drive, but doesn't implicitly add a guest drive for it.
First step in splitting host and guest configuration, check the
following patches to see how this can be used ...

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 sysemu.h |    1 +
 vl.c     |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/sysemu.h b/sysemu.h
index 21e132c..ac18890 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -151,6 +151,7 @@ extern unsigned int nb_prom_envs;
 #endif
 
 typedef enum {
+    IF_NONE,
     IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
     IF_COUNT
 } BlockInterfaceType;
diff --git a/vl.c b/vl.c
index 12423b2..2024822 100644
--- a/vl.c
+++ b/vl.c
@@ -1982,6 +1982,9 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
 	} else if (!strcmp(buf, "xen")) {
 	    type = IF_XEN;
             max_devs = 0;
+	} else if (!strcmp(buf, "none")) {
+	    type = IF_NONE;
+            max_devs = 0;
 	} else {
             fprintf(stderr, "qemu: unsupported bus type '%s'\n", buf);
             return NULL;
@@ -2195,6 +2198,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
     case IF_PFLASH:
     case IF_MTD:
     case IF_VIRTIO:
+    case IF_NONE:
         break;
     case IF_COUNT:
         abort();
-- 
1.6.2.5

[Qemu-devel] [PATCH 09/10] qdev/prop: add drive property.

[Gerd Hoffmann]

Adds a (host) drive property, intended to be used by virtual disk
backend drivers.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev-properties.c |   32 ++++++++++++++++++++++++++++++++
 hw/qdev.h            |    4 ++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0e4878e..5a3d10d 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1,3 +1,4 @@
+#include "sysemu.h"
 #include "qdev.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
@@ -141,6 +142,32 @@ PropertyInfo qdev_prop_hex64 = {
     .print = print_hex64,
 };
 
+/* --- drive --- */
+
+static int parse_drive(DeviceState *dev, Property *prop, const char *str)
+{
+    DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
+
+    *ptr = drive_get_by_id(str);
+    if (*ptr == NULL)
+        return -1;
+    return 0;
+}
+
+static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+    DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
+    return snprintf(dest, len, "%s", (*ptr)->id);
+}
+
+PropertyInfo qdev_prop_drive = {
+    .name  = "drive",
+    .type  = PROP_TYPE_DRIVE,
+    .size  = sizeof(DriveInfo*),
+    .parse = parse_drive,
+    .print = print_drive,
+};
+
 /* --- pointer --- */
 
 static int print_ptr(DeviceState *dev, Property *prop, char *dest, size_t len)
@@ -325,6 +352,11 @@ void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value)
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT64);
 }
 
+void qdev_prop_set_drive(DeviceState *dev, const char *name, DriveInfo *value)
+{
+    qdev_prop_set(dev, name, &value, PROP_TYPE_DRIVE);
+}
+
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_PTR);
diff --git a/hw/qdev.h b/hw/qdev.h
index 0f0acb1..91d9f5f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -2,6 +2,7 @@
 #define QDEV_H
 
 #include "hw.h"
+#include "sysemu.h"
 #include "sys-queue.h"
 #include "qemu-option.h"
 
@@ -63,6 +64,7 @@ enum PropertyType {
     PROP_TYPE_UINT64,
     PROP_TYPE_TADDR,
     PROP_TYPE_MACADDR,
+    PROP_TYPE_DRIVE,
     PROP_TYPE_PTR,
 };
 
@@ -155,6 +157,7 @@ extern PropertyInfo qdev_prop_hex32;
 extern PropertyInfo qdev_prop_hex64;
 extern PropertyInfo qdev_prop_ptr;
 extern PropertyInfo qdev_prop_macaddr;
+extern PropertyInfo qdev_prop_drive;
 extern PropertyInfo qdev_prop_pci_devfn;
 
 /* Set properties between creation and init.  */
@@ -164,6 +167,7 @@ void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyT
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
 void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
+void qdev_prop_set_drive(DeviceState *dev, const char *name, DriveInfo *value);
 /* FIXME: Remove opaque pointer properties.  */
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
 void qdev_prop_set_defaults(DeviceState *dev, Property *props);
-- 
1.6.2.5

[Qemu-devel] [PATCH 10/10] qdev-ify virtio-blk.

[Gerd Hoffmann]

First user of the new drive property.  With this patch applied host
and guest config can be specified separately, like this:

  -drive if=none,id=disk1,file=/path/to/disk.img
  -device virtio-blk-pci,drive=disk1

You can set any property for virtio-blk-pci now.  You can set the pci
address via addr=.  You can switch the device into 0.10 compat mode
using class=0x0180.  As this is per device you can have one 0.10 and one
0.11 virtio block device in a single virtual machine.

Old syntax continues to work.  Internally it does the same as the two
lines above though.  One side effect this has is a different
initialization order, which might result in a different pci address
being assigned by default.

Long term plan here is to have this working for all block devices, i.e.
once all scsi is properly qdev-ified you will be able to do something
like this:

  -drive if=none,id=sda,file=/path/to/disk.img
  -device lsi,id=lsi,addr=<pciaddr>
  -device scsi-disk,drive=sda,bus=lsi.0,lun=<n>

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pc.c                |   12 ------------
 hw/pci-hotplug.c       |    1 +
 hw/ppc440_bamboo.c     |   11 -----------
 hw/ppce500_mpc8544ds.c |   11 -----------
 hw/virtio-blk.c        |   10 ++++------
 hw/virtio-pci.c        |   12 ++++++++++--
 hw/virtio.h            |    3 ++-
 vl.c                   |    9 ++++++++-
 8 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index bc9e646..6e5669c 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1409,18 +1409,6 @@ static void pc_init1(ram_addr_t ram_size,
         }
     }
 
-    /* Add virtio block devices */
-    if (pci_enabled) {
-        int unit_id = 0;
-
-        while ((dinfo = drive_get(IF_VIRTIO, 0, unit_id)) != NULL) {
-            pci_dev = pci_create("virtio-blk-pci",
-                                 dinfo->devaddr);
-            qdev_init(&pci_dev->qdev);
-            unit_id++;
-        }
-    }
-
     /* Add virtio balloon device */
     if (pci_enabled && virtio_balloon) {
         pci_dev = pci_create("virtio-balloon-pci", virtio_balloon_devaddr);
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 43675e2..4da916c 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -136,6 +136,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
         break;
     case IF_VIRTIO:
         dev = pci_create("virtio-blk-pci", devaddr);
+        qdev_prop_set_drive(&dev->qdev, "drive", dinfo);
         break;
     default:
         dev = NULL;
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index 5011679..3c59f33 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -90,7 +90,6 @@ static void bamboo_init(ram_addr_t ram_size,
 {
     unsigned int pci_irq_nrs[4] = { 28, 27, 26, 25 };
     PCIBus *pcibus;
-    PCIDevice *pci_dev;
     CPUState *env;
     uint64_t elf_entry;
     uint64_t elf_lowaddr;
@@ -102,21 +101,11 @@ static void bamboo_init(ram_addr_t ram_size,
     target_ulong dt_base = 0;
     void *fdt;
     int i;
-    DriveInfo *dinfo;
 
     /* Setup CPU. */
     env = ppc440ep_init(&ram_size, &pcibus, pci_irq_nrs, 1, cpu_model);
 
     if (pcibus) {
-        int unit_id = 0;
-
-        /* Add virtio block devices. */
-        while ((dinfo = drive_get(IF_VIRTIO, 0, unit_id)) != NULL) {
-            pci_dev = pci_create("virtio-blk-pci", dinfo->devaddr);
-            qdev_init(&pci_dev->qdev);
-            unit_id++;
-        }
-
         /* Add virtio console devices */
         for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
             if (virtcon_hds[i]) {
diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index d154c7f..5120821 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -157,7 +157,6 @@ static void mpc8544ds_init(ram_addr_t ram_size,
                          const char *cpu_model)
 {
     PCIBus *pci_bus;
-    PCIDevice *pci_dev;
     CPUState *env;
     uint64_t elf_entry;
     uint64_t elf_lowaddr;
@@ -172,7 +171,6 @@ static void mpc8544ds_init(ram_addr_t ram_size,
     unsigned int pci_irq_nrs[4] = {1, 2, 3, 4};
     qemu_irq *irqs, *mpic, *pci_irqs;
     SerialState * serial[2];
-    DriveInfo *dinfo;
 
     /* Setup CPU */
     env = cpu_ppc_init("e500v2_v30");
@@ -217,15 +215,6 @@ static void mpc8544ds_init(ram_addr_t ram_size,
     isa_mmio_init(MPC8544_PCI_IO, MPC8544_PCI_IOLEN);
 
     if (pci_bus) {
-        int unit_id = 0;
-
-        /* Add virtio block devices. */
-        while ((dinfo = drive_get(IF_VIRTIO, 0, unit_id)) != NULL) {
-            pci_dev = pci_create("virtio-blk-pci", dinfo->devaddr);
-            qdev_init(&pci_dev->qdev);
-            unit_id++;
-        }
-
         /* Register network interfaces. */
         for (i = 0; i < nb_nics; i++) {
             pci_nic_init(&nd_table[i], "virtio", NULL);
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 5036b5b..c278d2e 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -414,29 +414,27 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-VirtIODevice *virtio_blk_init(DeviceState *dev)
+VirtIODevice *virtio_blk_init(DeviceState *dev, DriveInfo *dinfo)
 {
     VirtIOBlock *s;
     int cylinders, heads, secs;
     static int virtio_blk_id;
-    BlockDriverState *bs;
     char *ps;
 
     s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
                                           sizeof(struct virtio_blk_config),
                                           sizeof(VirtIOBlock));
 
-    bs = qdev_init_bdrv(dev, IF_VIRTIO);
     s->vdev.get_config = virtio_blk_update_config;
     s->vdev.get_features = virtio_blk_get_features;
     s->vdev.reset = virtio_blk_reset;
-    s->bs = bs;
+    s->bs = dinfo->bdrv;
     s->rq = NULL;
-    if (strlen(ps = (char *)drive_get_serial(bs)))
+    if (strlen(ps = (char *)drive_get_serial(s->bs)))
         strncpy(s->serial_str, ps, sizeof(s->serial_str));
     else
         snprintf(s->serial_str, sizeof(s->serial_str), "0");
-    bs->private = dev;
+    s->bs->private = dev;
     bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
     bdrv_set_geometry_hint(s->bs, cylinders, heads, secs);
 
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 703f4fe..8cf4b57 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -17,7 +17,7 @@
 
 #include "virtio.h"
 #include "pci.h"
-//#include "sysemu.h"
+#include "sysemu.h"
 #include "msix.h"
 #include "net.h"
 
@@ -89,6 +89,7 @@ typedef struct {
     uint32_t addr;
     uint32_t class_code;
     uint32_t nvectors;
+    DriveInfo *dinfo;
 } VirtIOPCIProxy;
 
 /* virtio device */
@@ -432,7 +433,10 @@ static void virtio_blk_init_pci(PCIDevice *pci_dev)
         proxy->class_code != PCI_CLASS_STORAGE_OTHER)
         proxy->class_code = PCI_CLASS_STORAGE_SCSI;
 
-    vdev = virtio_blk_init(&pci_dev->qdev);
+    if (!proxy->dinfo) {
+        fprintf(stderr, "drive property not set\n");
+    }
+    vdev = virtio_blk_init(&pci_dev->qdev, proxy->dinfo);
     virtio_init_pci(proxy, vdev,
                     PCI_VENDOR_ID_REDHAT_QUMRANET,
                     PCI_DEVICE_ID_VIRTIO_BLOCK,
@@ -502,6 +506,10 @@ static PCIDeviceInfo virtio_info[] = {
                 .name   = "class",
                 .info   = &qdev_prop_hex32,
                 .offset = offsetof(VirtIOPCIProxy, class_code),
+            },{
+                .name   = "drive",
+                .info   = &qdev_prop_drive,
+                .offset = offsetof(VirtIOPCIProxy, dinfo),
             },
             {/* end of list */}
         },
diff --git a/hw/virtio.h b/hw/virtio.h
index aa55677..c441a93 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -16,6 +16,7 @@
 
 #include "hw.h"
 #include "qdev.h"
+#include "sysemu.h"
 
 /* from Linux's linux/virtio_config.h */
 
@@ -161,7 +162,7 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
                         void *opaque);
 
 /* Base devices.  */
-VirtIODevice *virtio_blk_init(DeviceState *dev);
+VirtIODevice *virtio_blk_init(DeviceState *dev, DriveInfo *dinfo);
 VirtIODevice *virtio_net_init(DeviceState *dev);
 VirtIODevice *virtio_console_init(DeviceState *dev);
 VirtIODevice *virtio_balloon_init(DeviceState *dev);
diff --git a/vl.c b/vl.c
index 2024822..e9ea526 100644
--- a/vl.c
+++ b/vl.c
@@ -2197,9 +2197,16 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
         break;
     case IF_PFLASH:
     case IF_MTD:
-    case IF_VIRTIO:
     case IF_NONE:
         break;
+    case IF_VIRTIO:
+        /* add virtio block device */
+        opts = qemu_opts_create(&qemu_device_opts, NULL, 0);
+        qemu_opt_set(opts, "driver", "virtio-blk-pci");
+        qemu_opt_set(opts, "drive", dinfo->id);
+        if (devaddr)
+            qemu_opt_set(opts, "addr", devaddr);
+        break;
     case IF_COUNT:
         abort();
     }
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 01/10] QemuOpts: add some functions

[Luiz Capitulino]

On Fri, 31 Jul 2009 12:25:32 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> qemu_opt_foreach: loop over all QemuOpts entries.
> qemu_opts_id: return QemuOpts id.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qemu-option.c |   19 +++++++++++++++++++
>  qemu-option.h |    4 ++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu-option.c b/qemu-option.c
> index 591d178..7164ee8 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -607,6 +607,20 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
>      return 0;
>  }
>  
> +int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
> +                     int abort_on_failure)
> +{
> +    QemuOpt *opt;
> +    int rc = 0;
> +
> +    TAILQ_FOREACH(opt, &opts->head, next) {
> +        rc = func(opt->name, opt->str, opaque);
> +        if (abort_on_failure  &&  rc != 0)
> +            break;
> +    }
> +    return rc;
> +}
> +
>  QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id)
>  {
>      QemuOpts *opts;
> @@ -663,6 +677,11 @@ int qemu_opts_set(QemuOptsList *list, const char *id,
>      return qemu_opt_set(opts, name, value);
>  }
>  
> +const char *qemu_opts_id(QemuOpts *opts)
> +{
> +    return opts->id;
> +}
> +

 Can't you constify *opts in those functions?

Re: [Qemu-devel] [PATCH 05/10] QemuOpts: add -set option

[Anthony Liguori]

Gerd Hoffmann wrote:
> One use case will be file for drives (no filename quoting issues), i.e.
>
> 	-drive id=test,if=virtio
> 	-set drive.test.file=/vmdisk/test-virtio.img
>
> It will work for any other option (assuming handled by QemuOpts) though.
> Except for id= for obvious reasons ;).
>   

How can we make it work for id?

That is, it would be good to be able to fully define the drive via 
-set.  I think this is necessary to introduce a config file.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 05/10] QemuOpts: add -set option

[Gerd Hoffmann]

On 07/31/09 19:01, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> One use case will be file for drives (no filename quoting issues), i.e.
>>
>> -drive id=test,if=virtio
>> -set drive.test.file=/vmdisk/test-virtio.img
>>
>> It will work for any other option (assuming handled by QemuOpts) though.
>> Except for id= for obvious reasons ;).
>
> How can we make it work for id?

Do we want to?

> That is, it would be good to be able to fully define the drive via -set.
> I think this is necessary to introduce a config file.

Here is a short writeup how IDs are handled in the current implementation:

-drive params
   creates a new drive without id.  You can't use -set with that one.

-drive params,id=foo
   creates a new drive named 'foo'.  Trying to create another one named
   'foo' doesn't work, qemu will complain that it already exists.

-set drive.foo.param=value
   configure the (existing) drive foo.

So you need '-drive id=something' as minimum to create a new drive, then 
you can set all parameters via -set if you want.

Ordering of the command line options is important, the -drive switch 
which creates the new drive must come first.


This all isn't set in stone, it can be changed without too much effort. 
  We can make -set silently create a new drive in case the used id 
doesn't exist.  We can allow two -drive switches with the same id, so 
you could do -drive id=foo,if=scsi -drive id=foo,unit=3 and have the 
same effect as -drive id=foo,id=scsi,unit=3.  I just found the current 
behavior most useful for the command line.  If you mistype an ID you 
most likely get a useful error message from the parser for example.


A config file is a completely different story, the config file parser 
can implement different behavior.  It also depends on the config file 
format of course.  We could use git-style:

[drive "foo"]
   if=scsi
   unit=3
   file=/path/to/image

In that case we don't have that problem in the first place ;)

We could use somthing simliar to -set, i.e.

drive.foo.if = scsi
drive.foo.unit = 4
drive.foo.file = /path/to/image

Then the parser will of course create drive "foo" when it finds config 
entries for it.

cheers,
   Gerd