July 2024 - Devel - libvirt List Archives

[PATCH] qemu: add a monitor to /proc/$pid when killing times out
by Boris Fiuczynski 17 Jul '24

17 Jul '24

In cases when a QEMU process takes longer than the time sigterm and sigkill are issued to kill the process do not simply fail and leave the VM in state VIR_DOMAIN_SHUTDOWN until the daemon stops. Instead set up an fd on /proc/$pid and get notified when the QEMU process finally has terminated to cleanup the VM state. Resolves: https://issues.redhat.com/browse/RHEL-28819 Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> --- src/qemu/qemu_domain.c | 8 +++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 18 ++++++ src/qemu/qemu_process.c | 127 ++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_process.h | 1 + 5 files changed, 151 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2134b11038..96f4e41a11 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1889,6 +1889,11 @@ qemuDomainObjPrivateFree(void *data) virChrdevFree(priv->devs); + if (priv->watchPid >= 0) { + virEventRemoveHandle(priv->watchPid); + priv->watchPid = -1; + } + /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion")); @@ -1934,6 +1939,8 @@ qemuDomainObjPrivateAlloc(void *opaque) priv->blockjobs = virHashNew(virObjectUnref); priv->fds = virHashNew(g_object_unref); + priv->watchPid = -1; + /* agent commands block by default, user can choose different behavior */ priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK; priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; @@ -11680,6 +11687,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_RESET: case QEMU_PROCESS_EVENT_NBDKIT_EXITED: case QEMU_PROCESS_EVENT_MONITOR_EOF: + case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d777559119..e5366c6e8c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -119,6 +119,7 @@ struct _qemuDomainObjPrivate { bool beingDestroyed; char *pidfile; + int watchPid; virDomainPCIAddressSet *pciaddrs; virDomainUSBAddressSet *usbaddrs; @@ -469,6 +470,7 @@ typedef enum { QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION, QEMU_PROCESS_EVENT_RESET, QEMU_PROCESS_EVENT_NBDKIT_EXITED, + QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9f3013e231..6b1e4084f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4041,6 +4041,21 @@ processNbdkitExitedEvent(virDomainObj *vm, } +static void +processShutdownCompletedEvent(virQEMUDriver *driver, + virDomainObj *vm) +{ + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + return; + + if (virDomainObjIsActive(vm)) + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN, + VIR_ASYNC_JOB_NONE, 0); + + virDomainObjEndJob(vm); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4101,6 +4116,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_NBDKIT_EXITED: processNbdkitExitedEvent(vm, processEvent->data); break; + case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: + processShutdownCompletedEvent(driver, vm); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 25dfd04272..d6dbd7ba53 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,6 +25,7 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#include <sys/syscall.h> #if defined(__linux__) # include <linux/capability.h> #elif defined(__FreeBSD__) @@ -8387,9 +8388,119 @@ qemuProcessCreatePretendCmdBuild(virDomainObj *vm, } +typedef struct { + virDomainObj *vm; + int pidfd; +} qemuProcessInShutdownEventData; + + +static qemuProcessInShutdownEventData* +qemuProcessInShutdownEventDataNew(virDomainObj *vm, int pidfd) +{ + qemuProcessInShutdownEventData *d = g_new(qemuProcessInShutdownEventData, 1); + d->vm = virObjectRef(vm); + d->pidfd = pidfd; + return d; +} + + +static void +qemuProcessInShutdownEventDataFree(qemuProcessInShutdownEventData *d) +{ + virObjectUnref(d->vm); + VIR_FORCE_CLOSE(d->pidfd); + g_free(d); +} + + +static void +qemuProcessInShutdownStopMonitor(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + + VIR_DEBUG("vm=%p name=%s pid=%lld watchPid=%d", + vm, vm->def->name, (long long)vm->pid, + priv->watchPid); + + virObjectLock(vm); + if (priv->watchPid >= 0) { + virEventRemoveHandle(priv->watchPid); + priv->watchPid = -1; + } + virObjectUnlock(vm); +} + + +static void +qemuProcessInShutdownPidfdCb(int watch G_GNUC_UNUSED, + int fd, + int events G_GNUC_UNUSED, + void *opaque) +{ + qemuProcessInShutdownEventData *data = opaque; + virDomainObj *vm = data->vm; + + VIR_DEBUG("vm=%p name=%s pid=%lld fd=%d", + vm, vm->def->name, (long long)vm->pid, fd); + + VIR_DEBUG("QEMU process %lld finally completed termination", + (long long)vm->pid); + qemuProcessInShutdownStopMonitor(vm); + + qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED, + 0, 0, NULL); +} + + +static int +qemuProcessInShutdownStartMonitor(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + qemuProcessInShutdownEventData *data; + int pidfd; + int ret = -1; + + VIR_DEBUG("vm=%p name=%s pid=%lld watchPid=%d", + vm, vm->def->name, (long long)vm->pid, + priv->watchPid); + + if (priv->watchPid >= 0) { + VIR_DEBUG("Monitoring qemu in-shutdown process %i already set up", vm->pid); + goto cleanup; + } + + pidfd = syscall(SYS_pidfd_open, vm->pid, 0); + if (pidfd < 0) { + if (errno == ESRCH) /* process has already terminated */ + ret = 1; + goto cleanup; + } + + data = qemuProcessInShutdownEventDataNew(vm, pidfd); + if ((priv->watchPid = virEventAddHandle(pidfd, + VIR_EVENT_HANDLE_READABLE, + qemuProcessInShutdownPidfdCb, + data, + (virFreeCallback)qemuProcessInShutdownEventDataFree)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to monitor qemu in-shutdown process %1$i"), + vm->pid); + qemuProcessInShutdownEventDataFree(data); + goto cleanup; + } + VIR_DEBUG("Monitoring qemu in-shutdown process %i for termination", vm->pid); + ret = 0; + + cleanup: + return ret; +} + + int qemuProcessKill(virDomainObj *vm, unsigned int flags) { + int ret = -1; + VIR_DEBUG("vm=%p name=%s pid=%lld flags=0x%x", vm, vm->def->name, (long long)vm->pid, flags); @@ -8410,10 +8521,16 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags) /* Request an extra delay of two seconds per current nhostdevs * to be safe against stalls by the kernel freeing up the resources */ - return virProcessKillPainfullyDelay(vm->pid, - !!(flags & VIR_QEMU_PROCESS_KILL_FORCE), - vm->def->nhostdevs * 2, - false); + ret = virProcessKillPainfullyDelay(vm->pid, + !!(flags & VIR_QEMU_PROCESS_KILL_FORCE), + vm->def->nhostdevs * 2, + false); + + if (ret < 0 && (flags & VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR)) + if (qemuProcessInShutdownStartMonitor(vm) == 1) + ret = 0; /* process termination detected */ + + return ret; } @@ -8438,7 +8555,7 @@ qemuProcessBeginStopJob(virDomainObj *vm, * cleared inside qemuProcessStop */ priv->beingDestroyed = true; - if (qemuProcessKill(vm, killFlags) < 0) + if (qemuProcessKill(vm, killFlags|VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR) < 0) goto error; /* Wake up anything waiting on domain condition */ diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index cb67bfcd2d..2324aeb7bd 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -180,6 +180,7 @@ typedef enum { VIR_QEMU_PROCESS_KILL_FORCE = 1 << 0, VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1, VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* bypass the running vm check */ + VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR = 1 << 3, /* on error enable process monitor */ } virQemuProcessKillMode; int qemuProcessKill(virDomainObj *vm, unsigned int flags); -- 2.45.0

