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

2015-06-08 Thread 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-block] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

2015-06-08 Thread 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-block] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives

2015-06-08 Thread Peter Maydell
On 8 June 2015 at 09:18, Christian Borntraeger borntrae...@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



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

2015-06-04 Thread 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