On Tue, Dec 08, 2009 at 12:01:48PM -0700, Jim Fehlig wrote:
History
I found that virDomainDetachDevice did not work with inactive domains
and submitted a patch [1] to fix this. As it turns out, some drivers do
not support attaching/detaching devices on inactive domains and it was
preferred that virDomain{Attach,Detach}Device only be permitted on
active domains.
I posted a follow-up patch [2] that restricted
virDomain{Attach,Detach}Device to active domains only, although as
Daniel Veillard pointed out this restriction should have been enforced
in the libvirt front-end. Cole Robinson suggested improving virsh [3]
to handle the case of inactive vs. active by redefining the domain with
new device (or removed device) when domain is inactive. While
implementing this improvement I asked some questions on IRC that
resulted in another approach suggested by Dan Berrange as outlined below.
Introduce two new APIs
virDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned
flags)
virDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, unsigned
flags)
with flags being one or more from VIR_DOMAIN_DEVICE_ATTACH_CURRENT,
VIR_DOMAIN_DEVICE_ATTACH_LIVE, VIR_DOMAIN_DEVICE_ATTACH_CONFIG.
(I'm not sure about the ATTACH in these names. Perhaps CHANGE or MODIFY
would be more appropriate.)
If caller specifies CURRENT (default), add or remove the device
depending on the current state of domain. E.g. if domain is active add
or remove the device to/from live domain, if it is inactive change the
persistent config. If caller specifies LIVE, only change the active
domain. If caller specifies CONFIG, only change persistent config -
even if the domain is active. If caller specifies both LIVE and CONFIG,
then change both.
If a driver could not satisfy the exact requested flags it must return
an error. E.g if user specified LIVE but the driver can only change
live and persisted config, the driver must fail the request.
The existing virDomain{Attach,Detach}Device would be declared to be
equivalent to virDomain{Attach,Detach}DeviceFlags(LIVE).
Finally, virsh {attach,detach}-{disk,interface,device} would be modified
to add a --persistent flag in order to set the appropriate flags when
calling the new APIs.
This plan obviously gets my vote :-) I very much regret that we didn't
have a 'unsigned flags' parameter on this attach/detach APIs already.
This change will make it much easier for apps to guarantee they are
getting the exact semantics they need/want.
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|