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

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(a)linux.vnet.ibm.com&gt; > --- > 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/

> + * -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?

[...] > @@ -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.

Martin