Re: [libvirt] [PATCH 2/5] First level of plumbing for virInterface*.

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

 



On Mon, May 11, 2009 at 03:29:42PM -0400, Laine Stump wrote:
> On 05/11/2009 06:35 AM, Daniel P. Berrange wrote:
>> On Fri, May 08, 2009 at 01:52:24PM -0400, Laine Stump wrote:
>>   
>>> From: Laine Stump <laine@xxxxxxxxxx>
>>>
>>> ---
>>>  include/libvirt/libvirt.h    |   18 ++
>>>  include/libvirt/libvirt.h.in |   18 ++
>>>  include/libvirt/virterror.h  |    4 +
>>>  src/datatypes.h              |   25 ++
>>>  src/driver.h                 |   60 ++++
>>>  src/libvirt.c                |  695 ++++++++++++++++++++++++++++++++++++++++++
>>>  src/util.h                   |    2 -
>>>  src/virterror.c              |   21 ++
>>>  8 files changed, 841 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
>>> index 91af6fd..b0d93a2 100644
>>> --- a/include/libvirt/libvirt.h
>>> +++ b/include/libvirt/libvirt.h
>>> @@ -433,6 +433,24 @@ extern virConnectAuthPtr virConnectAuthPtrDefault;
>>>   #define VIR_UUID_STRING_BUFLEN (36+1)
>>>  +/**
>>> + * VIR_MAC_BUFLEN:
>>> + *
>>> + * This macro provides the length of the buffer required
>>> + * for an interface MAC address
>>> + */
>>> +
>>> +#define VIR_MAC_BUFLEN (6)
>>> +
>>> +/**
>>> + * VIR_MAC_STRING_BUFLEN:
>>> + *
>>> + * This macro provides the length of the buffer required
>>> + * for virInterfaceGetMACString()
>>> + */
>>> +
>>> +#define VIR_MAC_STRING_BUFLEN (VIR_MAC_BUFLEN * 3)
>>> +
>>>     
>>
>> Since this is now in our public API, I'm wondering whether we shouldn't
>> make the buflen longer. 6 bytes is fine with wired ethernet, but I'm
>> not sure its sufficient for all network device types  in general.
>>
>> eg, my wmaster0 device has a rather longer hardware address
>>
>> wmaster0  Link encap:UNSPEC  HWaddr 
>> 00-13-02-B9-F9-D3-F4-1F-00-00-00-00-00-00-00-00            UP BROADCAST 
>> RUNNING MULTICAST  MTU:1500  Metric:1
>>           RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>>           TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>>           collisions:0 txqueuelen:1000           RX bytes:0 (0.0 b)  TX 
>> bytes:0 (0.0 b)
>>   
> Yes, tun interfaces too. Since this is binary data rather than a  
> null-terminated string,
> we need to decide among the following three choices:
>
> 1) have a fixed length (how long? is 16 bytes long enough?) and  
> zero-fill the shorter ones.
>
> 2) Add a macLen arg to any API function that uses mac address (this will  
> need to be a return arg in some cases too)
>
> 3) Only provide the versions of the functions that accept/use ASCII mac  
> address args.

  IMHO, I would play safe at this point and pick 3)
First it's sufficient, from the ASCII version people can usually derive
the binary one if they really need it, but mostly I think people asked
for those interfaces at the libvirt level because they wanted the
ability to not mess with the low level stuff, so we should focuse on
the high level. And if this proves unsufficient we can stil add new APIs
based on the people feedback.

> Also, this has ramifications on other code outside virInterface* using  
> VIR_MAC_BUFLEN, so there will probably need to be a patch separate from  
> this series that grows VIR_MAC_BUFLEN and fixes that other code 
> accordingly
>
> Any opinion on how to proceed?

  I would go for 3) but keep the 2 macros internally either in the
internals.h file or in a network header and try to make sure we use
them everywhere :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]