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

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



On Fri, Jan 22, 2010 at 04:21:09PM -0700, Jim Fehlig wrote:
> Daniel P. Berrange wrote:
> > 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.
> 
> Ah, yes - good catch.  Thanks.
> 
> >>  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.
> >   
> 
> Do you prefer the latter option?  After a quick look, I didn't spot any
> existing compatibility code in the remote driver client.  The first
> option might be slightly better wrt maintenance.

Yes, the first option is certainly simpler / clearer code todo, so lets
just do that first. We can easily revisit adding compat code in the
remote driver client at a later date if we find it to be neccessary.

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]