From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1My7gE-0008Ng-MK for qemu-devel@nongnu.org; Wed, 14 Oct 2009 13:30:30 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1My7gA-0008N5-Bj for qemu-devel@nongnu.org; Wed, 14 Oct 2009 13:30:30 -0400 Received: from [199.232.76.173] (port=59534 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1My7gA-0008N2-8A for qemu-devel@nongnu.org; Wed, 14 Oct 2009 13:30:26 -0400 Received: from mail-fx0-f214.google.com ([209.85.220.214]:63691) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1My7g9-0007ZX-Q8 for qemu-devel@nongnu.org; Wed, 14 Oct 2009 13:30:26 -0400 Received: by fxm10 with SMTP id 10so4859fxm.8 for ; Wed, 14 Oct 2009 10:30:24 -0700 (PDT) MIME-Version: 1.0 Sender: dustin.kirkland@gmail.com In-Reply-To: <1255527031-19008-1-git-send-email-kraxel@redhat.com> References: <1255527031-19008-1-git-send-email-kraxel@redhat.com> Date: Wed, 14 Oct 2009 12:30:24 -0500 Message-ID: Subject: Re: [Qemu-devel] [STABLE PATCH] hotplug: fix scsi hotplug. From: Dustin Kirkland Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org On Wed, Oct 14, 2009 at 8:30 AM, Gerd Hoffmann wrote: > Well, partly just papering over the issues. =A0But without proper scsi bu= s > infrastructure we hardly can do better. =A0Changes: > > =A0* Avoid auto-attach by setting the bus number to -1. > =A0* Ignore the unit value calculated by drive_init(). > =A0* Explicitly attach the devices to the adapter. > =A0* Add sanity checks. =A0Don't allow attaching scsi drives to your > =A0 network device. > =A0* Kill the bus+unit printing. =A0The values are bogus, and we can't > =A0 easily figure the correct ones. =A0I doubt this ever worked correctly > =A0 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 > --- > =A0hw/pci-hotplug.c | =A0 24 +++++++++++++++++++----- > =A0pc-bios/bios.bin | =A0Bin 131072 -> 131072 bytes > =A02 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) > =A0{ > =A0 =A0 int dom, pci_bus; > =A0 =A0 unsigned slot; > - =A0 =A0int drive_idx, type, bus; > + =A0 =A0int drive_idx, type; > =A0 =A0 int success =3D 0; > =A0 =A0 PCIDevice *dev; > + =A0 =A0char buf[128]; > > =A0 =A0 if (pci_read_devaddr(mon, pci_addr, &dom, &pci_bus, &slot)) { > =A0 =A0 =A0 =A0 return; > @@ -74,11 +75,19 @@ void drive_hot_add(Monitor *mon, const char *pci_addr= , const char *opts) > =A0 =A0 =A0 =A0 return; > =A0 =A0 } > =A0 =A0 type =3D drives_table[drive_idx].type; > - =A0 =A0bus =3D drive_get_max_bus (type); > > =A0 =A0 switch (type) { > =A0 =A0 case IF_SCSI: > + =A0 =A0 =A0 =A0if (!dev->qdev.info || strcmp(dev->qdev.info->name, "lsi= 53c895a") !=3D 0) { > + =A0 =A0 =A0 =A0 =A0 =A0monitor_printf(mon, "Device is not a scsi adapte= r\n"); > + =A0 =A0 =A0 =A0 =A0 =A0break; > + =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 success =3D 1; > + =A0 =A0 =A0 =A0drives_table[drive_idx].bus =3D -1; > + =A0 =A0 =A0 =A0drives_table[drive_idx].unit =3D -1; > + =A0 =A0 =A0 =A0if (get_param_value(buf, sizeof(buf), "unit", opts)) { > + =A0 =A0 =A0 =A0 =A0 =A0drives_table[drive_idx].unit =3D atoi(buf); > + =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 lsi_scsi_attach(&dev->qdev, drives_table[drive_idx].bdrv, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 drives_table[drive_idx].u= nit); > =A0 =A0 =A0 =A0 break; > @@ -87,9 +96,7 @@ void drive_hot_add(Monitor *mon, const char *pci_addr, = const char *opts) > =A0 =A0 } > > =A0 =A0 if (success) > - =A0 =A0 =A0 =A0monitor_printf(mon, "OK bus %d, unit %d\n", > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 drives_table[drive_idx].bus= , > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 drives_table[drive_idx].uni= t); > + =A0 =A0 =A0 =A0monitor_printf(mon, "OK\n"); > =A0 =A0 return; > =A0} > > @@ -130,7 +137,14 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *= mon, > > =A0 =A0 switch (type) { > =A0 =A0 case IF_SCSI: > + =A0 =A0 =A0 =A0drives_table[drive_idx].bus =3D -1; > + =A0 =A0 =A0 =A0drives_table[drive_idx].unit =3D -1; > + =A0 =A0 =A0 =A0if (get_param_value(buf, sizeof(buf), "unit", opts)) { > + =A0 =A0 =A0 =A0 =A0 =A0drives_table[drive_idx].unit =3D atoi(buf); > + =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 dev =3D pci_create("lsi53c895a", devaddr); > + =A0 =A0 =A0 =A0lsi_scsi_attach(&dev->qdev, drives_table[drive_idx].bdrv= , > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0drives_table[drive_idx].= unit); > =A0 =A0 =A0 =A0 break; > =A0 =A0 case IF_VIRTIO: > =A0 =A0 =A0 =A0 dev =3D 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