Re: [libvirt] [PATCH 1/2] lxc: stop incorrectly validating interface type

On Wed, Dec 05, 2018 at 09:35:12PM -0500, Laine Stump wrote: > Commit 017dfa27d changed a few switch statements in the LXC code to > have all possible enum values, and in the process changed the switch > statement in virLXCControllerGetNICIndexes() such that it returned > error status for any interfaces that weren't implemented with a veth > pair when it should have just been ignoring those interfaces. > > Since the interface type will have already been validated before > reaching this function, we shouldn't be doing any validation at all - > just add the ifindex of the parent veth if its a veth pair, and ignore > it otherwise. > > Resolves: https://bugzilla.redhat.com/1656463 > Signed-off-by: Laine Stump <laine(a)laine.org&gt; > --- > src/lxc/lxc_controller.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c > index e853d02d65..1c20f451af 100644 > --- a/src/lxc/lxc_controller.c > +++ b/src/lxc/lxc_controller.c > @@ -364,6 +364,16 @@ static int virLXCControllerGetNICIndexes(virLXCControllerPtr ctrl) > size_t i; > int ret = -1; > > + /* Gather the ifindexes of the "parent" veths for all interfaces > + * implemented with a veth pair. These will be used when calling > + * virCgroupNewMachine (and eventually the dbus method > + * CreateMachineWithNetwork). ifindexes for the child veths, and > + * for macvlan interfaces, *should not* be in this list, as they > + * will be moved into the container. Only the interfaces that will > + * remain outside the container, but are used for communication > + * with the container, should be added to the list. > + */ > + > VIR_DEBUG("Getting nic indexes"); > for (i = 0; i < ctrl->def->nnets; i++) { > int nicindex = -1; > @@ -394,14 +404,9 @@ static int virLXCControllerGetNICIndexes(virLXCControllerPtr ctrl) > case VIR_DOMAIN_NET_TYPE_INTERNAL: > case VIR_DOMAIN_NET_TYPE_DIRECT: > case VIR_DOMAIN_NET_TYPE_HOSTDEV: > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > - _("Unsupported net type %s"), > - virDomainNetTypeToString(ctrl->def->nets[i]->type)); > - goto cleanup; > case VIR_DOMAIN_NET_TYPE_LAST: > default: > - virReportEnumRangeError(virDomainNetType, ctrl->def->nets[i]->type); > - goto cleanup; > + break; > } This is removing too much. Only ethernet, bridge, network & direct are going to work with LXC. All others should always result in an error message here, and absolutely the LAST/default case must always report enum error.