From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54381) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SpcTY-0003gO-5g for Qemu-devel@nongnu.org; Fri, 13 Jul 2012 05:47:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SpcTW-0007bl-C8 for Qemu-devel@nongnu.org; Fri, 13 Jul 2012 05:47:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9008) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SpcTW-0007bY-47 for Qemu-devel@nongnu.org; Fri, 13 Jul 2012 05:47:50 -0400 Message-ID: <4FFFEEC0.1070808@redhat.com> Date: Fri, 13 Jul 2012 11:47:44 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1342102838-17442-1-git-send-email-kwolf@redhat.com> <87629su9xd.fsf@codemonkey.ws> In-Reply-To: <87629su9xd.fsf@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC] Fix SCSI hotplug with invalid slot List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: afaerber@suse.de, Qemu-devel@nongnu.org [ Whoops, forgot qemu-devel in my original mail, added now ] Am 12.07.2012 20:11, schrieb Anthony Liguori: > Kevin Wolf writes: > >> While trying to hotplug an if=scsi disk with drive_add, I didn't pay >> attention to using a valid slot (it doesn't matter with the usual >> if=none...), and so I got: >> >> (qemu) drive_add 0 file=/tmp/test.qcow2,if=scsi >> Segmentation fault. >> >> qemu just takes the PCI device at slot 0 and starts working on its first >> child bus, no matter what device it is, and whether it even has a bus. >> This NULL pointer access is easy enough to fix, it's what this patch >> does. >> >> However this leaves a second case where the device in the slot does >> exist, has child buses and still isn't a SCSI bus. For example (here >> it's IDE): >> >> (qemu) drive_add 1 file=/tmp/test.qcow2,if=scsi >> Object 0x7fadb204bbf0 is not an instance of type SCSI >> Aborted. >> >> I couldn't find any obvious solution for checking if it has the right >> type without aborting. I'm sure that this is a pretty standard case, but >> my QOM knowledge is lacking... >> >> Signed-off-by: Kevin Wolf >> --- >> hw/pci-hotplug.c | 8 +++++++- >> 1 files changed, 7 insertions(+), 1 deletions(-) >> >> diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c >> index e7fb780..87d4721 100644 >> --- a/hw/pci-hotplug.c >> +++ b/hw/pci-hotplug.c >> @@ -77,10 +77,16 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, >> static int scsi_hot_add(Monitor *mon, DeviceState *adapter, >> DriveInfo *dinfo, int printinfo) >> { >> + BusState *bus; >> SCSIBus *scsibus; >> SCSIDevice *scsidev; >> >> - scsibus = SCSI_BUS(QLIST_FIRST(&adapter->child_bus)); >> + bus = QLIST_FIRST(&adapter->child_bus); >> + if (bus == NULL) { > > if (bus == NULL || object_dynamic_cast(bus, TYPE_SCSI_BUS) == NULL) { Ah well, so this is really the official way... Then I could as well use the result of that instead of calling it a second time in SCSI_BUS(). Should I send an updated patch for this, or is it a more general problem of the QOM conversions that checks were lost and you'll post a broader fix? Kevin