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

On 9/11/24 16:24, Laine Stump wrote: > 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, Yes, good point! The libxl driver already has domainValidateCallback, but now needs a deviceValidateCallback for this code. I'll make that change in V2. Before sending another version, I'd like to hear opinions on Demi's question about the other hypervisor drivers. Do they need a similar change?