Cole Robinson wrote:
On 11/20/2009 01:39 PM, Jim Fehlig wrote:
Daniel Veillard wrote:
On Mon, Nov 16, 2009 at 04:06:41PM -0700, Jim Fehlig wrote:
virDomain{Attach,Detach}Device is only permitted on active domains. Explicitly state this restriction in the API documentation.
Well, actually I'm not sure it's true. For exemple the XML xen driver has an implementation for inactive Xen domains, and if I look at the VirtualBox driver it seems to take care of domains which are not currently running (or paused).
So what do folks prefer? Allow the individual drivers to restrict attach/detach device or enforce restriction in the front-end? IMO, it should be delegated to the individual drivers, with a comment in the API description that some hypervisors may not support this operation on inactive domains. Why restrict a hypervisor's management functionality in the libvirt front-end?
Because it introduces hypervisor dependent API differences that aren't programmatically discoverable.
They are discoverable if you consider receiving a "not supported" error after making the call :-). But yes, it would be nice if apps knew these capabilities up front to enable/disable widgets and such. Perhaps libvirt will need a "hypervisor management service" capabilities element some day.
This makes life difficult for apps that want to support multiple hypervisors (like virt-manager, where we have already bumped up against this inconsistency between xen and qemu drivers). In this case, if xen allows offline device addition but qemu does not, virt-manager will have to use the safe subset and never attempt attach device for an offline VM, or hardcode HV differences into the app.
Yep, understood.
For that reason, I think we should either always allow offline AttachDevice or never allow it. And of the two, I highly favor the latter, since the alternative would force every driver to reimplement something that can easily be accomplished with DefineXML (unless the HV supports it natively, which in Xen's case sounds like it isn't handled consistently for all device types).
Maybe a way forward here is to clarify the virsh commands: we can alter attach/detach-device to manually edit the VM xml for inactive VMs (but also attempt hotplug if the VM is running), and introduce new commands hotplug-device and hotunplug-device which map straight through to the API. This way virsh won't regress, and indeed will improve: only straight API users will see a change in behavior.
Ok, IIUC you prefer to never allow offline {Attach,Detach}Device in the API. Do others concur? If so I can spin a patch that - Improves virDomain(Attach,Detach}Device documentation - Disallows these operations in libvirt front-end as DV suggested - Improves virsh app as Cole suggested Thanks, Jim