Re: [Qemu-devel] [STABLE PATCH] hotplug: fix scsi hotplug.

[Dustin Kirkland <kirkland@canonical.com>]

On Wed, Oct 14, 2009 at 8:30 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Well, partly just papering over the issues.  But without proper scsi bus
> infrastructure we hardly can do better.  Changes:
>
>  * Avoid auto-attach by setting the bus number to -1.
>  * Ignore the unit value calculated by drive_init().
>  * Explicitly attach the devices to the adapter.
>  * Add sanity checks.  Don't allow attaching scsi drives to your
>   network device.
>  * Kill the bus+unit printing.  The values are bogus, and we can't
>   easily figure the correct ones.  I doubt this ever worked correctly
>   with multiple scsi adapters present in the system.
>
> Should come more close to the expected behavior now ...
>
> Oh, and pc-bios/bios.bin needs a update too, otherwise pci hotplug
> doesn't work at all.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/pci-hotplug.c |   24 +++++++++++++++++++-----
>  pc-bios/bios.bin |  Bin 131072 -> 131072 bytes
>  2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
> index d0f2911..8bedea2 100644
> --- a/hw/pci-hotplug.c
> +++ b/hw/pci-hotplug.c
> @@ -52,9 +52,10 @@ void drive_hot_add(Monitor *mon, const char *pci_addr, const char *opts)
>  {
>     int dom, pci_bus;
>     unsigned slot;
> -    int drive_idx, type, bus;
> +    int drive_idx, type;
>     int success = 0;
>     PCIDevice *dev;
> +    char buf[128];
>
>     if (pci_read_devaddr(mon, pci_addr, &dom, &pci_bus, &slot)) {
>         return;
> @@ -74,11 +75,19 @@ void drive_hot_add(Monitor *mon, const char *pci_addr, const char *opts)
>         return;
>     }
>     type = drives_table[drive_idx].type;
> -    bus = drive_get_max_bus (type);
>
>     switch (type) {
>     case IF_SCSI:
> +        if (!dev->qdev.info || strcmp(dev->qdev.info->name, "lsi53c895a") != 0) {
> +            monitor_printf(mon, "Device is not a scsi adapter\n");
> +            break;
> +        }
>         success = 1;
> +        drives_table[drive_idx].bus = -1;
> +        drives_table[drive_idx].unit = -1;
> +        if (get_param_value(buf, sizeof(buf), "unit", opts)) {
> +            drives_table[drive_idx].unit = atoi(buf);
> +        }
>         lsi_scsi_attach(&dev->qdev, drives_table[drive_idx].bdrv,
>                         drives_table[drive_idx].unit);
>         break;
> @@ -87,9 +96,7 @@ void drive_hot_add(Monitor *mon, const char *pci_addr, const char *opts)
>     }
>
>     if (success)
> -        monitor_printf(mon, "OK bus %d, unit %d\n",
> -                       drives_table[drive_idx].bus,
> -                       drives_table[drive_idx].unit);
> +        monitor_printf(mon, "OK\n");
>     return;
>  }
>
> @@ -130,7 +137,14 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
>
>     switch (type) {
>     case IF_SCSI:
> +        drives_table[drive_idx].bus = -1;
> +        drives_table[drive_idx].unit = -1;
> +        if (get_param_value(buf, sizeof(buf), "unit", opts)) {
> +            drives_table[drive_idx].unit = atoi(buf);
> +        }
>         dev = pci_create("lsi53c895a", devaddr);
> +        lsi_scsi_attach(&dev->qdev, drives_table[drive_idx].bdrv,
> +                        drives_table[drive_idx].unit);
>         break;
>     case IF_VIRTIO:
>         dev = pci_create("virtio-blk-pci", devaddr);

Thanks, Gerd.

I applied this patch against qemu-kvm-0.11.0 stable, built, and tested
it.  I can verify that it fixes the scsi hot-add issues I was seeing.
I am now able to add/remove/add/remove/add/remove a scsi disk to a
running instance without segfaulting qemu.

Note that on remove, I do get a stack track in the guest's kernel
(2.6.31), though the remove does succeed, and the disk disappears.

Also note that I did not replace the bios.bin, as it appears to me
that the qemu-kvm-0.11 bios.bin is working properly.

Tested-by: Dustin Kirkland <kirkland@canonical.com>