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@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; }