Re: [PATCH 2/2] libxl: Reject VM config referencing nwfilters

On 9/11/24 5:02 PM, Jim Fehlig via Devel wrote: > The Xen libxl driver does not support nwfilter. Add a check for nwfilters > to the devicesPostParseCallback, returning VIR_ERR_CONFIG_UNSUPPORTED if > any are found. > > It's generally preferred for drivers to ignore unsupported XML features, I would instead characterize it as "drivers generally ignore *unrecognized* XML", but it's quite common for a bit of XML that's understood and supported in one context within libvirt to generate an UNSUPPORTED error when attempting to use it in a place where it isn't supported. > but ignoring a user's request to filter VM network traffic can be viewed > as a security issue. Definitely. > > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > src/libxl/libxl_domain.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > index 0f129ec69c..2f6cebb8ae 100644 > --- a/src/libxl/libxl_domain.c > +++ b/src/libxl/libxl_domain.c > @@ -131,6 +131,13 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDef *dev, > void *opaque G_GNUC_UNUSED, > void *parseOpaque G_GNUC_UNUSED) > { > + if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->filter) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("filterref is not supported in %1$s"), > + virDomainVirtTypeToString(def->virtType)); > + return -1; > + } > + This more properly should be in a function called libxlValidateDomainDeviceDef(), which would look something like qemuValidateDomainDeviceDef() and be added into libxlDomainDefParserConfig with this initialization: .deviceValidateCallback = libxlValidateDomainDeviceDef, (my understanding of the purpose of the two has always been that the PostParse callback is intended for performing post-parse modifications/fixups to the domain def, while the Validate only checks for correctness, without modifying anything. There are multiple cases of having validation in the postparse function, but I think those are the 1) leftovers from before the introduction of the validate callbacks, and 2) the result of misunderstanding and/or sloth (e.g. in cases where you want to validate something, but the driver you're adding this validation to doesn't already have a deviceValidateCallback)