diff mbox

[RFC] Allow adding empty SCSI controllers

Message ID 4B27C7B1.9030609@siemens.com
State New
Headers show

Commit Message

Wolfgang Mauerer Dec. 15, 2009, 5:30 p.m. UTC 
Hi Gerd,

in commit 5b684b5a56e81f6f, you introduced an explicit check
to prevent adding SCSI controllers without attached disks
to the system. Is there any other method to introduce
disk-less controllers into the system? If not, I'd suggest
to remove the check (patch is attached) -- there are some
situations when you want empty SCSI controllers, for instance for
the libvirt hotplug/remove framework currently under development
(see http://article.gmane.org/gmane.comp.emulators.libvirt/19043)

Thanks, Wolfgang

Comments

Gerd Hoffmann Dec. 16, 2009, 11:54 a.m. UTC | #1 
On 12/15/09 18:30, Wolfgang Mauerer wrote:
> Hi Gerd,
>
> in commit 5b684b5a56e81f6f, you introduced an explicit check
> to prevent adding SCSI controllers without attached disks
> to the system.

There was a patch from Daniel removing that check, isn't that one merged 
meanwhile?  Hmm, looks like it isn't.

Your patch does more than just killing the check, especially the chunk 
in lsi53c895a.c is clearly wrong.  I'd prefer Daniels patch being merged 
instead.

> Is there any other method to introduce
> disk-less controllers into the system?

device_add lsi,id=<hba-name>,addr=<slot.func>

adding disks then:

drive_add unused if=none,id=<disk-name>,file=...
device_add scsi-disk,drive=<disk-name>,bus=<hba-name>.0,scsi-id=<nr>

Note that this is the only way you can expect to work reliable with more 
than one scsi adapter being present in the system.

cheers,
   Gerd
Wolfgang Mauerer Dec. 16, 2009, 2:34 p.m. UTC | #2 
Gerd Hoffmann wrote:
> On 12/15/09 18:30, Wolfgang Mauerer wrote:
>> Hi Gerd,
>>
>> in commit 5b684b5a56e81f6f, you introduced an explicit check
>> to prevent adding SCSI controllers without attached disks
>> to the system.
> 
> There was a patch from Daniel removing that check, isn't that one merged 
> meanwhile?  Hmm, looks like it isn't.
> 
> Your patch does more than just killing the check, especially the chunk 
> in lsi53c895a.c is clearly wrong.  I'd prefer Daniels patch being merged 
> instead.

after sending the patch, Daniel told me that he had already sent one,
and I don't care which one you merge as long as I can add empty
contollers again ;-)
> 
>> Is there any other method to introduce
>> disk-less controllers into the system?
> 
> device_add lsi,id=<hba-name>,addr=<slot.func>
> 
> adding disks then:
> 
> drive_add unused if=none,id=<disk-name>,file=...
> device_add scsi-disk,drive=<disk-name>,bus=<hba-name>.0,scsi-id=<nr>
> 
> Note that this is the only way you can expect to work reliable with more 
> than one scsi adapter being present in the system.

Thanks!

Cheers, Wolfgang
diff mbox

Patch

commit c827742224b9a3a0d9dad0ce36c7e59c1a796ec1
Author: Wolfgang Mauerer <wolfgang.mauerer@siemens.com>
Date:   Tue Dec 15 18:06:19 2009 +0100

    Revert "hotplug: fix "pci_add storage if=scsi""
    
    This reverts commit 5b684b5a56e81f6f88234952fe8ed68010c36e19, and
    also includes some manual adaptions. There are some cases
    where addding an empty SCSI controller to a system makes
    sense, so we should not prohibit this option.
    
    Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com>

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 8b8a80b..eee206b 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -2103,9 +2103,7 @@  static int lsi_scsi_init(PCIDevice *dev)
     lsi_soft_reset(s);
 
     scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
-    if (!dev->qdev.hotplugged) {
-        scsi_bus_legacy_handle_cmdline(&s->bus);
-    }
+    scsi_bus_legacy_handle_cmdline(&s->bus);
     vmstate_register(-1, &vmstate_lsi_scsi, s);
     return 0;
 }
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 6e42776..075be31 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -183,14 +183,10 @@  static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
 
     switch (type) {
     case IF_SCSI:
-        if (!dinfo) {
-            monitor_printf(mon, "scsi requires a backing file/device.\n");
-            return NULL;
-        }
         dev = pci_create(bus, devfn, "lsi53c895a");
         if (qdev_init(&dev->qdev) < 0)
             dev = NULL;
-        if (dev) {
+        if (dev && dinfo) {
             if (scsi_hot_add(&dev->qdev, dinfo, 0) != 0) {
                 qdev_unplug(&dev->qdev);
                 dev = NULL;
@@ -204,12 +200,15 @@  static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
         }
         dev = pci_create(bus, devfn, "virtio-blk-pci");
         qdev_prop_set_drive(&dev->qdev, "drive", dinfo);
-        if (qdev_init(&dev->qdev) < 0)
-            dev = NULL;
         break;
     default:
         dev = NULL;
     }
+    if (!dev)
+        return NULL;
+    if (dinfo && qdev_init(&dev->qdev) < 0)
+        return NULL;
+
     return dev;
 }