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

Re: [libvirt] [RFC] virDomain{Attach,Detach}Device



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


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