On Wed, 12 Feb 2014, Markus Armbruster wrote:
> Markus Armbruster <arm...@redhat.com> writes:
>
> > Stefano Stabellini <stefano.stabell...@eu.citrix.com> writes:
> >
> >> On Thu, 6 Feb 2014, Markus Armbruster wrote:
> >>> Stefano Stabellini <stefano.stabell...@eu.citrix.com> writes:
> >>>
> >>> > On Wed, 5 Feb 2014, Markus Armbruster wrote:
> >>> >> [Note cc: Stefano]
> >>> >>
> >>> >> Kevin Wolf <kw...@redhat.com> 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 <arm...@redhat.com>
> >>> >> >
> >>> >> > 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.
>
> Stefano, I updated the branch, please retest it.
QEMU exists with:
hw/ide/piix.c:183:pci_piix3_xen_ide_unplug: Object 0x7f172045b050 is not an
instance of type ide-device
