On 9/11/24 7:42 PM, Demi Marie Obenour wrote:
On Wed, Sep 11, 2024 at 05:09:03PM -0600, Jim Fehlig wrote:
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@xxxxxxxx>
---
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?
Now that we're talking more about this, I'm having flashbacks of new
features being added in, and the author (e.g. me) basically updating the
hypervisor drivers they were personally interested in to support the
feature (or else explicitly log an error), but leaving the other drivers
untouched because "I can't test it, so I don't want to add code that
could potentially end up failing when somebody actually *can* test it"
(or some such other copout :-P) (e.g. in my case that used to be qemu
and lxc, but for a long time has been only qemu).
I'm not familiar with how libvirt works internally, but to me it seems
like one option would be to have a flag set on the drivers that support
network filtering. Generic code would then check the flag and fail if
filtering is requested with a driver that doesn't support it.
This has the advantage of not requiring changes to each and every
driver.
An interesting idea, but then we would really want to do that for every
individual XML knob; essentially generic "capabilities flags" similar to
the QEMU capabilities flags we use to determine whether a particular
feature is available for a particular QEMU binary.
