[Qemu-devel] [PATCH 1/4] block: Warn if an if=<something> drive was also connected manually

[Peter Maydell]

Improve the diagnosis of command line errors where the user requested
an automatic connection of a drive (via if=<something>, or by not
setting if= and using the board-default-if). We already fail this
case if the board actually handles if=<something>, but if the board
did not auto-connect the drive we should at least warn about the
problem, since the most likely reason is a forgotten if=none, and
this command line might become an error if the board starts handling
this if= type in future.

To do this we need to identify when a drive is automatically
connected by the board; we do this by assuming that all calls
to blk_by_legacy_dinfo() imply that we're about to assign the
drive to a device. This is a slightly ugly place to make the
test, but simpler than trying to locate and change every place
in the code that does automatic drive handling, and the worst
case is that we might print out a spurious warning.

Signed-off-by: Peter Maydell <address@hidden>
---
 block/block-backend.c     |  4 ++++
 blockdev.c                | 39 +++++++++++++++++++++++++++++++++++++++
 include/sysemu/blockdev.h |  2 ++
 3 files changed, 45 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 93e46f3..a45c207 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -260,6 +260,9 @@ DriveInfo *blk_set_legacy_dinfo(BlockBackend *blk, 
DriveInfo *dinfo)
 /*
  * Return the BlockBackend with DriveInfo @dinfo.
  * It must exist.
+ * For the purposes of providing helpful error messages, we assume
+ * that any call to this function implies that the drive is going
+ * to be automatically claimed by the board model.
  */
 BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo)
 {
@@ -267,6 +270,7 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo)
 
     QTAILQ_FOREACH(blk, &blk_backends, link) {
         if (blk->legacy_dinfo == dinfo) {
+            dinfo->auto_claimed = true;
             return blk;
         }
     }
diff --git a/blockdev.c b/blockdev.c
index de94a8b..97a56b9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -230,6 +230,32 @@ bool drive_check_orphaned(void)
                     dinfo->bus, dinfo->unit);
             rs = true;
         }
+        if (blk_get_attached_dev(blk) && dinfo->type != IF_NONE &&
+            !dinfo->auto_claimed) {
+            /* This drive is attached to something, but it was specified
+             * with if=<something> and it's not attached because it was
+             * automatically claimed by the board code because of the if=
+             * specification. The user probably forgot an if=none.
+             */
+            fprintf(stderr,
+                    "Warning: automatic connection of this drive requested ");
+            if (dinfo->type_is_board_default) {
+                fprintf(stderr, "(because if= was not specified and this "
+                        "machine defaults to if=%s) ",
+                        if_name[dinfo->type]);
+            } else {
+                fprintf(stderr, "(because if=%s was specified) ",
+                        if_name[dinfo->type]);
+            }
+            fprintf(stderr,
+                    "but it was also connected manually to a device: "
+                    "id=%s,file=%s,if=%s,bus=%d,unit=%d\n"
+                    "(If you don't want this drive auto-connected, "
+                    "use if=none.)\n",
+                    blk_name(blk), blk_bs(blk)->filename, if_name[dinfo->type],
+                    dinfo->bus, dinfo->unit);
+            rs = true;
+        }
     }
 
     return rs;
@@ -683,6 +709,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType 
block_default_type)
     const char *werror, *rerror;
     bool read_only = false;
     bool copy_on_read;
+    bool type_is_board_default = false;
     const char *serial;
     const char *filename;
     Error *local_err = NULL;
@@ -808,6 +835,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType 
block_default_type)
         }
     } else {
         type = block_default_type;
+        type_is_board_default = true;
     }
 
     /* Geometry */
@@ -994,6 +1022,17 @@ DriveInfo *drive_new(QemuOpts *all_opts, 
BlockInterfaceType block_default_type)
     dinfo->devaddr = devaddr;
     dinfo->serial = g_strdup(serial);
 
+    if (type == IF_VIRTIO) {
+        /* We created the automatic device earlier. For other types this
+         * will be set to true at the point where the drive is claimed
+         * by the IDE/SCSI/etc bus, when that code calls blk_by_legacy_dinfo()
+         * to find the block backend from the drive.
+         */
+        dinfo->auto_claimed = true;
+    }
+
+    dinfo->type_is_board_default = type_is_board_default;
+
     blk_set_legacy_dinfo(blk, dinfo);
 
     switch(type) {
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 3104150..f9c44e2 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -36,6 +36,8 @@ struct DriveInfo {
     int unit;
     int auto_del;               /* see blockdev_mark_auto_del() */
     bool is_default;            /* Added by default_drive() ?  */
+    bool auto_claimed;          /* Automatically claimed by board model? */
+    bool type_is_board_default; /* type is from board default, not user 'if=' 
*/
     int media_cd;
     int cyls, heads, secs, trans;
     QemuOpts *opts;
-- 
1.9.1