From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mqtms-0005Mz-RV for qemu-devel@nongnu.org; Thu, 24 Sep 2009 15:15:30 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mqtmo-0005L6-45 for qemu-devel@nongnu.org; Thu, 24 Sep 2009 15:15:30 -0400 Received: from [199.232.76.173] (port=35701 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mqtmn-0005L1-TN for qemu-devel@nongnu.org; Thu, 24 Sep 2009 15:15:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12788) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mqtmn-0002BP-8P for qemu-devel@nongnu.org; Thu, 24 Sep 2009 15:15:25 -0400 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n8OJFOpa021254 for ; Thu, 24 Sep 2009 15:15:24 -0400 Subject: Re: [Qemu-devel] [PATCH 10/13] Implement scsi device destruction References: <1253611767-6483-1-git-send-email-kraxel@redhat.com> <1253611767-6483-11-git-send-email-kraxel@redhat.com> From: Markus Armbruster Date: Thu, 24 Sep 2009 21:15:22 +0200 In-Reply-To: <1253611767-6483-11-git-send-email-kraxel@redhat.com> (Gerd Hoffmann's message of "Tue\, 22 Sep 2009 11\:29\:24 +0200") Message-ID: <8763b8b1dh.fsf@pike.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org Gerd Hoffmann writes: > Signed-off-by: Gerd Hoffmann > --- > hw/scsi-bus.c | 24 +++++++++++++++++++++--- > hw/scsi-disk.c | 6 ------ > hw/scsi-generic.c | 2 -- > 3 files changed, 21 insertions(+), 11 deletions(-) > > diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c > index 881e363..27defc4 100644 > --- a/hw/scsi-bus.c > +++ b/hw/scsi-bus.c > @@ -30,6 +30,7 @@ static int scsi_qdev_init(DeviceState *qdev, DeviceInfo *base) > SCSIDevice *dev = DO_UPCAST(SCSIDevice, qdev, qdev); > SCSIDeviceInfo *info = DO_UPCAST(SCSIDeviceInfo, qdev, base); > SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus); > + int rc = -1; > > if (dev->id == -1) { > for (dev->id = 0; dev->id < bus->ndev; dev->id++) { > @@ -43,21 +44,38 @@ static int scsi_qdev_init(DeviceState *qdev, DeviceInfo *base) > } > > if (bus->devs[dev->id]) { > - bus->devs[dev->id]->info->destroy(bus->devs[dev->id]); > + qdev_free(&bus->devs[dev->id]->qdev); > } If I understand this correctly, SCSI devices "overwrite": if you add a new one with an existing SCSI ID, the old one gets disconnected automatically. Isn't that inconsistent with other buses? PCI, specifically. Question applies before your patch already. > bus->devs[dev->id] = dev; > > dev->info = info; > - return dev->info->init(dev); > + rc = dev->info->init(dev); > + if (rc != 0) { > + bus->devs[dev->id] = NULL; > + } > > err: > - return -1; > + return rc; > +} > + > +static int scsi_qdev_exit(DeviceState *qdev) > +{ > + SCSIDevice *dev = DO_UPCAST(SCSIDevice, qdev, qdev); > + SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus); > + > + assert(bus->devs[dev->id] != NULL); > + if (bus->devs[dev->id]->info->destroy) { > + bus->devs[dev->id]->info->destroy(bus->devs[dev->id]); > + } > + bus->devs[dev->id] = NULL; > + return 0; > } You have scsi_qdev_exit() as qdev callback exit(). It does the generic stuff, then runs the SCSIDevice callback destroy() for the specific stuff. Shouldn't the two callbacks be named the same, to make their relation more obvious? > > void scsi_qdev_register(SCSIDeviceInfo *info) > { > info->qdev.bus_info = &scsi_bus_info; > info->qdev.init = scsi_qdev_init; > + info->qdev.exit = scsi_qdev_exit; > qdev_register(&info->qdev); > } > [...]