1 0

[PATCH] qemu: virtiofs: format --rlimit-nofile
by Adam Julis 17 Jul '24

17 Jul '24

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/485 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/qemu/qemu_virtiofs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index 0e3c7dbb58..703f1226a2 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -194,6 +194,9 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfig *cfg, if (fs->thread_pool_size >= 0) virCommandAddArgFormat(cmd, "--thread-pool-size=%i", fs->thread_pool_size); + if (fs->rlimit_nofile > 0) + virCommandAddArgFormat(cmd, "--rlimit-nofile=%llu", fs->rlimit_nofile); + if (cfg->virtiofsdDebug) { if (virBitmapIsBitSet(fs->caps, QEMU_VHOST_USER_FS_FEATURE_SEPARATE_OPTIONS)) virCommandAddArgList(cmd, "--log-level", "debug", NULL); -- 2.45.2

2 1

[PATCH] vmx: Be even more lax when trying to comprehend serial ports
by Martin Kletzander 16 Jul '24

16 Jul '24

So much can happen in the fileName field of the VMX that the easiest thing is to silently report a serial type="null". This effectively reverts commits de81bdb8d4cd and 62c53db0421a, but keeps the test files to show the fix is still in place. There is one instance where an error gets reset, but since that is a rare case on its own and on top of that does not happen in any of our long-running daemons with a logfile that might get monitored it should be fine to leave it there. Resolves: https://issues.redhat.com/browse/RHEL-32182 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/vmx/vmx.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index e5bc2d793c66..227744d06258 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2975,9 +2975,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, char fileName_name[48] = ""; g_autofree char *fileName = NULL; - char vspc_name[48] = ""; - g_autofree char *vspc = NULL; - char network_endPoint_name[48] = ""; g_autofree char *network_endPoint = NULL; @@ -3000,7 +2997,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, VMX_BUILD_NAME(startConnected); VMX_BUILD_NAME(fileType); VMX_BUILD_NAME(fileName); - VMX_BUILD_NAME(vspc); VMX_BUILD_NAME_EXTRA(network_endPoint, "network.endPoint"); /* vmx:present */ @@ -3030,10 +3026,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, if (virVMXGetConfigString(conf, fileName_name, &fileName, true) < 0) goto cleanup; - /* vmx:fileName -> def:data.file.path */ - if (virVMXGetConfigString(conf, vspc_name, &vspc, true) < 0) - goto cleanup; - /* vmx:network.endPoint -> def:data.tcp.listen */ if (virVMXGetConfigString(conf, network_endPoint_name, &network_endPoint, true) < 0) { @@ -3065,21 +3057,25 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, (*def)->target.port = port; (*def)->source->type = VIR_DOMAIN_CHR_TYPE_PIPE; (*def)->source->data.file.path = g_steal_pointer(&fileName); - } else if (STRCASEEQ(fileType, "network") && (vspc || !fileName || STREQ(fileName, ""))) { - (*def)->target.port = port; - (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL; } else if (STRCASEEQ(fileType, "network")) { (*def)->target.port = port; (*def)->source->type = VIR_DOMAIN_CHR_TYPE_TCP; - if (!(parsedUri = virURIParse(fileName))) - goto cleanup; + if (!(parsedUri = virURIParse(fileName))) { + /* + * Ignore anything we cannot parse since there are many variations + * that could lead to unusable or non-representable serial ports + * which are very commonly seen and the main consumer of this driver + * (virt-v2v) ignores them anyway, so let's at least not error out. + */ + virResetLastError(); + (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL; + return 0; + } if (parsedUri->port == 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("VMX entry '%1$s' doesn't contain a port part"), - fileName_name); - goto cleanup; + (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL; + return 0; } (*def)->source->data.tcp.host = g_strdup(parsedUri->server); -- 2.45.1

2 1

[PATCH] ci: Refresh generated files
by Michal Privoznik 15 Jul '24

15 Jul '24

This is supposed to unstuck FreeBSD as it switched to Python-3.11. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- ci/cirrus/freebsd-13.vars | 2 +- ci/cirrus/freebsd-14.vars | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/cirrus/freebsd-13.vars b/ci/cirrus/freebsd-13.vars index 278b5834f6..f24b921300 100644 --- a/ci/cirrus/freebsd-13.vars +++ b/ci/cirrus/freebsd-13.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='augeas bash-completion ca_root_nss ccache codespell cppi curl cyrus-sasl diffutils fusefs-libs gettext git glib gmake gnugrep gnutls gsed libpcap libpciaccess libssh libssh2 libxml2 libxslt meson ninja perl5 pkgconf polkit py39-black py39-docutils py39-flake8 py39-pytest python3 qemu readline yajl' +PKGS='augeas bash-completion ca_root_nss ccache codespell cppi curl cyrus-sasl diffutils fusefs-libs gettext git glib gmake gnugrep gnutls gsed libpcap libpciaccess libssh libssh2 libxml2 libxslt meson ninja perl5 pkgconf polkit py311-black py311-docutils py311-flake8 py311-pytest python3 qemu readline yajl' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' diff --git a/ci/cirrus/freebsd-14.vars b/ci/cirrus/freebsd-14.vars index 278b5834f6..f24b921300 100644 --- a/ci/cirrus/freebsd-14.vars +++ b/ci/cirrus/freebsd-14.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='augeas bash-completion ca_root_nss ccache codespell cppi curl cyrus-sasl diffutils fusefs-libs gettext git glib gmake gnugrep gnutls gsed libpcap libpciaccess libssh libssh2 libxml2 libxslt meson ninja perl5 pkgconf polkit py39-black py39-docutils py39-flake8 py39-pytest python3 qemu readline yajl' +PKGS='augeas bash-completion ca_root_nss ccache codespell cppi curl cyrus-sasl diffutils fusefs-libs gettext git glib gmake gnugrep gnutls gsed libpcap libpciaccess libssh libssh2 libxml2 libxslt meson ninja perl5 pkgconf polkit py311-black py311-docutils py311-flake8 py311-pytest python3 qemu readline yajl' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' -- 2.44.2

2 1

[PATCH 0/2] qemu: Add support for pauth Arm CPU feature
by Andrea Bolognani 15 Jul '24

15 Jul '24

Andrea Bolognani (2): cpu_map: Add pauth Arm CPU feature tests: Add coverage for pauth Arm CPU feature src/cpu_map/arm_features.xml | 3 ++ ...aarch64-features-pauth.aarch64-latest.args | 31 +++++++++++++++++++ .../aarch64-features-pauth.aarch64-latest.xml | 28 +++++++++++++++++ .../aarch64-features-pauth.xml | 17 ++++++++++ tests/qemuxmlconftest.c | 1 + 5 files changed, 80 insertions(+) create mode 100644 tests/qemuxmlconfdata/aarch64-features-pauth.aarch64-latest.args create mode 100644 tests/qemuxmlconfdata/aarch64-features-pauth.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/aarch64-features-pauth.xml -- 2.45.2

2 7

[PATCH] docs: use real examples for QEMU cli passthrough
by Daniel P. Berrangé 15 Jul '24

15 Jul '24

User feedback has shown that the examples are not clear enough to illustrate the cli passthrough concept in action. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- docs/drvqemu.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/drvqemu.rst b/docs/drvqemu.rst index 1ee4b1e366..e52d9ffd47 100644 --- a/docs/drvqemu.rst +++ b/docs/drvqemu.rst @@ -595,8 +595,11 @@ Example: <emulator>/usr/bin/qemu-system-x86_64</emulator> </devices> <qemu:commandline> - <qemu:arg value='-newarg'/> - <qemu:env name='QEMU_ENV' value='VAL'/> + <qemu:arg value='-device'/> + <qemu:arg value='ipmi-bmc-sim,id=bmc0'/> + <qemu:arg value='-device'/> + <qemu:arg value='smbus-impi,bmc=bmc0'/> + <qemu:env name='QEMU_MODULE_DIR' value='/usr/lib64/qemu'/> </qemu:commandline> </domain> -- 2.45.1

2 1

[PATCH v3 00/17] hw/sd/sdcard: Accumulation of cleanups and fixes
by Philippe Mathieu-Daudé 11 Jul '24

11 Jul '24

Since v2: - Tested-by from Cédric recorded - more patches added :S Since v1: - various patches merged, few more added Various SD card cleanups and fixes accumulated over the years. Various have been useful to help integrating eMMC support (which will come later). Full series for testing: https://gitlab.com/philmd/qemu/-/tags/emmc-v4 Cédric Le Goater (1): hw/sd/sdcard: Introduce definitions for EXT_CSD register Philippe Mathieu-Daudé (16): hw/sd/sdcard: Deprecate support for spec v1.10 hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Track last command used to help logging hw/sd/sdcard: Trace block offset in READ/WRITE data accesses hw/sd/sdcard: Trace requested address computed by sd_req_get_address() hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/sd/sdcard: Send WRITE_PROT bits MSB first (CMD30) hw/sd/sdcard: Send NUM_WR_BLOCKS bits MSB first (ACMD22) hw/sd/sdcard: Use READY_FOR_DATA definition instead of magic value hw/sd/sdcard: Assign SDCardStates enum values hw/sd/sdcard: Simplify sd_inactive_state handling hw/sd/sdcard: Restrict SWITCH_FUNCTION to sd_transfer_state (CMD6) hw/sd/sdcard: Add direct reference to SDProto in SDState hw/sd/sdcard: Extract sd_blk_len() helper tests/qtest: Disable npcm7xx_sdhci tests using hardcoded RCA hw/sd/sdcard: Generate random RCA value docs/about/deprecated.rst | 6 ++ hw/sd/sdmmc-internal.h | 97 +++++++++++++++++++++ hw/sd/sd.c | 145 ++++++++++++++++++------------- tests/qtest/npcm7xx_sdhci-test.c | 7 ++ hw/sd/trace-events | 6 +- 5 files changed, 199 insertions(+), 62 deletions(-) -- 2.41.0

4 34

[PATCH] qemu: Don't leave beingDestroyed=true on inactive domain
by Jiri Denemark 11 Jul '24

11 Jul '24

Recent commit v10.4.0-87-gd9935a5c4f made a reasonable change to only reset beingDestroyed back to false when vm->def->id is reset to make sure other code can detect a domain is (about to become) inactive. It even added a comment saying any caller of qemuProcessBeginStopJob is supposed to call qemuProcessStop to clear beingDestroyed. But not every caller really does so because they first call qemuProcessBeginStopJob and then check whether a domain is still running. If not the qemuProcessStop call is skipped leaving beingDestroyed=true. In case of a persistent domain this may block incoming migrations of such domain as the migration code would think the domain died unexpectedly (even though it's still running). The qemuProcessBeginStopJob function is a wrapper around virDomainObjBeginJob, but virDomainObjEndJob was used directly for cleanup. This patch introduces a new qemuProcessEndStopJob wrapper around virDomainObjEndJob to properly undo everything qemuProcessBeginStopJob did. https://issues.redhat.com/browse/RHEL-43309 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_process.c | 20 ++++++++++++++++---- src/qemu/qemu_process.h | 1 + 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f898d85667..9f3013e231 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2102,7 +2102,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, endjob: if (ret == 0) qemuDomainRemoveInactive(driver, vm, 0, false); - virDomainObjEndJob(vm); + qemuProcessEndStopJob(vm); cleanup: virDomainObjEndAPI(&vm); @@ -3888,7 +3888,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, endjob: qemuDomainRemoveInactive(driver, vm, 0, false); - virDomainObjEndJob(vm); + qemuProcessEndStopJob(vm); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7cbe521a6e..25dfd04272 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8421,7 +8421,8 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags) * qemuProcessBeginStopJob: * * Stop all current jobs by killing the domain and start a new one for - * qemuProcessStop. + * qemuProcessStop. The caller has to make sure qemuProcessEndStopJob is + * called to properly cleanup the job. */ int qemuProcessBeginStopJob(virDomainObj *vm, @@ -8448,8 +8449,9 @@ qemuProcessBeginStopJob(virDomainObj *vm, goto error; /* priv->beingDestroyed is deliberately left set to 'true' here. Caller - * is supposed to call qemuProcessStop, which will reset it after - * 'vm->def->id' is set to -1 */ + * is supposed to call qemuProcessStop (which will reset it after + * 'vm->def->id' is set to -1) and/or qemuProcessEndStopJob to do proper + * cleanup. */ return 0; error: @@ -8458,6 +8460,16 @@ qemuProcessBeginStopJob(virDomainObj *vm, } +void +qemuProcessEndStopJob(virDomainObj *vm) +{ + if (!virDomainObjIsActive(vm)) + QEMU_DOMAIN_PRIVATE(vm)->beingDestroyed = false; + + virDomainObjEndJob(vm); +} + + void qemuProcessStop(virQEMUDriver *driver, virDomainObj *vm, virDomainShutoffReason reason, @@ -8800,7 +8812,7 @@ qemuProcessAutoDestroy(virDomainObj *dom, qemuDomainRemoveInactive(driver, dom, 0, false); - virDomainObjEndJob(dom); + qemuProcessEndStopJob(dom); virObjectEventStateQueue(driver->domainEventState, event); } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index c1ea949215..cb67bfcd2d 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -169,6 +169,7 @@ typedef enum { int qemuProcessBeginStopJob(virDomainObj *vm, virDomainJob job, bool forceKill); +void qemuProcessEndStopJob(virDomainObj *vm); void qemuProcessStop(virQEMUDriver *driver, virDomainObj *vm, virDomainShutoffReason reason, -- 2.45.2

2 1

[PATCH] virt-host-validate: Rework calling of driver validation
by Michal Privoznik 11 Jul '24

11 Jul '24

It all started with me looking at the --help output which also printed "bhyve" as supported hypervisor type. Well, it's not on my Linux machine. To resolve this, I'm just creating a static array of { "$driver", callback() } pairs and iterating over it. The array is then initialized at compile time with supported drivers. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate.c | 80 +++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/tools/virt-host-validate.c b/tools/virt-host-validate.c index 426648a5d3..365b8acd92 100644 --- a/tools/virt-host-validate.c +++ b/tools/virt-host-validate.c @@ -29,6 +29,7 @@ #include "internal.h" #include "virerror.h" #include "virgettext.h" +#include "virglibutil.h" #include "virt-host-validate-common.h" #if WITH_QEMU @@ -44,26 +45,58 @@ # include "virt-host-validate-ch.h" #endif +typedef struct _virValidateCallbacks virValidateCallbacks; +struct _virValidateCallbacks { + const char *name; + int (*callback)(void); +}; + +static virValidateCallbacks validateCallbacks[] = { +#if WITH_QEMU + { "qemu", virHostValidateQEMU }, +#endif +#if WITH_LXC + { "lxc", virHostValidateLXC }, +#endif +#if WITH_BHYVE + { "bhyve", virHostValidateBhyve }, +#endif +#if WITH_CH + { "ch", virHostValidateCh }, +#endif +}; + static void show_help(FILE *out, const char *argv0) { + g_autofree char *hvs = NULL; + char *hvs_list[G_N_ELEMENTS(validateCallbacks) + 1] = { }; + size_t i; + + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + hvs_list[i] = g_strdup_printf(" - %1$s", validateCallbacks[i].name); + } + + hvs = g_strjoinv("\n", hvs_list); + + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + g_free(hvs_list[i]); + } + fprintf(out, _("\n" "syntax: %1$s [OPTIONS] [HVTYPE]\n" "\n" " Hypervisor types:\n" "\n" - " - qemu\n" - " - lxc\n" - " - bhyve\n" - " - ch\n" + "%2$s\n" "\n" " Options:\n" " -h, --help Display command line help\n" " -v, --version Display command version\n" " -q, --quiet Don't display progress information\n" "\n"), - argv0); + argv0, hvs); } static void @@ -87,6 +120,7 @@ main(int argc, char **argv) int ret = EXIT_SUCCESS; bool quiet = false; bool usedHvname = false; + size_t i; if (virGettextInitialize() < 0 || virErrorInitialize() < 0) { @@ -126,37 +160,13 @@ main(int argc, char **argv) virValidateSetQuiet(quiet); -#if WITH_QEMU - if (!hvname || STREQ(hvname, "qemu")) { - usedHvname = true; - if (virHostValidateQEMU() < 0) - ret = EXIT_FAILURE; + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + if (!hvname || STREQ(hvname, validateCallbacks[i].name)) { + usedHvname = true; + if (validateCallbacks[i].callback() < 0) + ret = EXIT_FAILURE; + } } -#endif - -#if WITH_LXC - if (!hvname || STREQ(hvname, "lxc")) { - usedHvname = true; - if (virHostValidateLXC() < 0) - ret = EXIT_FAILURE; - } -#endif - -#if WITH_BHYVE - if (!hvname || STREQ(hvname, "bhyve")) { - usedHvname = true; - if (virHostValidateBhyve() < 0) - ret = EXIT_FAILURE; - } -#endif - -#if WITH_CH - if (!hvname || STREQ(hvname, "ch")) { - usedHvname = true; - if (virHostValidateCh() < 0) - ret = EXIT_FAILURE; - } -#endif if (hvname && !usedHvname) { fprintf(stderr, _("%1$s: unsupported hypervisor name %2$s\n"), -- 2.44.2

2 1

[PATCH] virt-host-validate: Drop extra "PASS"
by Michal Privoznik 11 Jul '24

11 Jul '24

If virt-host-validate is ran on a SEV-SNP capable machine, an extra "PASS" is printed out. This is because virHostValidateAMDSev() prints "PASS" and then returns 1 (indicating success) which in turn makes the caller (virHostValidateSecureGuests()) print "PASS" again. Just drop the extra printing in the caller and let virHostValidateAMDSev() do all the printing. Fixes: 1a8f646f291775d2423ce4e4df62ad69f06ab827 Resolves: https://issues.redhat.com/browse/RHEL-46868 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate-common.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index a29a5b6d5f..86db4815c3 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -488,11 +488,7 @@ int virHostValidateSecureGuests(const char *hvname, return VIR_VALIDATE_FAILURE(level); } } else if (hasAMDSev) { - int rc = virHostValidateAMDSev(hvname, level); - - if (rc > 0) - virValidatePass(); - return rc; + return virHostValidateAMDSev(hvname, level); } virValidateFail(level, -- 2.44.2

2 1