Re: [PATCH 6/5] lxc: skip the netdev autogenerated name counter past existing devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/16/20 4:27 PM, Michal Privoznik wrote:
On 12/16/20 9:13 PM, Laine Stump wrote:
the lxc driver uses virNetDevGenerateName() for its veth device names
since patch 2dd0fb492, so it should be using virNetDevReserveName()
during daemon restart/reconnect to skip over the device names that are
in use.

Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
---

I meant to mention this during review of the abovementioned patch, but forgot.

(NB: a couple days ago I *removed* similar code from this same spot,
but it was trying to reserve the name of macvlan devices; a macvlan
device is moved into the container's namespace at startup, so it is
not visible to the host anyway. This new case is for the 1/2 of a veth
pair that does remain in the host's namespace
(type='bridge|network|ethernet' use a veth pair)


  src/lxc/lxc_process.c | 24 ++++++++++++++++++++++++
  1 file changed, 24 insertions(+)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 0f7c929535..a842ac91c5 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1640,6 +1640,30 @@ virLXCProcessReconnectNotifyNets(virDomainDefPtr def)
      for (i = 0; i < def->nnets; i++) {
          virDomainNetDefPtr net = def->nets[i];
+        /* type='bridge|network|ethernet' interfaces may be using an
+         * autogenerated netdev name, so we should update the counter
+         * for autogenerated names to skip past this one.
+         */
+        switch (virDomainNetGetActualType(net)) {
+        case VIR_DOMAIN_NET_TYPE_BRIDGE:
+        case VIR_DOMAIN_NET_TYPE_NETWORK:
+        case VIR_DOMAIN_NET_TYPE_ETHERNET:
+            virNetDevReserveName(net->ifname);
+            break;
+        case VIR_DOMAIN_NET_TYPE_DIRECT:
+        case VIR_DOMAIN_NET_TYPE_USER:
+        case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+        case VIR_DOMAIN_NET_TYPE_SERVER:
+        case VIR_DOMAIN_NET_TYPE_CLIENT:
+        case VIR_DOMAIN_NET_TYPE_MCAST:
+        case VIR_DOMAIN_NET_TYPE_INTERNAL:
+        case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+        case VIR_DOMAIN_NET_TYPE_UDP:
+        case VIR_DOMAIN_NET_TYPE_VDPA:
+        case VIR_DOMAIN_NET_TYPE_LAST:
+            break;
+        }
+

I remember Peter being picky about switch()-es and (almost) always he wanted me to add the default case with virReportEnumRangeError() despite the variable passed to switch() being verified earlier. IIUC his reasoning was that if we had a memory being overwritten somewhere it's better to error out (I say it's better to crash), but since I don't care that much, this could have:

         default:
            virReportEnumRangeError(virDomainNetType, virDomainNetGetActualType(net));
             return;

But if we add a default case to the switch, we won't get all the nice compile-time errors when we add a new value to the enum and forget to add it to every switch, will we?



At your discretion.

          if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
              if (!conn && !(conn = virGetConnectNetwork()))
                  continue;


Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux