[PATCH 0/4] qemu: Init ext devices paths on reconnect
*** BLURB HERE *** Michal Prívozník (4): qemu_process: Document qemuProcessPrepare{Domain,Host}() order qemu_extdevice: Init paths in qemuExtDevicesPrepareDomain() qemu_extdevice: Expose qemuExtDevicesInitPaths() qemu: Init ext devices paths on reconnect src/qemu/qemu_extdevice.c | 8 ++++---- src/qemu/qemu_extdevice.h | 5 +++++ src/qemu/qemu_process.c | 7 +++++++ 3 files changed, 16 insertions(+), 4 deletions(-) -- 2.37.4
The domain startup process is split into multiple phases. One of them is preparing the domain (at that point live) XML, private data, various paths, etc - see qemuProcessPrepareDomain(); the other prepares the host - see qemuProcessPrepareHost(). It's obvious that the domain XML preparation function must be called before the host preparation function (e.g. the host preparation might try to create a file which path is generated in the domain preparation phase). Nevertheless, let's document this expectation. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e1c18dde90..19b9242623 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6634,6 +6634,8 @@ qemuProcessPrepareChardevSource(virDomainDef *def, * start the domain but create a valid qemu command. If some code shouldn't be * executed in this case, make sure to check this flag. * + * This function MUST be called before qemuProcessPrepareHost(). + * * TODO: move all XML modification from qemuBuildCommandLine into this function */ int @@ -7151,6 +7153,8 @@ qemuProcessPrepareHostBackendChardevHotplug(virDomainObj *vm, * update live XML) to prepare environment for a domain which is about to start * and it's the only place to do those modifications. * + * This function MUST be called only after qemuProcessPrepareDomain(). + * * TODO: move all host modification from qemuBuildCommandLine into this function */ int -- 2.37.4
The path generation phase belongs conceptually into domain preparation phase and not host preparation. Move qemuExtDevicesInitPaths() call from qemuExtDevicesPrepareHost() into qemuExtDevicesPrepareDomain(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_extdevice.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 3eaf6571a2..34454891f6 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -97,6 +97,9 @@ qemuExtDevicesPrepareDomain(virQEMUDriver *driver, int ret = 0; size_t i; + if (qemuExtDevicesInitPaths(driver, vm->def) < 0) + return -1; + for (i = 0; i < vm->def->nvideos; i++) { virDomainVideoDef *video = vm->def->videos[i]; @@ -134,9 +137,6 @@ qemuExtDevicesPrepareHost(virQEMUDriver *driver, virDomainDef *def = vm->def; size_t i; - if (qemuExtDevicesInitPaths(driver, def) < 0) - return -1; - for (i = 0; i < def->ntpms; i++) { virDomainTPMDef *tpm = def->tpms[i]; -- 2.37.4
This function is going to be called outside of qemu_extdevice.c. Expose it to the rest of the driver. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_extdevice.c | 2 +- src/qemu/qemu_extdevice.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 34454891f6..d5c3e8ed71 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -64,7 +64,7 @@ qemuExtDeviceLogCommand(virQEMUDriver *driver, * stored and we can remove directories and files in case of domain XML * changes. */ -static int +int qemuExtDevicesInitPaths(virQEMUDriver *driver, virDomainDef *def) { diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 86e7133a2a..d4ac9f395c 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -30,6 +30,11 @@ int qemuExtDeviceLogCommand(virQEMUDriver *driver, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; +int +qemuExtDevicesInitPaths(virQEMUDriver *driver, + virDomainDef *def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; + int qemuExtDevicesPrepareDomain(virQEMUDriver *driver, virDomainObj *vm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) -- 2.37.4
Paths for external devices (well, so far only vTPM) are not stored in the status XML. Therefore, we need to regenerate them after we've been restarted and reconnecting to a running domain. Otherwise these will remain NULL which may later lead to a NULL dereference. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2150760 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 19b9242623..c542be5036 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8896,6 +8896,9 @@ qemuProcessReconnect(void *opaque) if (qemuDomainMasterKeyReadFile(priv) < 0) goto error; + if (qemuExtDevicesInitPaths(driver, obj->def) < 0) + goto error; + /* If we are connecting to a guest started by old libvirt there is no * allowReboot in status XML and we need to initialize it. */ qemuProcessPrepareAllowReboot(obj); -- 2.37.4
On a Monday in 2022, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (4): qemu_process: Document qemuProcessPrepare{Domain,Host}() order qemu_extdevice: Init paths in qemuExtDevicesPrepareDomain() qemu_extdevice: Expose qemuExtDevicesInitPaths() qemu: Init ext devices paths on reconnect
src/qemu/qemu_extdevice.c | 8 ++++---- src/qemu/qemu_extdevice.h | 5 +++++ src/qemu/qemu_process.c | 7 +++++++ 3 files changed, 16 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik