[libvirt] [PATCH] qemu: call drive_unplug in DetachPciDiskDevice

[Anthony Liguori]

On 10/21/2010 11:45 AM, Daniel P. Berrange wrote:
> On Thu, Oct 21, 2010 at 08:50:35AM -0500, Ryan Harper wrote:
>    
>> Currently libvirt doesn't confirm whether the guest has responded to the
>> disk removal request.  In some cases this can leave the guest with
>> continued access to the device while the mgmt layer believes that it has
>> been removed.  With a recent qemu monitor command[1] we can
>> deterministically revoke a guests access to the disk (on the QEMU side)
>> to ensure no futher access is permitted.
>>
>> This patch adds support for the drive_unplug() command and introduces it
>> in the disk removal paths.  There is some discussion to be had about how
>> to handle the case where the guest is running in a QEMU without this
>> command (and the fact that we currently don't have a way of detecting
>> what monitor commands are available).
>>      
> Basically we try to run the command and then catch the failure.
>
> For QMP, we can check for a error with a class of 'CommandNotFound',
>
> For HMP, QEMU will print 'unknown command' in the reply.
>
> Neither is ideal, since neither is a guarenteed part of the monitor
> interface, but it is all we have to go on, and ensure other critical
> errors will still be treated as fatal by libvirt.
>
>    
>> My current implementation assumes that if you don't have a QEMU with
>> this capability that we should fail the device removal.  This is a
>> strong statement around hotplug that isn't consistent with previous
>> releases so I'm open to other approachs, but given the potential data
>> leakage problem hot-remove can lead to without drive_unplug, I think
>> it's the right thing to do.
>>      
> I don't think we can do this, since it obviously breaks every single
> existing deployment out there. Users who have sVirt enabled will
> have a level of protection from the data leakage, so I don't think
> it is a severe problem.
>    

I think that's reasonable but if sVirt is disabled, libvirt at least 
should log something somewhere to indicate that something may be wrong.

Regards,

Anthony Liguori