Re: [libvirt] [PATCH 4/9] Public API Implementation

On Tue, Jan 19, 2010 at 07:40:14PM +0000, Daniel P. Berrange wrote: > On Thu, Jan 14, 2010 at 10:42:41AM -0700, Jim Fehlig wrote: > >> Implementation of public API for virDomain{Attach,Detach}DeviceFlags. >> --- >> src/libvirt.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++---- >> 1 files changed, 98 insertions(+), 8 deletions(-) >> >> diff --git a/src/libvirt.c b/src/libvirt.c >> index 1145561..77f76bc 100644 >> --- a/src/libvirt.c >> +++ b/src/libvirt.c >> @@ -5129,7 +5129,6 @@ error: >> int >> virDomainAttachDevice(virDomainPtr domain, const char *xml) >> { >> - virConnectPtr conn; >> DEBUG("domain=%p, xml=%s", domain, xml); >> >> virResetLastError(); >> @@ -5147,17 +5146,63 @@ virDomainAttachDevice(virDomainPtr domain, const char *xml) >> virLibDomainError(domain, VIR_ERR_OPERATION_INVALID, __FUNCTION__); >> goto error; >> } >> + >> + return virDomainAttachDeviceFlags(domain, xml, >> + VIR_DOMAIN_DEVICE_MODIFY_LIVE); >> + >> +error: >> + virDispatchError(domain->conn); >> + return -1; >> +} >> > This looks safe, but there's a subtle problem with changing the existing > virDomainAttachDevice() entry point to call virDomainAttachDeviceFlags(). > It will break compatability with old libvirtd, even though the old > libvirtd supports the virDomainAttachDevice() code.

> So we need to keep > the distinct paths in the public API & driver definitions. The eventual > low level hypervisor drivers can of course just turn their existing > impl into a thin wrapper to the new method.. > There's one other option actually - we could put compatability code in the remote driver client instead. Either, make it always invoke the old RPC call if flags == VIR_DOMAIN_DEVICE_MODIFY_LIVE, or have it invoke the new RPC call & fallback to the old RPC call if it gets an error indicating the new one doesn't exist.