[libvirt] [PATCH 1/2] util: Introduce flags field for macvtap creation

[Matthew Rosato]

On 07/23/2014 08:56 AM, Martin Kletzander wrote:
> On Tue, Jul 01, 2014 at 02:00:56PM -0400, Matthew Rosato wrote:
>> Currently, there is one flag passed in during macvtap creation
>> (withTap) -- Let's convert this field to an unsigned int flag
>> field for future expansion.
>>
>> Signed-off-by: Matthew Rosato <mjrosato at linux.vnet.ibm.com>
>> ---
>> src/lxc/lxc_process.c       |    2 +-
>> src/qemu/qemu_command.c     |    3 ++-
>> src/util/virnetdevmacvlan.c |   24 +++++++++++++++---------
>> src/util/virnetdevmacvlan.h |    8 +++++++-
>> 4 files changed, 25 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
>> index cb85b74..bfbecbf 100644
>> --- a/src/util/virnetdevmacvlan.c
>> +++ b/src/util/virnetdevmacvlan.c
>> @@ -790,6 +790,7 @@ virNetDevMacVLanVPortProfileRegisterCallback(const
>> char *ifname,
>>  * @macaddress: The MAC address for the macvtap device
>>  * @linkdev: The interface name of the NIC to connect to the external
>> bridge
>>  * @mode: int describing the mode for 'bridge', 'vepa', 'private' or
>> 'passthru'.
>> + * @flags: OR of virNetDevMacVLanCreateFlags.
>>  * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it
>>  * @vmuuid: The UUID of the VM the macvtap belongs to
>>  * @virtPortProfile: pointer to object holding the virtual port
>> profile data
>> @@ -797,14 +798,15 @@
>> virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
>>  *     interface will be stored into if everything succeeded. It is up
>>  *     to the caller to free the string.
>>  *
>> - * Returns file descriptor of the tap device in case of success with
>> @withTap,
>> - * otherwise returns 0; returns -1 on error.
>> + * Returns file descriptor of the tap device in case of success with
>> + * flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP, otherwise returns 0;
>> returns
> 
> s/flags/@flags/

Thanks.

> 
>> + * -1 on error.
>>  */
>> int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
>>                                            const virMacAddr *macaddress,
>>                                            const char *linkdev,
>>                                            virNetDevMacVLanMode mode,
>> -                                           bool withTap,
>> +                                           unsigned int flags,
>>                                            int vnet_hdr,
>>                                            const unsigned char *vmuuid,
>>                                            virNetDevVPortProfilePtr
>> virtPortProfile,
> 
> Having @flags as some middle parameter feels itchy, could you move it
> to the end?
> 

Sure.

> [...]
>> @@ -813,9 +815,12 @@ int virNetDevMacVLanCreateWithVPortProfile(const
>> char *tgifname,
>>                                            char *stateDir,
>>                                            virNetDevBandwidthPtr
>> bandwidth)
>> {
>> -    const char *type = withTap ? "macvtap" : "macvlan";
>> -    const char *prefix = withTap ? MACVTAP_NAME_PREFIX :
>> MACVLAN_NAME_PREFIX;
>> -    const char *pattern = withTap ? MACVTAP_NAME_PATTERN :
>> MACVLAN_NAME_PATTERN;
>> +    const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
>> +        "macvtap" : "macvlan";
>> +    const char *prefix = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
>> +        MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;
>> +    const char *pattern = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
>> +        MACVTAP_NAME_PATTERN : MACVLAN_NAME_PATTERN;
> 
> Since these are now multiline anyway, it would be nice to have them
> set in an if for more readability.

Sure.

Thanks for the comments - Sorry for the delayed reply, just returning
from vacation.

Matt

> 
> Martin