Re: [PATCH 6/5] lxc: skip the netdev autogenerated name counter past existing devices
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(a)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?
At your discretion.
> if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> if (!conn && !(conn = virGetConnectNetwork()))
> continue;
>
Michal