[Qemu-devel] [PATCH 26/28] blockdev: Reject multiple definitions for the same drive

[Kevin Wolf <kwolf@redhat.com>]

From: Markus Armbruster <armbru@redhat.com>

We silently ignore multiple definitions for the same drive:

    $ qemu-system-x86_64 -nodefaults -vnc :1 -S -monitor stdio -drive if=ide,index=1,file=tmp.qcow2 -drive if=ide,index=1,file=nonexistant
    QEMU 0.13.50 monitor - type 'help' for more information
    (qemu) info block
    ide0-hd1: type=hd removable=0 file=tmp.qcow2 backing_file=tmp.img ro=0 drv=qcow2 encrypted=0

With if=none, this can become quite confusing:

    $ qemu-system-x86_64 -nodefaults -vnc :1 -S -monitor stdio -drive if=none,index=1,file=tmp.qcow2,id=eins -drive if=none,index=1,file=nonexistant,id=zwei -device ide-drive,drive=eins -device ide-drive,drive=zwei
    qemu-system-x86_64: -device ide-drive,drive=zwei: Property 'ide-drive.drive' can't find value 'zwei'

The second -device fails, because it refers to drive zwei, which got
silently ignored.

Make multiple drive definitions fail cleanly.

Unfortunately, there's code that relies on multiple drive definitions
being silently ignored: main() merrily adds default drives even when
the user already defined these drives.  Fix that up.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c |    5 +++--
 vl.c       |   45 ++++++++++++++++++++++++++++++---------------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 01228f6..3f7b560 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -429,11 +429,12 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
     }
 
     /*
-     * ignore multiple definitions
+     * catch multiple definitions
      */
 
     if (drive_get(type, bus_id, unit_id) != NULL) {
-        *fatal_error = 0;
+        error_report("drive with bus=%d, unit=%d (index=%d) exists",
+                     bus_id, unit_id, index);
         return NULL;
     }
 
diff --git a/vl.c b/vl.c
index 5399e33..99f5bfe 100644
--- a/vl.c
+++ b/vl.c
@@ -649,6 +649,29 @@ static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
     return 0;
 }
 
+static void default_drive(int enable, int snapshot, int use_scsi,
+                          BlockInterfaceType type, int index,
+                          const char *optstr)
+{
+    QemuOpts *opts;
+
+    if (type == IF_DEFAULT) {
+        type = use_scsi ? IF_SCSI : IF_IDE;
+    }
+
+    if (!enable || drive_get_by_index(type, index)) {
+        return;
+    }
+
+    opts = drive_add(type, index, NULL, optstr);
+    if (snapshot) {
+        drive_enable_snapshot(opts, NULL);
+    }
+    if (drive_init_func(opts, &use_scsi)) {
+        exit(1);
+    }
+}
+
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
 {
     boot_set_handler = func;
@@ -2893,27 +2916,19 @@ int main(int argc, char **argv, char **envp)
 
     blk_mig_init();
 
-    if (default_cdrom) {
-        /* we always create the cdrom drive, even if no disk is there */
-        drive_add(IF_DEFAULT, 2, NULL, CDROM_OPTS);
-    }
-
-    if (default_floppy) {
-        /* we always create at least one floppy */
-        drive_add(IF_FLOPPY, 0, NULL, FD_OPTS);
-    }
-
-    if (default_sdcard) {
-        /* we always create one sd slot, even if no card is in it */
-        drive_add(IF_SD, 0, NULL, SD_OPTS);
-    }
-
     /* open the virtual block devices */
     if (snapshot)
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);
     if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, &machine->use_scsi, 1) != 0)
         exit(1);
 
+    default_drive(default_cdrom, snapshot, machine->use_scsi,
+                  IF_DEFAULT, 2, CDROM_OPTS);
+    default_drive(default_floppy, snapshot, machine->use_scsi,
+                  IF_FLOPPY, 0, FD_OPTS);
+    default_drive(default_sdcard, snapshot, machine->use_scsi,
+                  IF_SD, 0, SD_OPTS);
+
     register_savevm_live(NULL, "ram", 0, 4, NULL, ram_save_live, NULL,
                          ram_load, NULL);
 
-- 
1.7.2.3