* Re: [Qemu-devel] [STABLE PATCH] hotplug: fix scsi hotplug.
[not found] <1255527031-19008-1-git-send-email-kraxel@redhat.com>
@ 2009-10-14 17:30 ` Dustin Kirkland
2009-10-15 7:42 ` Gerd Hoffmann
0 siblings, 1 reply; 2+ messages in thread
From: Dustin Kirkland @ 2009-10-14 17:30 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, kvm
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>
^ permalink raw reply [flat|nested] 2+ messages in thread