[libvirt] [PATCH 1/2] util: Functions for getting/setting device options

[Tony Krowiak]

On 01/21/2015 01:34 PM, Laine Stump wrote:
> On 01/19/2015 11:18 AM, akrowiak at linux.vnet.ibm.com wrote:
>> From: Tony Krowiak <akrowiak at linux.vnet.ibm.com>
>>
>> This patch provides the utility functions needed to synchronize
>> the rxfilter changes made to a guest domain with the corresponding
>> macvtap devices on the host:
>>
>> * Get/set PROMISC flag
>> * Get/set ALLMULTI, MULTICAST
>>
>> Signed-off-by: Tony Krowiak <akrowiak at linux.vnet.ibm.com>
>> ---
>>   src/libvirt_private.syms |    6 +
>>   src/util/virnetdev.c     |  346 ++++++++++++++++++++++++++++++++++++++++++++++
>>   src/util/virnetdev.h     |   14 ++
>>   3 files changed, 366 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index a2eec83..6b49b08 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -1646,6 +1646,9 @@ virNetDevGetVirtualFunctionInfo;
>>   virNetDevGetVirtualFunctions;
>>   virNetDevGetVLanID;
>>   virNetDevIsOnline;
>> +virNetDevIsPromiscuous;
>> +virNetDevIsRcvAllMulti;
>> +virNetDevIsRcvMulti;
>>   virNetDevIsVirtualFunction;
>>   virNetDevLinkDump;
>>   virNetDevReplaceMacAddress;
>> @@ -1663,6 +1666,9 @@ virNetDevSetMTUFromDevice;
>>   virNetDevSetName;
>>   virNetDevSetNamespace;
>>   virNetDevSetOnline;
>> +virNetDevSetPromiscuous;
>> +virNetDevSetRcvAllMulti;
>> +virNetDevSetRcvMulti;
>>   virNetDevSetupControl;
>>   virNetDevValidateConfig;
>>   
>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
>> index ef96b2b..c610f5b 100644
>> --- a/src/util/virnetdev.c
>> +++ b/src/util/virnetdev.c
>> @@ -2337,6 +2337,333 @@ int virNetDevDelMulti(const char *ifname ATTRIBUTE_UNUSED,
>>   }
>>   #endif
> There's a lot of duplicated code here. Rather than completely
> reimplementing the contents of virNetDev(Set|Is)Online for each of
> these, how about making two static helper functions:
>
> int virNetDevGetIFFlag(const char *ifname, int flag, bool *val)
> int virNetDevSetIFFlag(const char *ifname, int flag, bool val)
>
> then the exported function for each flag would simply be, eg:
>
> int
> virNetDevGetPromiscuous(const char *ifname, bool *promiscuous)
> {
>      return virNetDevGetIFFlag(ifname, IFF_PROMISC, promiscuous);
> }
>
> int
> virNetDevSetPromiscuous(const char *ifname, bool promiscuous)
> {
>      return virNetDevSetIFFlag(ifname, IFF_PROMISC, promiscuous);
> }
>
> (also reimplement virNetDev(Set|Is)Online based on the helper functions)
>
> Because IFF_blah may not exist on all platforms, you'll need to provide
> the stub implementation of the toplevel functions, rather than stubs for
> virNetDev(Get|Set)IFFlag(), but it's still much less code.
>
> Oh, and I would define *all* of these SIOC[SG]IFFLAGS-using functions
> inside a single #if, rather than a separate one for each.
>
> Finally, although the existing function for "Online" uses "Is" as the
> verb in its name, I think it is more proper to use "Get", so all the
> functions you named "virNetDevIsBlah" should be named "virNetDevGetBlah"
> instead (rename virNetDevIsOnline while you're at it - don't forget to
> change the name in the symfile).
Sounds like a plan.
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>