qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 0/3] blockdev: Defer creation of implicit PCI


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v2 0/3] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
Date: Tue, 9 Jun 2015 20:16:40 +0100

On 9 June 2015 at 18:48, Peter Maydell <address@hidden> wrote:
> 1) explicit if=foo drive manually wired up to a device is an error
>    (and always diagnosed as such, not indirectly via it being double-used)

I had a look at getting this to be diagnosed properly, and the sketch
of the patch is:
1. add "bool auto_claimed;" to struct DriveInfo
2. everywhere we grab a DriveInfo and auto-connect it to a device,
   set dinfo->auto_claimed to true
3. in drive_check_orphaned() do this:

+        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 drive specified "
+                    "(using if=%s or by not specifying if= at all) "
+                    "but it was also connected manually (try using
if=none ?): "
+                    "id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
+                    if_name[dinfo->type],
+                    blk_name(blk), blk_bs(blk)->filename, if_name[dinfo->type],
+                    dinfo->bus, dinfo->unit);
+            rs = true;
+        }

(We could distinguish the "using if=" and "not specifying if="
but we'd need another bool in the DriveInfo just to track that for
the benefit of the warning.)

This works, but part 2 is a bit awkward. I think it's the case that
we do auto-claiming if and only if somebody calls blk_by_legacy_dinfo()
on the dinfo, but it seems like a hack to add the "auto_claimed = true"
inside that function... (OTOH, that function's called in about 50
places, so it would be very handy not to need to change all of the
callsites!)

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]