Re: [Qemu-devel] [PATCH v3] xen: implement unplug protocol in xen_platform

[Alexander Graf]

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 <address@hidden>

Ah, must have missed it :). Thanks.

Stefano, could you please rebase? The patch doesn't apply cleanly anymore.


Alex