[Qemu-devel] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

[Qemu-devel] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

[Peter Maydell]

blockdev.c will create implicit virtio-blk-* devices for IF_VIRTIO
drives. I want to turn this on for the ARM virt board (now it has PCI),
so that users can use shorter and more comprehensible command lines.

Unfortunately, the code as it stands will always create an implicit
device, which means that setting the virt block_default_type to IF_VIRTIO
would break existing command lines like:

  -drive file=arm-wheezy.qcow2,id=foo
  -device virtio-blk-pci,drive=foo

because the creation of the drive causes us to create an implied
virtio-blk-pci which steals the drive, and then the explicit
device creation fails because 'foo' is already in use.

This patchset fixes this problem by deferring the creation of the
implicit devices to drive_check_orphaned(), which means we can do
it only for the drives which haven't been explicitly connected to
a device by the user.

The slight downside of deferral is that it is effectively rearranging
the order in which the devices are created, which means they will
end up in different PCI slots, etc. We can get away with this for PCI,
because at the moment the only machines which set their block_default_type
to IF_VIRTIO are the S390X ones. I have special-cased S390X to retain
the old behaviour, which is a bit ugly but functional. If S390X doesn't
care about cross-version migration we can probably drop that and
just have everything defer. S390X people?

The code in master didn't seem to take much account of the possibility
of hotplug -- if the user created a drive via the monitor we would
apparently try to create the implicit drive, but in fact not do so
because vl.c had already done device creation long before. I've included
a patch that makes it more explicit that hotplug does not get you the
magic implicit devices.

The last patch is the oneliner to enable the default for virt once
the underlying stuff lets us do this without breaking existing user
command lines.


Peter Maydell (4):
  blockdev: Factor out create_implicit_virtio_device
  blockdev: Don't call create_implicit_virtio_device() when it has no
    effect
  blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
  hw/arm/virt: Make block devices default to virtio

 blockdev.c    | 72 +++++++++++++++++++++++++++++++++++++++++++++++------------
 hw/arm/virt.c |  1 +
 2 files changed, 59 insertions(+), 14 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/4] blockdev: Factor out create_implicit_virtio_device

[Peter Maydell]

Factor out the code which creates the implicit virtio device
for IF_VIRTIO drives, so we can call it from drive_check_orphaned().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 blockdev.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index de94a8b..9cf6123 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -212,6 +212,27 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit)
     return NULL;
 }
 
+/* Create the virtio device whose existence is implied by the
+ * creation of a block device with if=virtio.
+ */
+static void create_implicit_virtio_device(const char *driveid,
+                                          const char *devaddr)
+{
+    QemuOpts *devopts;
+
+    devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+                               &error_abort);
+    if (arch_type == QEMU_ARCH_S390X) {
+        qemu_opt_set(devopts, "driver", "virtio-blk-s390", &error_abort);
+    } else {
+        qemu_opt_set(devopts, "driver", "virtio-blk-pci", &error_abort);
+    }
+    qemu_opt_set(devopts, "drive", driveid, &error_abort);
+    if (devaddr) {
+        qemu_opt_set(devopts, "addr", devaddr, &error_abort);
+    }
+}
+
 bool drive_check_orphaned(void)
 {
     BlockBackend *blk;
@@ -929,19 +950,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     }
 
     if (type == IF_VIRTIO) {
-        QemuOpts *devopts;
-        devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
-                                   &error_abort);
-        if (arch_type == QEMU_ARCH_S390X) {
-            qemu_opt_set(devopts, "driver", "virtio-blk-s390", &error_abort);
-        } else {
-            qemu_opt_set(devopts, "driver", "virtio-blk-pci", &error_abort);
-        }
-        qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
-                     &error_abort);
-        if (devaddr) {
-            qemu_opt_set(devopts, "addr", devaddr, &error_abort);
-        }
+        create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
     }
 
     filename = qemu_opt_get(legacy_opts, "file");
-- 
1.9.1

[Qemu-devel] [PATCH 2/4] blockdev: Don't call create_implicit_virtio_device() when it has no effect

[Peter Maydell]

