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(a)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;
}