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

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 :|