From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35079) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDEWV-0007S9-4c for qemu-devel@nongnu.org; Tue, 11 Feb 2014 09:41:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WDEWQ-0003f6-JU for qemu-devel@nongnu.org; Tue, 11 Feb 2014 09:41:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39094) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDEWQ-0003ey-Bi for qemu-devel@nongnu.org; Tue, 11 Feb 2014 09:41:14 -0500 From: Markus Armbruster References: <1391084198-21021-1-git-send-email-armbru@redhat.com> <1391084198-21021-5-git-send-email-armbru@redhat.com> <20140203145406.GN3643@dhcp-200-207.str.redhat.com> <878utp8xh1.fsf@blackfin.pond.sub.org> <87wqh8wwpj.fsf@blackfin.pond.sub.org> Date: Tue, 11 Feb 2014 15:41:08 +0100 In-Reply-To: (Stefano Stabellini's message of "Fri, 7 Feb 2014 12:58:43 +0000") Message-ID: <8738jpaf0b.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: Kevin Wolf , afaerber@suse.de, qemu-devel@nongnu.org, stefanha@redhat.com, agraf@suse.de Stefano Stabellini writes: > On Thu, 6 Feb 2014, Markus Armbruster wrote: >> Stefano Stabellini writes: >> >> > On Wed, 5 Feb 2014, Markus Armbruster wrote: >> >> [Note cc: Stefano] >> >> >> >> Kevin Wolf writes: >> >> >> >> > Am 30.01.2014 um 13:16 hat Markus Armbruster geschrieben: >> >> >> It's a copy of dev->conf.bs. The copy was needed for non-qdevified >> >> >> controllers, which lacked dev. >> >> >> >> >> >> Note how pci_piix3_xen_ide_unplug() cleared the copy. We'll get back >> >> >> to that in the next few commits. >> >> >> >> >> >> Signed-off-by: Markus Armbruster >> >> > >> >> > So this pci_piix3_xen_ide_unplug() is what happens here: >> >> > >> >> >> --- a/hw/ide/piix.c >> >> >> +++ b/hw/ide/piix.c >> >> >> @@ -169,12 +169,9 @@ static int pci_piix_ide_initfn(PCIDevice *dev) >> >> >> >> >> >> static int pci_piix3_xen_ide_unplug(DeviceState *dev) >> >> >> { >> >> >> - PCIIDEState *pci_ide; >> >> >> DriveInfo *di; >> >> >> int i = 0; >> >> >> >> >> >> - pci_ide = PCI_IDE(dev); >> >> >> - >> >> >> for (; i < 3; i++) { >> >> >> di = drive_get_by_index(IF_IDE, i); >> >> >> if (di != NULL && !di->media_cd) { >> >> >> @@ -183,7 +180,6 @@ static int pci_piix3_xen_ide_unplug(DeviceState *dev) >> >> >> bdrv_detach_dev(di->bdrv, ds); >> >> >> } >> >> >> bdrv_close(di->bdrv); >> >> >> - pci_ide->bus[di->bus].ifs[di->unit].bs = NULL; >> >> >> drive_put_ref(di); >> >> >> } >> >> >> } >> >> > >> >> > Probably I'm just missing the obvious, but it seems to me that the >> >> > copy was cleared here, while the original was left around. This was >> >> > no problem because the original was unused anyway after device >> >> > initialisation. >> >> > >> >> > Now that the copy doesn't exist any more, we can't clear it, obviously, >> >> > but why don't we have to clear the original? Won't we still run the >> >> > "device is attached" code branches even though the device is really >> >> > unplugged? >> >> >> >> It's been a while since I wrote this. Almost 14 months, in fact. >> >> >> >> No other IDE controller implements DeviceClass method unplug(). I can't >> >> remember why the normal code to detach the backend (release_drive()) >> >> doesn't do here. Stefano, can you help? >> > >> > Too long to be able to remember the exact details :-/ >> > However if you point me to a branch I can give it a try and tell you if >> > the unplug still works as it used to. >> >> Series trivially rebased to current master available at >> git://repo.or.cz/qemu/armbru.git branch kill-non-qdev-ide > > Unfortunately it doesn't work: I can see both sda and xvda being present > after the system has booted. Thank you for testing. I'll try to clear the originals like Kevin suggested. Hopefully, that'll pass your test.