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

[Daniel P. Berrange]

On Thu, Oct 21, 2010 at 11:48:33AM -0500, Anthony Liguori wrote:
> 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.

We can syslog a warning anytime we try to run  drive_unplug and it is not
present.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|