create_implicit_virtio_device() just adds a device to the "device" QEMU
options list. This means that it only has an effect if it is called
before vl.c processes that list to create all the devices. If it is
called after that (ie for hotplug drives created from the monitor) then
it has no effect. To avoid confusion, don't call the code at all if
it isn't going to do anything.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 blockdev.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 9cf6123..177b285 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -78,6 +78,11 @@ static int if_max_devs[IF_COUNT] = {
     [IF_SCSI] = 7,
 };
 
+/* True once we've created all the command line drives and run
+ * drive_check_orphaned().
+ */
+static bool done_orphan_check;
+
 /**
  * Boards may call this to offer board-by-board overrides
  * of the default, global values.
@@ -239,6 +244,8 @@ bool drive_check_orphaned(void)
     DriveInfo *dinfo;
     bool rs = false;
 
+    done_orphan_check = true;
+
     for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
         dinfo = blk_legacy_dinfo(blk);
         /* If dinfo->bdrv->dev is NULL, it has no device attached. */
@@ -949,7 +956,10 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    if (type == IF_VIRTIO) {
+    if (type == IF_VIRTIO && !done_orphan_check) {
+        /* Drives created via the monitor (hotplugged) do not get the
+         * magic implicit device created for them.
+         */
         create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
     }
 
-- 
1.9.1

[Qemu-devel] [PATCH 3/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

[Peter Maydell]

If a user requests an IF_VIRTIO drive on the command line, don't
create the implicit PCI virtio device immediately, but wait until
the rest of the command line has been processed and only create the
device if the drive would otherwise be orphaned. This means that
if the user said drive,id=something,... -device drive=something,..,.
we'll allow the drive to be connected to the user's specified
device rather than stealing it to connect to the implicit virtio
device.

This deferral is *not* done for S390 devices, purely to ensure that
we retain backwards compatibility with command lines. We can
change the behaviour for PCI because right now no machine
specifies a block_default_type of IF_VIRTIO except for the
S390 machines.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 blockdev.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 177b285..c480f64 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -46,6 +46,7 @@
 #include "qmp-commands.h"
 #include "trace.h"
 #include "sysemu/arch_init.h"
+#include "monitor/qdev.h"
 
 static const char *const if_name[IF_COUNT] = {
     [IF_NONE] = "none",
@@ -236,6 +237,17 @@ static void create_implicit_virtio_device(const char *driveid,
     if (devaddr) {
         qemu_opt_set(devopts, "addr", devaddr, &error_abort);
     }
+
+    if (done_orphan_check) {
+        /* If we're called after vl.c has processed the -device options
+         * then we need to create the device ourselves now.
+         */
+        DeviceState *dev = qdev_device_add(devopts);
+        if (!dev) {
+            exit(1);
+        }
+        object_unref(OBJECT(dev));
+    }
 }
 
 bool drive_check_orphaned(void)
@@ -252,6 +264,14 @@ bool drive_check_orphaned(void)
         /* Unless this is a default drive, this may be an oversight. */
         if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
             dinfo->type != IF_NONE) {
+            if (dinfo->type == IF_VIRTIO) {
+                /* An orphaned virtio drive might be waiting for us to
+                 * create the implicit device for it.
+                 */
+                create_implicit_virtio_device(blk_name(blk), dinfo->devaddr);
+                continue;
+            }
+
             fprintf(stderr, "Warning: Orphaned drive without device: "
                     "id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
                     blk_name(blk), blk_bs(blk)->filename, if_name[dinfo->type],
@@ -956,8 +976,13 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    if (type == IF_VIRTIO && !done_orphan_check) {
-        /* Drives created via the monitor (hotplugged) do not get the
+    if (type == IF_VIRTIO && !done_orphan_check
+        && arch_type == QEMU_ARCH_S390X) {
+        /* Virtio drives created on the command line get an implicit device
+         * created for them. For non-s390x command line drives, the creation
+         * of the implicit device is deferred to drive_check_orphaned. (S390x
+         * is special-cased purely for backwards compatibility.)
+         * Drives created via the monitor (hotplugged) do not get the
          * magic implicit device created for them.
          */
         create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
-- 
1.9.1

[Qemu-devel] [PATCH 4/4] hw/arm/virt: Make block devices default to virtio

[Peter Maydell]

Now we have virtio-pci, we can make the virt board's default block
device type be IF_VIRTIO. This allows users to use simplified
command lines that don't have to explicitly create virtio-pci-blk
devices; the -hda &c very short options now also work.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0a75cc8..14afe86 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -961,6 +961,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
     mc->init = machvirt_init;
     mc->max_cpus = 8;
     mc->has_dynamic_sysbus = true;
+    mc->block_default_type = IF_VIRTIO;
 }
 
 static const TypeInfo machvirt_info = {
-- 
1.9.1

Re: [Qemu-devel] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

[Christian Borntraeger]

Am 04.06.2015 um 18:20 schrieb Peter Maydell:
> blockdev.c will create implicit virtio-blk-* devices for IF_VIRTIO
> drives. I want to turn this on for the ARM virt board (now it has PCI),
> so that users can use shorter and more comprehensible command lines.
> 
> Unfortunately, the code as it stands will always create an implicit
> device, which means that setting the virt block_default_type to IF_VIRTIO
> would break existing command lines like:
> 
>   -drive file=arm-wheezy.qcow2,id=foo
>   -device virtio-blk-pci,drive=foo
> 
> because the creation of the drive causes us to create an implied
> virtio-blk-pci which steals the drive, and then the explicit
> device creation fails because 'foo' is already in use.
> 
> This patchset fixes this problem by deferring the creation of the
> implicit devices to drive_check_orphaned(), which means we can do
> it only for the drives which haven't been explicitly connected to
> a device by the user.
> 
> The slight downside of deferral is that it is effectively rearranging
> the order in which the devices are created, which means they will
> end up in different PCI slots, etc. We can get away with this for PCI,
> because at the moment the only machines which set their block_default_type
> to IF_VIRTIO are the S390X ones. I have special-cased S390X to retain
> the old behaviour, which is a bit ugly but functional. If S390X doesn't
> care about cross-version migration we can probably drop that and
> just have everything defer. S390X people?

Actually we will care about cross-version migration, the thing is that 
2.4 will be the the first version that migrates all necessary pieces
like local interrupts for current Linux guests. 2.3 and before did work
from time to time but not reliable under load (1)

So migration between 2.3 and 2.4 was broken anyway and we intend to 
care about migration compatiblity with 2.4 and later.

Now: The implicit defintion does only work for the non-ccw machine, due
to the limited aliasing capabilities 

e.g.
# qemu-system-s390x -nographic -enable-kvm -machine s390-ccw .. -drive file=image,format=raw,id=d1 -device virtio-blk-ccw,drive=d1
fails
qemu-system-s390x: -drive file=image,format=raw,id=d1: No 's390-virtio-bus' bus found for device 'virtio-blk-s390'

but 

# qemu-system-s390x -nographic -enable-kvm -machine s390-ccw .. -drive file=image,format=raw,id=d1,if=none -device virtio-blk-ccw,drive=d1
                            ^^^^^^^
works


So I would prefer to not have this workaround and doing 

index c480f64..7627d57 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -976,17 +976,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    if (type == IF_VIRTIO && !done_orphan_check
-        && arch_type == QEMU_ARCH_S390X) {
-        /* Virtio drives created on the command line get an implicit device
-         * created for them. For non-s390x command line drives, the creation
-         * of the implicit device is deferred to drive_check_orphaned. (S390x
-         * is special-cased purely for backwards compatibility.)
-         * Drives created via the monitor (hotplugged) do not get the
-         * magic implicit device created for them.
-         */
-        create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
-    }
 
     filename = qemu_opt_get(legacy_opts, "file");

actually enables the first command line to work as well.
So lets get rid of the s390 special handling (but I really appreciate that
you cared about s390)


(1) storage key migration is still missing but newer Linux guests do
not care. Lets see if we can provide this before 2.4
> 
> The code in master didn't seem to take much account of the possibility
> of hotplug -- if the user created a drive via the monitor we would
> apparently try to create the implicit drive, but in fact not do so
> because vl.c had already done device creation long before. I've included
> a patch that makes it more explicit that hotplug does not get you the
> magic implicit devices.
> 
> The last patch is the oneliner to enable the default for virt once
> the underlying stuff lets us do this without breaking existing user
> command lines.
> 
> 
> Peter Maydell (4):
>   blockdev: Factor out create_implicit_virtio_device
>   blockdev: Don't call create_implicit_virtio_device() when it has no
>     effect
>   blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
>   hw/arm/virt: Make block devices default to virtio
> 
>  blockdev.c    | 72 +++++++++++++++++++++++++++++++++++++++++++++++------------
>  hw/arm/virt.c |  1 +
>  2 files changed, 59 insertions(+), 14 deletions(-)
> 

Re: [Qemu-devel] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

[Christian Borntraeger]

Am 08.06.2015 um 10:02 schrieb Christian Borntraeger:
> So I would prefer to not have this workaround and doing 
> 
> index c480f64..7627d57 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -976,17 +976,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>          goto fail;
>      }
>  
> -    if (type == IF_VIRTIO && !done_orphan_check
> -        && arch_type == QEMU_ARCH_S390X) {
> -        /* Virtio drives created on the command line get an implicit device
> -         * created for them. For non-s390x command line drives, the creation
> -         * of the implicit device is deferred to drive_check_orphaned. (S390x
> -         * is special-cased purely for backwards compatibility.)
> -         * Drives created via the monitor (hotplugged) do not get the
> -         * magic implicit device created for them.
> -         */
> -        create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
> -    }
>  
>      filename = qemu_opt_get(legacy_opts, "file");
> 
> actually enables the first command line to work as well.
> So lets get rid of the s390 special handling (but I really appreciate that
> you cared about s390)

As a side note, I cannot test this with libvirt right now, as current qemu broke
libvirts capability query
see
https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg01806.html
https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg01488.html


Christian

Re: [Qemu-devel] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

[Peter Maydell]

On 8 June 2015 at 09:18, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> Am 08.06.2015 um 10:02 schrieb Christian Borntraeger:
>> So I would prefer to not have this workaround and doing
>>
>> index c480f64..7627d57 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -976,17 +976,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>>          goto fail;
>>      }
>>
>> -    if (type == IF_VIRTIO && !done_orphan_check
>> -        && arch_type == QEMU_ARCH_S390X) {
>> -        /* Virtio drives created on the command line get an implicit device
>> -         * created for them. For non-s390x command line drives, the creation
>> -         * of the implicit device is deferred to drive_check_orphaned. (S390x
>> -         * is special-cased purely for backwards compatibility.)
>> -         * Drives created via the monitor (hotplugged) do not get the
>> -         * magic implicit device created for them.
>> -         */
>> -        create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
>> -    }
>>
>>      filename = qemu_opt_get(legacy_opts, "file");
>>
>> actually enables the first command line to work as well.
>> So lets get rid of the s390 special handling

Cool, that certainly makes the code nicer not to have to keep the
special case. I'll respin with that change folded in.

>> (but I really appreciate that you cared about s390)

Non-x86 architectures have to stick together, you know :-)

> As a side note, I cannot test this with libvirt right now, as current qemu broke
> libvirts capability query
> see
> https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg01806.html
> https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg01488.html

There's a fix for that that I've just now committed to master.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> blockdev.c will create implicit virtio-blk-* devices for IF_VIRTIO
> drives. I want to turn this on for the ARM virt board (now it has PCI),
> so that users can use shorter and more comprehensible command lines.

I had to read further until understood your goal: you want to make
-drive default to if=virtio for this machine type.  It currently
defaults to if=ide, but the board doesn't pick those up.

> Unfortunately, the code as it stands will always create an implicit
> device, which means that setting the virt block_default_type to IF_VIRTIO
> would break existing command lines like:
>
>   -drive file=arm-wheezy.qcow2,id=foo
>   -device virtio-blk-pci,drive=foo

This works basically by accident.  The documented way to do it is -drive
if=none.  Omitting if= like you do defaults to the board's
block_default_type, in this case if=ide.

With a board that actually implements if=ide, such as a PC, your command
line is correctly rejected:

    -device virtio-blk-pci,drive=foo: Property 'virtio-blk-device.drive' can't take value 'foo', it's in use

Below the hood, the board creates a suitable device model and attaches
the drive to it.  When -device tries to attach, it fails.

With a board that doesn't implement if=ide, such as ARM virt, the drive
remains unconnected.  Evidence: without the -device using it, we get

    Warning: Orphaned drive without device: id=foo,file=arm-wheezy.qcow2,if=ide,bus=0,unit=0

Since the drive remains unconnected, -device using it succeeds, even
though if=ide.  -device should really require if=none.  But since it has
never bothered to, this misuse may have hardened into de facto ABI.

Same for any block_default_type other than IF_NONE.

> because the creation of the drive causes us to create an implied
> virtio-blk-pci which steals the drive, and then the explicit
> device creation fails because 'foo' is already in use.

Yes, changing a board's block_default_type changes the meaning of -drive
without if=none.  Works as designed :)

> This patchset fixes this problem by deferring the creation of the
> implicit devices to drive_check_orphaned(), which means we can do
> it only for the drives which haven't been explicitly connected to
> a device by the user.

This changes QEMU from rejecting a certain pattern of incorrect usage to
guessing what the user meant.  Example:

    $ qemu-system-x86_64 -nodefaults -display none -drive if=virtio,file=tmp.qcow2,id=foo -device ide-hd,drive=foo

The user asks for the drive to be both a virtio and an IDE device, which
is of course nonsense.

Before your patch, we reject the nonsense:

    qemu-system-x86_64: -drive if=virtio,file=tmp.qcow2,id=foo: Property 'virtio-blk-device.drive' can't take value 'foo', it's in use

Afterwards, we accept it, guessing the user really meant -device ide=hd,
and not if=virtio.

But if you try the same with if=scsi, we still reject it.

Is this really a good idea?

> The slight downside of deferral is that it is effectively rearranging
> the order in which the devices are created, which means they will
> end up in different PCI slots, etc. We can get away with this for PCI,
> because at the moment the only machines which set their block_default_type
> to IF_VIRTIO are the S390X ones. I have special-cased S390X to retain
> the old behaviour, which is a bit ugly but functional. If S390X doesn't
> care about cross-version migration we can probably drop that and
> just have everything defer. S390X people?
>
> The code in master didn't seem to take much account of the possibility
> of hotplug -- if the user created a drive via the monitor

Full monitor command, please?

>                                                           we would
> apparently try to create the implicit drive, but in fact not do so
> because vl.c had already done device creation long before. I've included
> a patch that makes it more explicit that hotplug does not get you the
> magic implicit devices.

PATCH 2/4.  Haven't thought through its implications.

> The last patch is the oneliner to enable the default for virt once
> the underlying stuff lets us do this without breaking existing user
> command lines.

Re: [Qemu-devel] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

[Peter Maydell]

On 8 June 2015 at 15:18, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> blockdev.c will create implicit virtio-blk-* devices for IF_VIRTIO
>> drives. I want to turn this on for the ARM virt board (now it has PCI),
>> so that users can use shorter and more comprehensible command lines.
>
> I had to read further until understood your goal: you want to make
> -drive default to if=virtio for this machine type.  It currently
> defaults to if=ide, but the board doesn't pick those up.
>
>> Unfortunately, the code as it stands will always create an implicit
>> device, which means that setting the virt block_default_type to IF_VIRTIO
>> would break existing command lines like:
>>
>>   -drive file=arm-wheezy.qcow2,id=foo
>>   -device virtio-blk-pci,drive=foo
>
> This works basically by accident.  The documented way to do it is -drive
> if=none.  Omitting if= like you do defaults to the board's
> block_default_type, in this case if=ide.

Yes, but since we didn't reject these command lines, they
worked anyway. (We should ideally have rejected them as "hey
you asked for an IDE drive but it wasn't connected to anything".)
So we should avoid breaking them now.

> Since the drive remains unconnected, -device using it succeeds, even
> though if=ide.  -device should really require if=none.  But since it has
> never bothered to, this misuse may have hardened into de facto ABI.
>
> Same for any block_default_type other than IF_NONE.

Indeed.

>> This patchset fixes this problem by deferring the creation of the
>> implicit devices to drive_check_orphaned(), which means we can do
>> it only for the drives which haven't been explicitly connected to
>> a device by the user.
>
> This changes QEMU from rejecting a certain pattern of incorrect usage to
> guessing what the user meant.  Example:
>
>     $ qemu-system-x86_64 -nodefaults -display none -drive if=virtio,file=tmp.qcow2,id=foo -device ide-hd,drive=foo
>
> The user asks for the drive to be both a virtio and an IDE device, which
> is of course nonsense.
>
> Before your patch, we reject the nonsense:
>
>     qemu-system-x86_64: -drive if=virtio,file=tmp.qcow2,id=foo: Property 'virtio-blk-device.drive' can't take value 'foo', it's in use
>
> Afterwards, we accept it, guessing the user really meant -device ide=hd,
> and not if=virtio.
>
> But if you try the same with if=scsi, we still reject it.
>
> Is this really a good idea?

We're only rejecting it by accident, I think, because the PC machine
happens to support SCSI. If you remove the pc_pci_device_init()
code in pc.c which creates SCSI adaptors, then we will happily
accept the command line and plug the if=scsi drive into the
IDE adaptor.

Similarly, the pc machine will currently accept the combination
 -drive if=sd,...,id=foo -device ide=hd,drive=foo

If we care about catching mismatched drive-types and devices, we
should actually check that, rather than rejecting them if the
machine supports devices for that drive type and ignoring the
mismatch if it does not.

>> The slight downside of deferral is that it is effectively rearranging
>> the order in which the devices are created, which means they will
>> end up in different PCI slots, etc. We can get away with this for PCI,
>> because at the moment the only machines which set their block_default_type
>> to IF_VIRTIO are the S390X ones. I have special-cased S390X to retain
>> the old behaviour, which is a bit ugly but functional. If S390X doesn't
>> care about cross-version migration we can probably drop that and
>> just have everything defer. S390X people?
>>
>> The code in master didn't seem to take much account of the possibility
>> of hotplug -- if the user created a drive via the monitor
>
> Full monitor command, please?

I haven't tested creating anything via the monitor. I merely
noted that the other code path calling drive_new() is via
hmp_drive_add(). (I notice now that that will actually fail
on anything other than an IF_NONE drive, so it's just as well
we weren't creating devices for IF_VIRTIO, or we'd probably
have leaked them.)

>>                                                           we would
>> apparently try to create the implicit drive, but in fact not do so
>> because vl.c had already done device creation long before. I've included
>> a patch that makes it more explicit that hotplug does not get you the
>> magic implicit devices.
>
> PATCH 2/4.  Haven't thought through its implications.

It's not supposed to have implications, it's supposed to be
a "no behaviour change" patch :-)

Without the s390 special casing patches 2 and 3 collapse down
into a single patch, incidentally.

-- PMM

[Qemu-devel] [PATCH 4/4] hw/arm/virt: Make block devices default to virtio

[Peter Maydell]

Now we have virtio-pci, we can make the virt board's default block
device type be IF_VIRTIO. This allows users to use simplified
command lines that don't have to explicitly create virtio-pci-blk
devices; the -hda &c very short options now also work.

This means we also need to set no_cdrom to avoid getting a
default cdrom device -- this is needed because the virtio-blk
device will fail if it is connected to a block backend with
no media, which is what the default cdrom device typically is.
Providing a cdrom with media via -cdrom will still work.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 1b1cc71..c42ca32 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -961,6 +961,8 @@ static void virt_class_init(ObjectClass *oc, void *data)
     mc->init = machvirt_init;
     mc->max_cpus = 8;
     mc->has_dynamic_sysbus = true;
+    mc->block_default_type = IF_VIRTIO;
+    mc->no_cdrom = 1;
 }
 
 static const TypeInfo machvirt_info = {
-- 
1.9.1