On Mon, 18 Jul 2011, Alexander Graf wrote:
> On 15.07.2011, at 12:52, Kevin Wolf wrote:
>
> > Am 15.07.2011 12:34, schrieb Stefano Stabellini:
> >> On Fri, 1 Jul 2011, Stefano Stabellini wrote:
> >>> On Fri, 1 Jul 2011, Kevin Wolf wrote:
> >>>> Am 30.06.2011 16:16, schrieb Stefano Stabellini:
> >>>>> On Thu, 30 Jun 2011, Kevin Wolf wrote:
> >>>>>>> +static int pci_piix3_xen_ide_unplug(DeviceState *dev)
> >>>>>>> +{
> >>>>>>> + PCIDevice *pci_dev;
> >>>>>>> + PCIIDEState *pci_ide;
> >>>>>>> + DriveInfo *di;
> >>>>>>> + int i = 0;
> >>>>>>> +
> >>>>>>> + pci_dev = DO_UPCAST(PCIDevice, qdev, dev);
> >>>>>>> + pci_ide = DO_UPCAST(PCIIDEState, dev, pci_dev);
> >>>>>>> +
> >>>>>>> + for (; i < 3; i++) {
> >>>>>>> + di = drive_get_by_index(IF_IDE, i);
> >>>>>>> + if (di != NULL && di->bdrv != NULL && !di->bdrv->removable) {
> >>>>>>> + DeviceState *ds = bdrv_get_attached(di->bdrv);
> >>>>>>> + if (ds) {
> >>>>>>> + bdrv_detach(di->bdrv, ds);
> >>>>>>> + }
> >>>>>>> + bdrv_close(di->bdrv);
> >>>>>>> + pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
> >>>>>>
> >>>>>> Have you tested if this is enough if the guest tries to continue using
> >>>>>> the device? I don't know of any case where it's not sufficient, just
> >>>>>> trying to make sure that it's really true in practice.
> >>>>>
> >>>>> The purpose of this is to "hide" the disk from the guest. The unplug is
> >>>>> supposed to happen *before* the guest enumerates the IDE disks; it is
> >>>>> responsibility of the guest to make sure of it.
> >>>>> I tested it with Linux PV on HVM drivers, and Linux doesn't see the
> >>>>> emulated disk after the unplug, as it should be.
> >>>>
> >>>> Yeah. What I meant is that we should make sure that a misbehaving guest,
> >>>> which just keeps on playing with the IDE ports anyway, can't crash qemu.
> >>>> A quick review suggests that it is the case, but testing it anyway would
> >>>> be better.
> >>>
> >>> I see what you mean: I tested it, a guest cannot crash Qemu.
> >>>
> >>
> >> ping?
> >
> > I thought Alex had already merged it. I'm pretty sure that I stated
> > somewhere that the patch is okay for me now. In case I didn't:
> >
> > Acked-by: Kevin Wolf <kw...@redhat.com>
>
> Ah, must have missed it :). Thanks.
>
> Stefano, could you please rebase? The patch doesn't apply cleanly anymore.
OK, I'll send it again based on your latest xen-next branch.
