Allow VF vlanid to be passed as a pointer

There should be a way to show no intent in programming a VLAN at all
(including clearing it). This allows handling error conditions
differently when VLAN clearing is explicit (vlan id == 0) vs implicit
(vlanid == NULL - try to clear it if possible).

Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

src/hypervisor/virhostdev.c

@@ -463,6 +463,7 @@ virHostdevSetNetConfig(virDomainHostdevDef *hostdev,
     const virNetDevVPortProfile *virtPort;
     int vf = -1;
     bool port_profile_associate = true;
+    bool setVlan = false;
 
     if (!virHostdevIsPCINetDevice(hostdev))
         return 0;
@@ -471,6 +472,7 @@ virHostdevSetNetConfig(virDomainHostdevDef *hostdev,
         return -1;
 
     vlan = virDomainNetGetActualVlan(hostdev->parentnet);
+    setVlan = vlan != NULL;
     virtPort = virDomainNetGetActualVirtPortProfile(hostdev->parentnet);
     if (virtPort) {
         if (vlan) {
@@ -486,7 +488,7 @@ virHostdevSetNetConfig(virDomainHostdevDef *hostdev,
             return -1;
     } else {
         if (virNetDevSetNetConfig(linkdev, vf, &hostdev->parentnet->mac,
-                                  vlan, NULL, true) < 0)
+                                  vlan, NULL, setVlan) < 0)
             return -1;
     }
 

src/util/virnetdev.c

@@ -1588,19 +1588,23 @@ virNetDevSendVfSetLinkRequest(const char *ifname,
 int
 virNetDevSetVfVlan(const char *ifname,
                    int vf,
-                   int vlanid)
+                   const int *vlanid)
 {
     int ret = -1;
     struct ifla_vf_vlan ifla_vf_vlan = {
         .vf = vf,
-        .vlan = vlanid,
+        .vlan = 0,
         .qos = 0,
     };
 
-    /* VLAN ids 0 and 4095 are reserved per 802.1Q but are valid values. */
+    /* If vlanid is NULL, assume it needs to be cleared. */
-    if ((vlanid < 0 || vlanid > 4095)) {
+    if (vlanid) {
-        virReportError(ERANGE, _("vlanid out of range: %d"), vlanid);
+        /* VLAN ids 0 and 4095 are reserved per 802.1Q but are valid values. */
-        return -ERANGE;
+        if ((*vlanid < 0 || *vlanid > 4095)) {
+            virReportError(ERANGE, _("vlanid out of range: %d"), *vlanid);
+            return -ERANGE;
+        }
+        ifla_vf_vlan.vlan = *vlanid;
     }
 
     ret = virNetDevSendVfSetLinkRequest(ifname, IFLA_VF_VLAN,
@@ -1609,11 +1613,11 @@ virNetDevSetVfVlan(const char *ifname,
     if (ret < 0) {
         virReportSystemError(-ret,
                              _("Cannot set interface vlanid to %d for ifname %s vf %d"),
-                             vlanid, ifname ? ifname : "(unspecified)", vf);
+                             ifla_vf_vlan.vlan, ifname ? ifname : "(unspecified)", vf);
     }
 
     VIR_DEBUG("RTM_SETLINK %s vf %d vlanid=%d - %s",
-              ifname, vf, vlanid, ret < 0 ? "Fail" : "Success");
+              ifname, vf, ifla_vf_vlan.vlan, ret < 0 ? "Fail" : "Success");
     return ret;
 }
 
@@ -1663,7 +1667,7 @@ int
 virNetDevSetVfConfig(const char *ifname,
                      int vf,
                      const virMacAddr *macaddr,
-                     int vlanid,
+                     const int *vlanid,
                      bool *allowRetry)
 {
     int ret = -1;
@@ -2200,7 +2204,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
     const char *pfDevName = NULL;
     g_autofree char *pfDevOrig = NULL;
     g_autofree char *vfDevOrig = NULL;
-    int vlanTag = -1;
+    g_autofree int *vlanTag = NULL;
     g_autoptr(virPCIDevice) vfPCIDevice = NULL;
 
     if (vf >= 0) {
@@ -2259,10 +2263,17 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
                 return -1;
             }
 
-            vlanTag = vlan->tag[0];
+            vlanTag = g_new0(int, 1);
+            *vlanTag = vlan->tag[0];
 
         } else if (setVlan) {
-            vlanTag = 0; /* assure any existing vlan tag is reset */
+            vlanTag = g_new0(int, 1);
+            /* Assure any existing vlan tag is reset. */
+            *vlanTag = 0;
+        } else {
+            /* Indicate that setting a VLAN has not been explicitly requested.
+             * This allows selected errors in clearing a VF VLAN to be ignored. */
+            vlanTag = NULL;
         }
     }
 
@@ -2344,7 +2355,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
         }
     }
 
