[libvirt] [PATCH] virNetDevMacVLanTapSetup: Work around older systems

[Laine Stump]

On 12/12/2015 02:20 AM, Michal Privoznik wrote:
> Some older systems, e.g. RHEL-6 do not have IFF_MULTI_QUEUE flag
> which we use to enable multiqueue feature. Therefore one gets the
> following compile error there:
>
>    CC     util/libvirt_util_la-virnetdevmacvlan.lo
> util/virnetdevmacvlan.c: In function 'virNetDevMacVLanTapSetup':
> util/virnetdevmacvlan.c:338: error: 'IFF_MULTI_QUEUE' undeclared (first use in this function)
> util/virnetdevmacvlan.c:338: error: (Each undeclared identifier is reported only once
> util/virnetdevmacvlan.c:338: error: for each function it appears in.)
> make[3]: *** [util/libvirt_util_la-virnetdevmacvlan.lo] Error 1
>
> So, whenever user wants us to enable the feature on such systems,
> we will just throw a runtime error instead.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>   src/util/virnetdevmacvlan.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index d8d1d90..28c9f22 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -307,49 +307,57 @@ static int
>   virNetDevMacVLanTapSetup(int *tapfd, size_t tapfdSize, bool vnet_hdr, bool multiqueue)
>   {
>       unsigned int features;
>       struct ifreq ifreq;
>       short new_flags = 0;
>       size_t i;
>   
>       for (i = 0; i < tapfdSize; i++) {
>           memset(&ifreq, 0, sizeof(ifreq));
>   
>           if (ioctl(tapfd[i], TUNGETIFF, &ifreq) < 0) {
>               virReportSystemError(errno, "%s",
>                                    _("cannot get interface flags on macvtap tap"));
>               return -1;
>           }
>   
>           new_flags = ifreq.ifr_flags;
>   
>           if (vnet_hdr) {
>               if (ioctl(tapfd[i], TUNGETFEATURES, &features) < 0) {
>                   virReportSystemError(errno, "%s",
>                                        _("cannot get feature flags on macvtap tap"));
>                   return -1;
>               }
>               if (features & IFF_VNET_HDR)
>                   new_flags |= IFF_VNET_HDR;
>           } else {
>               new_flags &= ~IFF_VNET_HDR;
>           }
>   
> +#ifdef IFF_MULTI_QUEUE
>           if (multiqueue)
>               new_flags |= IFF_MULTI_QUEUE;
>           else
>               new_flags &= ~IFF_MULTI_QUEUE;
> +#else
> +        if (multiqueue) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("Multiqueue devices are not supported on this system"));
> +            return -1;
> +        }
> +#endif

Yep, missed that difference with the tap version in review.

BTW, this caused me to look back at patch 5 and 6 of your 7 patch 
series, and I noticed a couple things:


1) you pass down a separate bool, multiqueue, to this function in the 
macvtap version, but in the tap version you just check "tapfdSize" right 
at the spot where you're setting it, eliminating the need for an extra 
arg. That seems cleaner.

2) When creating the bool to pass into this function, you use:

> +        if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, vnet_hdr, tapfdSize > 0) < 0) {

That should be "tapfdSize > 1" (and as I implied in (1), I think it would be simpler
to just do that directly in virNetDevMacVLanTapSetup rather than passing it in a separate
bool.

ACK to this change.

It looks like you haven't pushed the other patches yet, so how about
you just squash this change into the other patches so they are all
correct from the start?


>   
>           if (new_flags != ifreq.ifr_flags) {
>               ifreq.ifr_flags = new_flags;
>               if (ioctl(tapfd[i], TUNSETIFF, &ifreq) < 0) {
>                   virReportSystemError(errno, "%s",
>                                        _("unable to set vnet or multiqueue flags on macvtap"));
>                   return -1;
>               }
>           }
>       }
>   
>       return 0;
>   }
>   
>