Re: [Qemu-devel] [RESEND PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci.

[Frederic Konrad <fred.konrad@greensocs.com>]

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.konrad@greensocs.com wrote:
>>>>> From: KONRAD Frederic <fred.konrad@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-stable@nongnu.org
>>>>> Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>> Reviewed-by: Andreas Färber <afaerber@suse.de>
>>>>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>>>> Acked-by: Michael S. Tsirkin <mst@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?
>
>
>>
>>>>> ---
>>>>>   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);
>>>>>       }
>>>>>   
>>>>>       /*
>