[PATCH 0/4] qemu: Don't use updated qemuCaps in validation code
See patch 4/4 for the justification. CI run: https://gitlab.com/pipo.sk/libvirt/-/pipelines/226815271 since the series is modifying stuff I don't compile usually. Peter Krempa (4): virDomainDefValidate: Add per-run 'opaque' data qemu: validate: Don't check that qemuCaps is non-NULL qemuValidateDomainDeviceDefFS: Fix block indentation qemu: validate: Prefer existing qemuCaps src/bhyve/bhyve_domain.c | 3 +- src/conf/domain_conf.c | 21 +++--- src/conf/domain_conf.h | 9 ++- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_validate.c | 138 ++++++++++++++++++++------------------- src/qemu/qemu_validate.h | 7 +- src/vz/vz_driver.c | 6 +- 7 files changed, 103 insertions(+), 83 deletions(-) -- 2.28.0
virDomainDefPostParse infrastructure has apart from the global opaque data also per-run data, but this was not duplicated into the validation callbacks. This is important when drivers want to use correct run-state for the validation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_domain.c | 3 ++- src/conf/domain_conf.c | 21 +++++++++++++-------- src/conf/domain_conf.h | 9 ++++++--- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_validate.c | 6 ++++-- src/qemu/qemu_validate.h | 7 +++++-- src/vz/vz_driver.c | 6 ++++-- 7 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 6935609b96..f0e553113f 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -199,7 +199,8 @@ virBhyveDriverCreateXMLConf(bhyveConnPtr driver) static int bhyveDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def G_GNUC_UNUSED, - void *opaque G_GNUC_UNUSED) + void *opaque G_GNUC_UNUSED, + void *parseOpaque G_GNUC_UNUSED) { if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER && dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA && diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66ee658e7b..fa19563a35 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6974,14 +6974,15 @@ static int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt) + virDomainXMLOptionPtr xmlopt, + void *parseOpaque) { /* validate configuration only in certain places */ if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE) return 0; if (xmlopt->config.deviceValidateCallback && - xmlopt->config.deviceValidateCallback(dev, def, xmlopt->config.priv)) + xmlopt->config.deviceValidateCallback(dev, def, xmlopt->config.priv, parseOpaque)) return -1; if (virDomainDeviceDefValidateInternal(dev, def) < 0) @@ -6999,7 +7000,8 @@ virDomainDefValidateDeviceIterator(virDomainDefPtr def, { struct virDomainDefPostParseDeviceIteratorData *data = opaque; return virDomainDeviceDefValidate(dev, def, - data->parseFlags, data->xmlopt); + data->parseFlags, data->xmlopt, + data->parseOpaque); } @@ -7357,6 +7359,7 @@ virDomainDefValidateInternal(const virDomainDef *def, * @caps: driver capabilities object * @parseFlags: virDomainDefParseFlags * @xmlopt: XML parser option object + * @parseOpaque: hypervisor driver specific data for this validation run * * This validation function is designed to take checks of globally invalid * configurations that the parser needs to accept so that VMs don't vanish upon @@ -7369,11 +7372,13 @@ virDomainDefValidateInternal(const virDomainDef *def, int virDomainDefValidate(virDomainDefPtr def, unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt) + virDomainXMLOptionPtr xmlopt, + void *parseOpaque) { struct virDomainDefPostParseDeviceIteratorData data = { .xmlopt = xmlopt, .parseFlags = parseFlags, + .parseOpaque = parseOpaque, }; /* validate configuration only in certain places */ @@ -7382,7 +7387,7 @@ virDomainDefValidate(virDomainDefPtr def, /* call the domain config callback */ if (xmlopt->config.domainValidateCallback && - xmlopt->config.domainValidateCallback(def, xmlopt->config.priv) < 0) + xmlopt->config.domainValidateCallback(def, xmlopt->config.priv, parseOpaque) < 0) return -1; /* iterate the devices */ @@ -17220,7 +17225,7 @@ virDomainDeviceDefParse(const char *xmlStr, return NULL; /* validate the configuration */ - if (virDomainDeviceDefValidate(dev, def, flags, xmlopt) < 0) + if (virDomainDeviceDefValidate(dev, def, flags, xmlopt, parseOpaque) < 0) return NULL; return g_steal_pointer(&dev); @@ -22617,7 +22622,7 @@ virDomainObjParseXML(xmlDocPtr xml, goto error; /* validate configuration */ - if (virDomainDefValidate(obj->def, flags, xmlopt) < 0) + if (virDomainDefValidate(obj->def, flags, xmlopt, parseOpaque) < 0) goto error; return obj; @@ -22701,7 +22706,7 @@ virDomainDefParseNode(xmlDocPtr xml, return NULL; /* validate configuration */ - if (virDomainDefValidate(def, flags, xmlopt) < 0) + if (virDomainDefValidate(def, flags, xmlopt, parseOpaque) < 0) return NULL; return g_steal_pointer(&def); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cdf08de60a..72771c46b9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2867,13 +2867,15 @@ typedef void (*virDomainDefPostParseDataFree)(void *parseOpaque); * for configurations that were previously accepted. This shall not modify the * config. */ typedef int (*virDomainDefValidateCallback)(const virDomainDef *def, - void *opaque); + void *opaque, + void *parseOpaque); /* Called once per device, for adjusting per-device settings while * leaving the overall domain otherwise unchanged. */ typedef int (*virDomainDeviceDefValidateCallback)(const virDomainDeviceDef *dev, const virDomainDef *def, - void *opaque); + void *opaque, + void *parseOpaque); struct _virDomainDefParserConfig { /* driver domain definition callbacks */ @@ -2993,7 +2995,8 @@ bool virDomainDeviceAliasIsUserAlias(const char *aliasStr); int virDomainDefValidate(virDomainDefPtr def, unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt); + virDomainXMLOptionPtr xmlopt, + void *parseOpaque); int virDomainActualNetDefValidate(const virDomainNetDef *net); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e0b42ee2c9..474cb93bad 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5464,7 +5464,7 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, * VM that was running before (migration, snapshots, save). It's more * important to start such VM than keep the configuration clean */ if ((flags & VIR_QEMU_PROCESS_START_NEW) && - virDomainDefValidate(vm->def, 0, driver->xmlopt) < 0) + virDomainDefValidate(vm->def, 0, driver->xmlopt, qemuCaps) < 0) return -1; if (qemuProcessStartValidateGraphics(vm) < 0) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 52d15defed..62d7243e21 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1066,7 +1066,8 @@ qemuValidateDomainDefPanic(const virDomainDef *def, int qemuValidateDomainDef(const virDomainDef *def, - void *opaque) + void *opaque, + void *parseOpaque G_GNUC_UNUSED) { virQEMUDriverPtr driver = opaque; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); @@ -4670,7 +4671,8 @@ qemuValidateDomainDeviceDefShmem(virDomainShmemDefPtr shmem, int qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, const virDomainDef *def, - void *opaque) + void *opaque, + void *parseOpaque G_GNUC_UNUSED) { int ret = 0; virQEMUDriverPtr driver = opaque; diff --git a/src/qemu/qemu_validate.h b/src/qemu/qemu_validate.h index acf7d26ce0..b6c5441f90 100644 --- a/src/qemu/qemu_validate.h +++ b/src/qemu/qemu_validate.h @@ -26,10 +26,13 @@ #include "qemu_capabilities.h" #include "qemu_conf.h" -int qemuValidateDomainDef(const virDomainDef *def, void *opaque); +int qemuValidateDomainDef(const virDomainDef *def, + void *opaque, + void *parseOpaque); int qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, const virDomainDef *def, virQEMUCapsPtr qemuCaps); int qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, const virDomainDef *def, - void *opaque); + void *opaque, + void *parseOpaque); diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 8d47b90bdb..b60e99d4f5 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -259,7 +259,8 @@ vzDomainDefPostParse(virDomainDefPtr def, static int vzDomainDefValidate(const virDomainDef *def, - void *opaque) + void *opaque, + void *parseOpaque G_GNUC_UNUSED) { if (vzCheckUnsupportedControllers(def, opaque) < 0) return -1; @@ -295,7 +296,8 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, static int vzDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, - void *opaque) + void *opaque, + void *parseOpaque G_GNUC_UNUSED) { vzDriverPtr driver = opaque; -- 2.28.0
The validation callbacks always fetch latest qemuCaps so it won't ever be NULL. Remove the tautological conditions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 103 +++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 54 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 62d7243e21..81a9c9d45b 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1099,8 +1099,7 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; } - if (qemuCaps && - !virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) { + if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Emulator '%s' does not support machine type '%s'"), def->emulator, def->os.machine); @@ -2648,37 +2647,35 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, return -1; } - if (qemuCaps) { - if (disk->serial && - disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && - disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("scsi-block 'lun' devices do not support the " - "serial property")); - return -1; - } + if (disk->serial && + disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && + disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("scsi-block 'lun' devices do not support the " + "serial property")); + return -1; + } - if (disk->discard && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DISCARD)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("discard is not supported by this QEMU binary")); - return -1; - } + if (disk->discard && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DISCARD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("discard is not supported by this QEMU binary")); + return -1; + } + + if (disk->detect_zeroes && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DETECT_ZEROES)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("detect_zeroes is not supported by this QEMU binary")); + return -1; + } - if (disk->detect_zeroes && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DETECT_ZEROES)) { + if (disk->iomode == VIR_DOMAIN_DISK_IO_URING) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_AIO_IO_URING)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("detect_zeroes is not supported by this QEMU binary")); + _("io uring is not supported by this QEMU binary")); return -1; } - - if (disk->iomode == VIR_DOMAIN_DISK_IO_URING) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_AIO_IO_URING)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("io uring is not supported by this QEMU binary")); - return -1; - } - } } if (disk->serial && @@ -2753,33 +2750,31 @@ qemuValidateDomainDeviceDefDiskBlkdeviotune(const virDomainDiskDef *disk, return -1; } - if (qemuCaps) { - /* block I/O throttling 1.7 */ - if (virDomainBlockIoTuneInfoHasMax(&disk->blkdeviotune) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("there are some block I/O throttling parameters " - "that are not supported with this QEMU binary")); - return -1; - } + /* block I/O throttling 1.7 */ + if (virDomainBlockIoTuneInfoHasMax(&disk->blkdeviotune) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("there are some block I/O throttling parameters " + "that are not supported with this QEMU binary")); + return -1; + } - /* block I/O group 2.4 */ - if (disk->blkdeviotune.group_name && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_GROUP)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the block I/O throttling group parameter is " - "not supported with this QEMU binary")); - return -1; - } + /* block I/O group 2.4 */ + if (disk->blkdeviotune.group_name && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_GROUP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the block I/O throttling group parameter is " + "not supported with this QEMU binary")); + return -1; + } - /* block I/O throttling length 2.6 */ - if (virDomainBlockIoTuneInfoHasMaxLength(&disk->blkdeviotune) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("there are some block I/O throttling length parameters " - "that are not supported with this QEMU binary")); - return -1; - } + /* block I/O throttling length 2.6 */ + if (virDomainBlockIoTuneInfoHasMaxLength(&disk->blkdeviotune) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("there are some block I/O throttling length parameters " + "that are not supported with this QEMU binary")); + return -1; } return 0; @@ -2822,7 +2817,7 @@ qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk, return -1; } - if (qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("transient disk not supported by this QEMU binary (%s)"), disk->dst); -- 2.28.0
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 81a9c9d45b..cb0f830cf1 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3997,8 +3997,7 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs, return -1; } if (fs->multidevs != VIR_DOMAIN_FS_MODEL_DEFAULT && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_MULTIDEVS)) - { + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_MULTIDEVS)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("multidevs is not supported with this QEMU binary")); return -1; -- 2.28.0
The validation callback always fetched a fresh copy of 'qemuCaps' to use for validation which is wrong in cases when the VM is already running, such as device hotplug. The newly-fetched qemuCaps may contain flags which weren't originally in use when starting the VM e.g. on a libvirtd upgrade. Since the post-parse/validation machinery has a per-run 'parseOpaque' field filled with qemuCaps of the actual process we can reuse the caps in cases when we get them. The code still fetches a fresh copy if parseOpaque doesn't have a per-run copy to preserve existing functionality. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index cb0f830cf1..bf8127a575 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1067,16 +1067,21 @@ qemuValidateDomainDefPanic(const virDomainDef *def, int qemuValidateDomainDef(const virDomainDef *def, void *opaque, - void *parseOpaque G_GNUC_UNUSED) + void *parseOpaque) { virQEMUDriverPtr driver = opaque; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - g_autoptr(virQEMUCaps) qemuCaps = NULL; + g_autoptr(virQEMUCaps) qemuCapsLocal = NULL; + virQEMUCapsPtr qemuCaps = parseOpaque; size_t i; - if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, - def->emulator))) - return -1; + if (!qemuCaps) { + if (!(qemuCapsLocal = virQEMUCapsCacheLookup(driver->qemuCapsCache, + def->emulator))) + return -1; + + qemuCaps = qemuCapsLocal; + } if (def->os.type != VIR_DOMAIN_OSTYPE_HVM) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -4666,15 +4671,20 @@ int qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, const virDomainDef *def, void *opaque, - void *parseOpaque G_GNUC_UNUSED) + void *parseOpaque) { int ret = 0; virQEMUDriverPtr driver = opaque; - g_autoptr(virQEMUCaps) qemuCaps = NULL; + g_autoptr(virQEMUCaps) qemuCapsLocal = NULL; + virQEMUCapsPtr qemuCaps = parseOpaque; - if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, - def->emulator))) - return -1; + if (!qemuCaps) { + if (!(qemuCapsLocal = virQEMUCapsCacheLookup(driver->qemuCapsCache, + def->emulator))) + return -1; + + qemuCaps = qemuCapsLocal; + } if ((ret = qemuValidateDomainDeviceDefAddress(dev, qemuCaps)) < 0) return ret; -- 2.28.0
On 12/8/20 3:01 PM, Peter Krempa wrote:
The validation callback always fetched a fresh copy of 'qemuCaps' to use for validation which is wrong in cases when the VM is already running, such as device hotplug. The newly-fetched qemuCaps may contain flags which weren't originally in use when starting the VM e.g. on a libvirtd upgrade.
Since the post-parse/validation machinery has a per-run 'parseOpaque' field filled with qemuCaps of the actual process we can reuse the caps in cases when we get them.
The code still fetches a fresh copy if parseOpaque doesn't have a per-run copy to preserve existing functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)
So this will use the old qemuCaps even though a feature might already be available in qemu but libvirt didn't detect it. For instance the virtio-pmem I'm implementing - guests started today won't have the qemuCap for virtio-pmem because it's not implemented yet. But when they update libvirt they still won't get the feature because we're using old qemuCaps for validation and thus new memory model will be denied because of lacking capability. I can see the value in when we have to chose between old and new style of doing things, but I worry it will kill new features. Michal
On Tue, Dec 08, 2020 at 16:09:07 +0100, Michal Privoznik wrote:
On 12/8/20 3:01 PM, Peter Krempa wrote:
The validation callback always fetched a fresh copy of 'qemuCaps' to use for validation which is wrong in cases when the VM is already running, such as device hotplug. The newly-fetched qemuCaps may contain flags which weren't originally in use when starting the VM e.g. on a libvirtd upgrade.
Since the post-parse/validation machinery has a per-run 'parseOpaque' field filled with qemuCaps of the actual process we can reuse the caps in cases when we get them.
The code still fetches a fresh copy if parseOpaque doesn't have a per-run copy to preserve existing functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)
So this will use the old qemuCaps even though a feature might already be available in qemu but libvirt didn't detect it. For instance the virtio-pmem I'm implementing - guests started today won't have the qemuCap for virtio-pmem because it's not implemented yet. But when they update libvirt they still won't get the feature because we're using old qemuCaps for validation and thus new memory model will be denied because of lacking capability.
Oh, this is actually what we do and it's required! Old libvirt might have configured the device in a way which is not compatible with the new one and thus attempting to talk to it using the new interface would just break. We currently store the old state of 'qemuCaps' and everything uses it in the state when the VM was started. Well except for the validator, which was broken and this patchset is fixing it. You will never get new features with an old VM.
On 12/8/20 4:20 PM, Peter Krempa wrote:
On Tue, Dec 08, 2020 at 16:09:07 +0100, Michal Privoznik wrote:
On 12/8/20 3:01 PM, Peter Krempa wrote:
The validation callback always fetched a fresh copy of 'qemuCaps' to use for validation which is wrong in cases when the VM is already running, such as device hotplug. The newly-fetched qemuCaps may contain flags which weren't originally in use when starting the VM e.g. on a libvirtd upgrade.
Since the post-parse/validation machinery has a per-run 'parseOpaque' field filled with qemuCaps of the actual process we can reuse the caps in cases when we get them.
The code still fetches a fresh copy if parseOpaque doesn't have a per-run copy to preserve existing functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)
So this will use the old qemuCaps even though a feature might already be available in qemu but libvirt didn't detect it. For instance the virtio-pmem I'm implementing - guests started today won't have the qemuCap for virtio-pmem because it's not implemented yet. But when they update libvirt they still won't get the feature because we're using old qemuCaps for validation and thus new memory model will be denied because of lacking capability.
Oh, this is actually what we do and it's required! Old libvirt might have configured the device in a way which is not compatible with the new one and thus attempting to talk to it using the new interface would just break.
We currently store the old state of 'qemuCaps' and everything uses it in the state when the VM was started. Well except for the validator, which was broken and this patchset is fixing it.
You will never get new features with an old VM.
Okay then. Michal
On a Tuesday in 2020, Peter Krempa wrote:
See patch 4/4 for the justification.
CI run: https://gitlab.com/pipo.sk/libvirt/-/pipelines/226815271
since the series is modifying stuff I don't compile usually.
Peter Krempa (4): virDomainDefValidate: Add per-run 'opaque' data qemu: validate: Don't check that qemuCaps is non-NULL qemuValidateDomainDeviceDefFS: Fix block indentation qemu: validate: Prefer existing qemuCaps
src/bhyve/bhyve_domain.c | 3 +- src/conf/domain_conf.c | 21 +++--- src/conf/domain_conf.h | 9 ++- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_validate.c | 138 ++++++++++++++++++++------------------- src/qemu/qemu_validate.h | 7 +- src/vz/vz_driver.c | 6 +- 7 files changed, 103 insertions(+), 83 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Ján Tomko -
Michal Privoznik -
Peter Krempa