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

[Jim Fehlig]

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.

Thanks,
Jim