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 <[email protected]>
---
  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;
  }

--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to