[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs |
Date: |
Wed, 5 Feb 2014 17:09:00 +0000 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
On Wed, 5 Feb 2014, Markus Armbruster wrote:
> [Note cc: Stefano]
>
> Kevin Wolf <address@hidden> 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 <address@hidden>
> >
> > 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.
- Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs, Kevin Wolf, 2014/02/03
- Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs, Markus Armbruster, 2014/02/05
- Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs,
Stefano Stabellini <=
- Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs, Markus Armbruster, 2014/02/06
- Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs, Stefano Stabellini, 2014/02/07
- Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs, Markus Armbruster, 2014/02/11
- Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs, Markus Armbruster, 2014/02/12
- Re: [Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs, Stefano Stabellini, 2014/02/12