[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]