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

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



On 08/28/2014 08:45 AM, Martin Kletzander wrote:
> On Wed, Aug 27, 2014 at 10:34:13AM -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 linux vnet ibm com>
>> ---
>> src/lxc/lxc_process.c       |    4 ++--
>> src/qemu/qemu_command.c     |    6 ++++--
>> src/util/virnetdevmacvlan.c |   28 +++++++++++++++++-----------
>> src/util/virnetdevmacvlan.h |   14 ++++++++++----
>> 4 files changed, 33 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
>> index 3353dc1..ed30c37 100644
>> --- a/src/lxc/lxc_process.c
>> +++ b/src/lxc/lxc_process.c
>> @@ -331,12 +331,12 @@ char
>> *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
>>             net->ifname, &net->mac,
>>             virDomainNetGetActualDirectDev(net),
>>             virDomainNetGetActualDirectMode(net),
>> -            false, false, def->uuid,
>> +            false, def->uuid,
>>             virDomainNetGetActualVirtPortProfile(net),
>>             &res_ifname,
>>             VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
>>             cfg->stateDir,
>> -            virDomainNetGetActualBandwidth(net)) < 0)
>> +            virDomainNetGetActualBandwidth(net), 0) < 0)
>>         goto cleanup;
>>
>>     ret = res_ifname;
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 9241f57..1f71fa3 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -177,6 +177,7 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
>>     char *res_ifname = NULL;
>>     int vnet_hdr = 0;
>>     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> +    unsigned int macvlan_create_flags =
>> VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
>>
>>     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNET_HDR) &&
>>         net->model && STREQ(net->model, "virtio"))
>> @@ -186,11 +187,12 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
>>         net->ifname, &net->mac,
>>         virDomainNetGetActualDirectDev(net),
>>         virDomainNetGetActualDirectMode(net),
>> -        true, vnet_hdr, def->uuid,
>> +        vnet_hdr, def->uuid,
>>         virDomainNetGetActualVirtPortProfile(net),
>>         &res_ifname,
>>         vmop, cfg->stateDir,
>> -        virDomainNetGetActualBandwidth(net));
>> +        virDomainNetGetActualBandwidth(net),
>> +        macvlan_create_flags);
>>     if (rc >= 0) {
>>         virDomainAuditNetDevice(def, net, res_ifname, true);
>>         VIR_FREE(net->ifname);
>> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
>> index 054194b..9ddeca4 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.
> 
> I took the liberty of moving this as a last parameter as well.
> 
>>  * @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,25 +798,29 @@
>> 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
>> + * -1 on error.
>>  */
>> int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
>>                                            const virMacAddr *macaddress,
>>                                            const char *linkdev,
>>                                            virNetDevMacVLanMode mode,
>> -                                           bool withTap,
>>                                            int vnet_hdr,
>>                                            const unsigned char *vmuuid,
>>                                            virNetDevVPortProfilePtr
>> virtPortProfile,
>>                                            char **res_ifname,
>>                                            virNetDevVPortProfileOp vmOp,
>>                                            char *stateDir,
>> -                                           virNetDevBandwidthPtr
>> bandwidth)
>> +                                           virNetDevBandwidthPtr
>> bandwidth,
>> +                                           unsigned int flags)
>> {
>> -    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;
>>     int c, rc;
>>     char ifname[IFNAMSIZ];
>>     int retries, do_retry = 0;
>> @@ -903,7 +908,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const
>> char *tgifname,
>>         goto disassociate_exit;
>>     }
>>
>> -    if (withTap) {
>> +    if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) {
>>         if ((rc = virNetDevMacVLanTapOpen(cr_ifname, 10)) < 0)
>>             goto disassociate_exit;
>>
>> @@ -922,7 +927,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const
>> char *tgifname,
>>     }
>>
>>     if (virNetDevBandwidthSet(cr_ifname, bandwidth, false) < 0) {
>> -        if (withTap)
>> +        if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP)
>>             VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
>>         else
>>             rc = -1;
>> @@ -1066,15 +1071,16 @@ int
>> virNetDevMacVLanCreateWithVPortProfile(const char *ifname
>> ATTRIBUTE_UNUSED,
>>                                            const virMacAddr
>> *macaddress ATTRIBUTE_UNUSED,
>>                                            const char *linkdev
>> ATTRIBUTE_UNUSED,
>>                                            virNetDevMacVLanMode mode
>> ATTRIBUTE_UNUSED,
>> -                                           bool withTap
>> ATTRIBUTE_UNUSED,
>>                                            int vnet_hdr ATTRIBUTE_UNUSED,
>>                                            const unsigned char *vmuuid
>> ATTRIBUTE_UNUSED,
>>                                            virNetDevVPortProfilePtr
>> virtPortProfile ATTRIBUTE_UNUSED,
>>                                            char **res_ifname
>> ATTRIBUTE_UNUSED,
>>                                            virNetDevVPortProfileOp
>> vmop ATTRIBUTE_UNUSED,
>>                                            char *stateDir
>> ATTRIBUTE_UNUSED,
>> -                                           virNetDevBandwidthPtr
>> bandwidth ATTRIBUTE_UNUSED)
>> +                                           virNetDevBandwidthPtr
>> bandwidth ATTRIBUTE_UNUSED,
>> +                                           unsigned int flags)
>> {
>> +    virCheckFlags(0, NULL);
> 
> The flag check could be fixed not to report in such cases, but it's
> easier just to check the flags for now.  But it should be:
> 
> virCheckFlags(0, -1);
> 
>>     virReportSystemError(ENOSYS, "%s",
>>                          _("Cannot create macvlan devices on this
>> platform"));
>>     return -1;
>> diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h
>> index 9b15a31..e92da4a 100644
>> --- a/src/util/virnetdevmacvlan.h
>> +++ b/src/util/virnetdevmacvlan.h
>> @@ -40,6 +40,12 @@ typedef enum {
>> } virNetDevMacVLanMode;
>> VIR_ENUM_DECL(virNetDevMacVLanMode)
>>
>> +typedef enum {
>> +   VIR_NETDEV_MACVLAN_CREATE_NONE = 0,
>> +   /* Create with a tap device */
>> +   VIR_NETDEV_MACVLAN_CREATE_WITH_TAP       = 1 << 0,
> 
> You aligned only one value.  Unless you object to it, I'll align them
> together without too many whitespaces.
> 
> Other than that, it looks fine, ACK after 1.2.8, I'll push it with the
> changes mentioned after the release if you're OK with that.

Yep, I'm fine with the changes you've mentioned.  Thanks Martin!

> 
> Martin


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