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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

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.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?


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 <aik@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.



>>
>>
>>>
>>>>>> ---
>>>>>>   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