On Thu, Jun 13, 2013 at 04:46:09PM +1000, Alexey Kardashevskiy wrote: > On 06/13/2013 04:28 PM, Frederic Konrad wrote: > > On 12/06/2013 13:21, Alexey Kardashevskiy wrote: > >> On 06/12/2013 07:16 PM, Michael S. Tsirkin wrote: > >>> On Wed, Jun 12, 2013 at 07:04:48PM +1000, Alexey Kardashevskiy wrote: > >>>> On 06/12/2013 07:03 PM, Michael S. Tsirkin wrote: > >>>>> On Wed, Jun 12, 2013 at 08:15:17AM +0200, fred.kon...@greensocs.com > >>>>> wrote: > >>>>>> From: KONRAD Frederic <fred.kon...@greensocs.com> > >>>>>> > >>>>>> This fix a bug with scsi hotplug on virtio-scsi-pci: > >>>>>> > >>>>>> As virtio-scsi-pci doesn't have any scsi bus, we need to forward > >>>>>> scsi-hot-add > >>>>>> to the virtio-scsi-device plugged on the virtio-bus. > >>>>>> > >>>>>> Cc: qemu-sta...@nongnu.org > >>>>>> Reported-by: Alexey Kardashevskiy <a...@ozlabs.ru> > >>>>>> Reviewed-by: Andreas Färber <afaer...@suse.de> > >>>>>> Signed-off-by: KONRAD Frederic <fred.kon...@greensocs.com> > >>>>> Acked-by: Michael S. Tsirkin <m...@redhat.com> > >>>>> > >>>>> Note: we don't seem to have any decent way to > >>>>> add disks to devices: no QMP interface, > >>>>> pci address is required instead of using an id ... > >>>>> > >>>>> Anyone can be bothered to fix this? > >>>> > >>>> Actually PCI address is not always required, this field (we are talking > >>>> about "drive_add"?) is ignored when "if=none". > >>>> > >>> Then documentation in hmp-commands.hx is wrong, isn't it? > >>> Add that to the list. > >>> > >>> if=none can't be actually used to hot-add > >>> a disk to a device, can it? It creates a disc and assumes you will > >>> use it by a device created later. > >> > >> Yep. I run QEMU with -device "virtio-scsi-pci,id=device0" and then do in > >> console: > >> drive_add auto file=virtimg/fc18guest,if=none,id=bar1 > >> device_add scsi-disk,bus=device0.0,drive=bar1 > >> > >> Pretty hot plug :) > > > > I thought you use drive_add 0 if=scsi? > > > That's the other option, I posted a bug but I did not actually try the fix > till now :) > > It works now if I run QEMU with "-device virtio-scsi-pci" and do this in > qemu console: > drive_add 0 file=virtimg/fc18guest > > No extra parameters or anything, cool, thanks, and :) > > Tested-by: Alexey Kardashevskiy <a...@ozlabs.ru> > > > The only problem with it that it still wants PCI SCSI adapter while > spapr-vscsi is VIO device so if the guest kernel does not have virtio-scsi > support, I have to do what I described in the quote but this is a different > story.
Okay. How about: - document that pci_addr is optional in hmp - if no pci_addr assume if=none - add drive_add to qmp without the pci_addr and if options We are left with the bus=device0.0 syntax for device_add which is also gross - user asked for device0, the .0 part is qemu internals exposed to users. How about teaching qdev that if there's a single bus under a device, naming the device itself should be identical? This will solve the problem neatly without virtio specific hacks, won't it? > > > >> > >> > >>> > >>>>>> --- > >>>>>> hw/pci/pci-hotplug.c | 19 +++++++++++++++++-- > >>>>>> 1 file changed, 17 insertions(+), 2 deletions(-) > >>>>>> > >>>>>> diff --git a/hw/pci/pci-hotplug.c b/hw/pci/pci-hotplug.c > >>>>>> index 12287d1..c708752 100644 > >>>>>> --- a/hw/pci/pci-hotplug.c > >>>>>> +++ b/hw/pci/pci-hotplug.c > >>>>>> @@ -30,6 +30,8 @@ > >>>>>> #include "monitor/monitor.h" > >>>>>> #include "hw/scsi/scsi.h" > >>>>>> #include "hw/virtio/virtio-blk.h" > >>>>>> +#include "hw/virtio/virtio-scsi.h" > >>>>>> +#include "hw/virtio/virtio-pci.h" > >>>>>> #include "qemu/config-file.h" > >>>>>> #include "sysemu/blockdev.h" > >>>>>> #include "qapi/error.h" > >>>>>> @@ -79,13 +81,26 @@ static int scsi_hot_add(Monitor *mon, DeviceState > >>>>>> *adapter, > >>>>>> { > >>>>>> SCSIBus *scsibus; > >>>>>> SCSIDevice *scsidev; > >>>>>> + VirtIOPCIProxy *virtio_proxy; > >>>>>> scsibus = (SCSIBus *) > >>>>>> object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)), > >>>>>> TYPE_SCSI_BUS); > >>>>>> if (!scsibus) { > >>>>>> - error_report("Device is not a SCSI adapter"); > >>>>>> - return -1; > >>>>>> + /* > >>>>>> + * Check if the adapter is a virtio-scsi-pci, and forward > >>>>>> scsi_hot_add > >>>>>> + * to the virtio-scsi-device. > >>>>>> + */ > >>>>>> + if (!object_dynamic_cast(OBJECT(adapter), > >>>>>> TYPE_VIRTIO_SCSI_PCI)) { > >>>>>> + error_report("Device is not a SCSI adapter"); > >>>>>> + return -1; > >>>>>> + } > >>>>>> + virtio_proxy = VIRTIO_PCI(adapter); > >>>>>> + adapter = DEVICE(virtio_proxy->bus.vdev); > >>>>>> + scsibus = (SCSIBus *) > >>>>>> + > >>>>>> object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)), > >>>>>> + TYPE_SCSI_BUS); > >>>>>> + assert(scsibus); > >>>>>> } > >>>>>> /* > >> > > > > > -- > Alexey