qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Frederic Konrad
Subject: Re: [Qemu-devel] [RESEND PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci.
Date: Thu, 20 Jun 2013 10:26:18 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4

On 18/06/2013 17:21, Michael S. Tsirkin wrote:
On Fri, Jun 14, 2013 at 08:13:29AM +0200, Frederic Konrad wrote:
On 13/06/2013 09:59, Michael S. Tsirkin wrote:
On Thu, Jun 13, 2013 at 09:34:30AM +0200, Frederic Konrad wrote:
On 13/06/2013 09:23, Michael S. Tsirkin wrote:
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, address@hidden
wrote:
From: KONRAD Frederic <address@hidden>

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: address@hidden
Reported-by: Alexey Kardashevskiy <address@hidden>
Reviewed-by: Andreas Färber <address@hidden>
Signed-off-by: KONRAD Frederic <address@hidden>
Acked-by: Michael S. Tsirkin <address@hidden>

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 <address@hidden>


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?
Yes why not seems a good idea, but you'll pass it through bus= option?
This will solve the problem neatly without virtio specific hacks,
won't it?
The issue here is command line back-compatibility for pci_addr,
which won't be solved with
the "single bus" idea?
Why not? This code:
     scsibus = (SCSIBus *)
         object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
                             TYPE_SCSI_BUS);
should be replaced with code from qdev that we'll write
that goes down the chain as long as there's 1 device
on each bus, looking for a device of the appropriate type.
Ok, understood what you mean :).

Why not if everybody is happy with it.

Fred
Ok so - want to try implementing this?

Ok, will try to look at it next week.

What about the stable release?
Wouldn't be safe to take this patch for the stable?

Fred

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]