On 12/17/20 1:28 AM, Laine Stump wrote:
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@redhat.com> ---
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?
We will. I worried about the same, but as it turned out we will get errors if a new item is added into the enum. Michal