-    if (adminMAC || vlanTag >= 0) {
+    if (adminMAC) {
         /* Set vlanTag and admin MAC using an RTM_SETLINK request sent to
          * PFdevname+VF#, if mac != NULL this will set the "admin MAC" via
          * the PF, *not* the actual VF MAC - the admin MAC only takes
@@ -2442,7 +2453,7 @@ virNetDevSendVfSetLinkRequest(const char *ifname G_GNUC_UNUSED,
 int
 virNetDevSetVfVlan(const char *ifname G_GNUC_UNUSED,
                    int vf G_GNUC_UNUSED,
-                   int vlanid G_GNUC_UNUSED)
+                   const int *vlanid G_GNUC_UNUSED)
 {
     virReportSystemError(ENOSYS, "%s",
                          _("Unable to set a VF VLAN on this platform"));
@@ -2464,7 +2475,7 @@ int
 virNetDevSetVfConfig(const char *ifname G_GNUC_UNUSED,
                      int vf G_GNUC_UNUSED,
                      const virMacAddr *macaddr G_GNUC_UNUSED,
-                     int vlanid G_GNUC_UNUSED,
+                     const int *vlanid G_GNUC_UNUSED,
                      bool *allowRetry G_GNUC_UNUSED)
 {
     virReportSystemError(ENOSYS, "%s",

src/util/virnetdevpriv.h

@@ -35,7 +35,7 @@ virNetDevSendVfSetLinkRequest(const char *ifname,
 int
 virNetDevSetVfVlan(const char *ifname,
                    int vf,
-                   int vlanid);
+                   const int *vlanid);
 
 int
 virNetDevSetVfMac(const char *ifname,
@@ -47,5 +47,5 @@ int
 virNetDevSetVfConfig(const char *ifname,
                      int vf,
                      const virMacAddr *macaddr,
-                     int vlanid,
+                     const int *vlanid,
                      bool *allowRetry);

tests/virnetdevtest.c

@@ -82,7 +82,7 @@ int
 int
 (*real_virNetDevSetVfVlan)(const char *ifname,
                            int vf,
-                           int vlanid);
+                           const int *vlanid);
 
 static void
 init_syms(void)
@@ -119,7 +119,7 @@ virNetDevSetVfMac(const char *ifname,
 int
 virNetDevSetVfVlan(const char *ifname,
                    int vf,
-                   int vlanid)
+                   const int *vlanid)
 {
     init_syms();
 
@@ -222,6 +222,11 @@ testVirNetDevSetVfVlan(const void *opaque G_GNUC_UNUSED)
         const int vlan_id;
         const int rc;
     };
+    struct nullVlanTestCase {
+        const char *ifname;
+        const int vf_num;
+        const int rc;
+    };
     size_t i = 0;
     int rc = 0;
     const struct testCase testCases[] = {
@@ -242,13 +247,27 @@ testVirNetDevSetVfVlan(const void *opaque G_GNUC_UNUSED)
         { .ifname = "fakeiface-ok", .vf_num = 1, .vlan_id = 42, .rc = 0 },
     };
 
+    const struct nullVlanTestCase nullVLANTestCases[] = {
+        { .ifname = "fakeiface-eperm", .vf_num = 1, .rc = -EPERM },
+        { .ifname = "fakeiface-eagain", .vf_num = 1, .rc = -EAGAIN },
+        /* Successful requests with vlan id 0 need to have a zero return code. */
+        { .ifname = "fakeiface-ok", .vf_num = 1, .rc = 0 },
+    };
+
     for (i = 0; i < sizeof(testCases) / sizeof(struct testCase); ++i) {
-       rc = virNetDevSetVfVlan(testCases[i].ifname, testCases[i].vf_num, testCases[i].vlan_id);
+       rc = virNetDevSetVfVlan(testCases[i].ifname, testCases[i].vf_num, &testCases[i].vlan_id);
        if (rc != testCases[i].rc) {
            return -1;
        }
     }
 
+    for (i = 0; i < sizeof(nullVLANTestCases) / sizeof(struct nullVlanTestCase); ++i) {
+       rc = virNetDevSetVfVlan(nullVLANTestCases[i].ifname, nullVLANTestCases[i].vf_num, NULL);
+       if (rc != nullVLANTestCases[i].rc) {
+           return -1;
+       }
+    }
+
     return 0;
 }
 
@@ -276,7 +295,7 @@ testVirNetDevSetVfConfig(const void *opaque G_GNUC_UNUSED)
     };
 
     for (i = 0; i < sizeof(testCases) / sizeof(struct testCase); ++i) {
-       rc = virNetDevSetVfConfig(testCases[i].ifname, vfNum, &mac, vlanid, allowRetry);
+       rc = virNetDevSetVfConfig(testCases[i].ifname, vfNum, &mac, &vlanid, allowRetry);
        if (rc != testCases[i].rc) {
            return -1;
        }