- Devel - libvirt List Archives

[PATCH v2] virsh: Provide completer for some pool-X-as commands
by Abhiram Tilak 23 Jul '24

23 Jul '24

Provides completers for auth-type and source-format commands for virsh pool-create-as and pool-define-as commands. Use Empty completers for options where completions are not required. Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9 Signed-off-by: Abhiram Tilak <atp.exp(a)gmail.com> --- Changes in v2: - Fix all formatting errors - Change some options using Empty completers, to use LocalPath completers. - Add completers for AdapterName and AdapterParent using information from node devices. src/libvirt_private.syms | 2 + tools/virsh-completer-pool.c | 128 +++++++++++++++++++++++++++++++++++ tools/virsh-completer-pool.h | 20 ++++++ tools/virsh-pool.c | 9 +++ 4 files changed, 159 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f0f7aa8654..fcb0ef7afe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1117,6 +1117,8 @@ virStorageAuthDefCopy; virStorageAuthDefFormat; virStorageAuthDefFree; virStorageAuthDefParse; +virStorageAuthTypeFromString; +virStorageAuthTypeToString; virStorageFileFeatureTypeFromString; virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c index 3568bb985b..7db2a20347 100644 --- a/tools/virsh-completer-pool.c +++ b/tools/virsh-completer-pool.c @@ -23,6 +23,7 @@ #include "virsh-completer-pool.h" #include "virsh-util.h" #include "conf/storage_conf.h" +#include "conf/node_device_conf.h" #include "virsh-pool.h" #include "virsh.h" @@ -106,3 +107,130 @@ virshPoolTypeCompleter(vshControl *ctl, return virshCommaStringListComplete(type_str, (const char **)tmp); } + + +char ** +virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + size_t i = 0; + size_t j = 0; + g_auto(GStrv) tmp = NULL; + size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST + + VIR_STORAGE_POOL_DISK_LAST + VIR_STORAGE_POOL_LOGICAL_LAST; + + virCheckFlags(0, NULL); + + tmp = g_new0(char *, nformats + 1); + + /* Club all PoolFormats for completion */ + for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i)); + + for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i)); + + for (i = 1; i < VIR_STORAGE_POOL_DISK_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i)); + + for (i = 1; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++) + tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i)); + + return g_steal_pointer(&tmp); +} + + +char ** +virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + size_t i = 0; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1); + + for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++) + tmp[i] = g_strdup(virStorageAuthTypeToString(i)); + + return g_steal_pointer(&tmp); +} + + +char ** +virshAdapterNameCompleter(vshControl *ctl, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + virshControl *priv = ctl->privData; + virNodeDevicePtr *devs = NULL; + int ndevs = 0; + size_t i = 0; + char **ret = NULL; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST; + if ((ndevs = virConnectListAllNodeDevices(priv->conn, &devs, flags)) < 0) + return NULL; + + tmp = g_new0(char *, ndevs + 1); + + for (i = 0; i < ndevs; i++) { + const char *name = virNodeDeviceGetName(devs[i]); + + tmp[i] = g_strdup(name); + } + + ret = g_steal_pointer(&tmp); + + for (i = 0; i < ndevs; i++) + virshNodeDeviceFree(devs[i]); + g_free(devs); + return ret; +} + + +char ** +virshAdapterParentCompleter(vshControl *ctl, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + virshControl *priv = ctl->privData; + virNodeDevicePtr *devs = NULL; + int ndevs = 0; + size_t i = 0; + char **ret = NULL; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS; + if ((ndevs = virConnectListAllNodeDevices(priv->conn, &devs, flags)) < 0) + return NULL; + + tmp = g_new0(char *, ndevs + 1); + + for (i = 0; i < ndevs; i++) { + const char *name = virNodeDeviceGetName(devs[i]); + + tmp[i] = g_strdup(name); + } + + ret = g_steal_pointer(&tmp); + + for (i = 0; i < ndevs; i++) + virshNodeDeviceFree(devs[i]); + g_free(devs); + return ret; +} diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h index bff3e5742b..eccc08a73f 100644 --- a/tools/virsh-completer-pool.h +++ b/tools/virsh-completer-pool.h @@ -40,3 +40,23 @@ char ** virshPoolTypeCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** +virshPoolFormatCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshPoolAuthTypeCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshAdapterNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshAdapterParentCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index f9aad8ded0..0cbd1417e6 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -80,31 +80,37 @@ {.name = "source-path", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("source path for underlying storage") \ }, \ {.name = "source-dev", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("source device for underlying storage") \ }, \ {.name = "source-name", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompleteEmpty, \ .help = N_("source name for underlying storage") \ }, \ {.name = "target", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompletePathLocalExisting, \ .help = N_("target for underlying storage") \ }, \ {.name = "source-format", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshPoolFormatCompleter, \ .help = N_("format for underlying storage") \ }, \ {.name = "auth-type", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshPoolAuthTypeCompleter, \ .help = N_("auth type to be used for underlying storage") \ }, \ {.name = "auth-username", \ @@ -126,6 +132,7 @@ {.name = "adapter-name", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshAdapterNameCompleter, \ .help = N_("adapter name to be used for underlying storage") \ }, \ {.name = "adapter-wwnn", \ @@ -141,6 +148,7 @@ {.name = "adapter-parent", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshAdapterParentCompleter, \ .help = N_("adapter parent scsi_hostN to be used for underlying vHBA storage") \ }, \ {.name = "adapter-parent-wwnn", \ @@ -161,6 +169,7 @@ {.name = "source-protocol-ver", \ .type = VSH_OT_STRING, \ .unwanted_positional = true, \ + .completer = virshCompleteEmpty, \ .help = N_("nfsvers value for NFS pool mount option") \ }, \ {.name = "source-initiator", \ -- 2.39.2

3 2

[PATCH] qemuProcessStop: Don't unlock domain until most of the cleanup is done
by Michal Privoznik 23 Jul '24

23 Jul '24

Imagine two threads. Thread A is executing qemuProcessStop() and thread B is executing qemuDomainCreateXML(). To make things more interesting, the domain XML passed to qemuDomainCreateXML matches name + UUID of that the virDomainObj that qemuProcessStop() is currently working with. Here's what happens. 1) Thread A locks @vm, enters qemuProcessStop(). 2) Thread B parses given XML, calls virDomainObjListAdd() -> virDomainObjListAdd() -> virDomainObjListAddLocked() -> virDomainObjListFindByUUIDLocked(). Since there's a match on UUID, an virDomainObj object is returned and the thread proceeds to calling virObjectLock(). NB, it's the same object as in thread A. 3) Thread A sets vm->def->id = -1; this means that from this point on, virDomainObjIsActive() will return false. 4) Thread A calls qemuDomainObjStopWorker() which unlocks the @vm. 5) Thread B acquires the @vm lock and since virDomainObjIsActive() returns false, it proceeds to calling virDomainObjAssignDef() where vm->def is replaced. 6) Thread B then calls qemuProcessBeginJob() which unlocks the @vm temporarily. 7) Thread A, still in qemuDomainObjStopWorker() acquires @vm lock and proceeds with cleanup. 8) Thread A finds different definition than the one needing cleanup. In my testing I've seen stale pointers, e.g. vm->def->nets[0]->priv was NULL, which lead to a SIGSEGV as there's 'QEMU_DOMAIN_NETWORK_PRIVATE(net)->created' line when cleaning up nets. Your mileage may vary. Even if we did not crash, the plain fact that vm->def is changed in the middle of qemuProcessStop() means we might be cleaning up something else than intended. As a fix, I'm moving all lines that obviously touch vm->def before the domain object is unlocked. That left virHookCall(VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END) nearly next to virHookCall(VIR_HOOK_QEMU_OP_RELEASE, VIR_HOOK_SUBOP_END) which I figured is not something we want. So I've shuffled things a bit more. Fixes: 3865410e7f67ca4ec66e9a905e75f452762a97f0 Resolves: https://issues.redhat.com/browse/RHEL-49607 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_process.c | 122 ++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 25dfd04272..9ea6c678b8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8530,6 +8530,18 @@ void qemuProcessStop(virQEMUDriver *driver, VIR_QEMU_PROCESS_KILL_FORCE| VIR_QEMU_PROCESS_KILL_NOCHECK)); + vm->pid = 0; + + /* now that we know it's stopped call the hook if present */ + if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { + g_autofree char *xml = qemuDomainDefFormatXML(driver, NULL, vm->def, 0); + + /* we can't stop the operation even if the script raised an error */ + ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, + VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END, + NULL, xml, NULL)); + } + if (priv->agent) { g_clear_pointer(&priv->agent, qemuAgentClose); } @@ -8553,25 +8565,6 @@ void qemuProcessStop(virQEMUDriver *driver, qemuDBusStop(driver, vm); - /* Only after this point we can reset 'priv->beingDestroyed' so that - * there's no point at which the VM could be considered as alive between - * entering the destroy job and this point where the active "flag" is - * cleared. - */ - vm->def->id = -1; - priv->beingDestroyed = false; - - /* Wake up anything waiting on domain condition */ - virDomainObjBroadcast(vm); - - /* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent - * deadlocks with the per-VM event loop thread. This MUST be done after - * marking the VM as dead */ - qemuDomainObjStopWorker(vm); - - if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) - driver->inhibitCallback(false, driver->inhibitOpaque); - /* Clear network bandwidth */ virDomainClearNetBandwidth(vm->def); @@ -8588,18 +8581,6 @@ void qemuProcessStop(virQEMUDriver *driver, } } - virPortAllocatorRelease(priv->nbdPort); - priv->nbdPort = 0; - - if (priv->monConfig) { - if (priv->monConfig->type == VIR_DOMAIN_CHR_TYPE_UNIX) - unlink(priv->monConfig->data.nix.path); - g_clear_pointer(&priv->monConfig, virObjectUnref); - } - - /* Remove the master key */ - qemuDomainMasterKeyRemove(priv); - ignore_value(virDomainChrDefForeach(vm->def, false, qemuProcessCleanupChardevDevice, @@ -8609,22 +8590,6 @@ void qemuProcessStop(virQEMUDriver *driver, /* Its namespace is also gone then. */ qemuDomainDestroyNamespace(driver, vm); - virFileDeleteTree(priv->libDir); - virFileDeleteTree(priv->channelTargetDir); - - /* Stop autodestroy in case guest is restarted */ - virCloseCallbacksDomainRemove(vm, NULL, qemuProcessAutoDestroy); - - /* now that we know it's stopped call the hook if present */ - if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { - g_autofree char *xml = qemuDomainDefFormatXML(driver, NULL, vm->def, 0); - - /* we can't stop the operation even if the script raised an error */ - ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, - VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END, - NULL, xml, NULL)); - } - /* Reset Security Labels unless caller don't want us to */ if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL)) qemuSecurityRestoreAllLabel(driver, vm, @@ -8672,8 +8637,6 @@ void qemuProcessStop(virQEMUDriver *driver, virResctrlAllocRemove(vm->def->resctrls[i]->alloc); } - qemuProcessRemoveDomainStatus(driver, vm); - /* Remove VNC and Spice ports from port reservation bitmap, but only if they were reserved by the driver (autoport=yes) */ @@ -8706,20 +8669,9 @@ void qemuProcessStop(virQEMUDriver *driver, } } - for (i = 0; i < vm->ndeprecations; i++) - g_free(vm->deprecations[i]); - g_clear_pointer(&vm->deprecations, g_free); - vm->ndeprecations = 0; - vm->taint = 0; - vm->pid = 0; - virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); for (i = 0; i < vm->def->niothreadids; i++) vm->def->iothreadids[i]->thread_id = 0; - /* clean up a possible backup job */ - if (priv->backup) - qemuBackupJobTerminate(vm, VIR_DOMAIN_JOB_STATUS_CANCELED); - /* Do this explicitly after vm->pid is reset so that security drivers don't * try to enter the domain's namespace which is non-existent by now as qemu * is no longer running. */ @@ -8753,6 +8705,56 @@ void qemuProcessStop(virQEMUDriver *driver, qemuSecurityReleaseLabel(driver->securityManager, vm->def); + /* Only after this point we can reset 'priv->beingDestroyed' so that + * there's no point at which the VM could be considered as alive between + * entering the destroy job and this point where the active "flag" is + * cleared. + */ + vm->def->id = -1; + priv->beingDestroyed = false; + + /* Wake up anything waiting on domain condition */ + virDomainObjBroadcast(vm); + + /* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent + * deadlocks with the per-VM event loop thread. This MUST be done after + * marking the VM as dead */ + qemuDomainObjStopWorker(vm); + + if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) + driver->inhibitCallback(false, driver->inhibitOpaque); + + virPortAllocatorRelease(priv->nbdPort); + priv->nbdPort = 0; + + if (priv->monConfig) { + if (priv->monConfig->type == VIR_DOMAIN_CHR_TYPE_UNIX) + unlink(priv->monConfig->data.nix.path); + g_clear_pointer(&priv->monConfig, virObjectUnref); + } + + /* Remove the master key */ + qemuDomainMasterKeyRemove(priv); + + virFileDeleteTree(priv->libDir); + virFileDeleteTree(priv->channelTargetDir); + + /* Stop autodestroy in case guest is restarted */ + virCloseCallbacksDomainRemove(vm, NULL, qemuProcessAutoDestroy); + + qemuProcessRemoveDomainStatus(driver, vm); + + for (i = 0; i < vm->ndeprecations; i++) + g_free(vm->deprecations[i]); + g_clear_pointer(&vm->deprecations, g_free); + vm->ndeprecations = 0; + vm->taint = 0; + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); + + /* clean up a possible backup job */ + if (priv->backup) + qemuBackupJobTerminate(vm, VIR_DOMAIN_JOB_STATUS_CANCELED); + /* clear all private data entries which are no longer needed */ qemuDomainObjPrivateDataClear(priv); -- 2.44.2

3 4

[PATCH v2] virt-host-validate: Allow longer list of CPU flags
by Michal Privoznik 23 Jul '24

23 Jul '24

On various occasions, virt-host-validate parses /proc/cpuinfo to learn about CPU flags (see virHostValidateGetCPUFlags()). It does so, by reading the file line by line until the line with CPU flags is reached. Then the line is split into individual flags (using space as a delimiter) and the list of flags is then iterated over. This works, except for cases when the line with CPU flags is too long. Problem is - the line is capped at 1024 bytes and on newer CPUs (and newer kernels), the line can be significantly longer. I've seen a line that's ~1200 characters long (with 164 flags reported). Switch to unbounded read from the file (getline()). Resolves: https://issues.redhat.com/browse/RHEL-39969 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/LK74… diff to v2: - Keep trimming of the optional newline tools/virt-host-validate-common.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 591143c24d..63cc3dbe7b 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -106,21 +106,19 @@ virBitmap *virHostValidateGetCPUFlags(void) { FILE *fp; virBitmap *flags = NULL; + g_autofree char *line = NULL; + size_t linelen = 0; if (!(fp = fopen("/proc/cpuinfo", "r"))) return NULL; flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST); - do { - char line[1024]; + while (getline(&line, &linelen, fp) > 0) { char *start; g_auto(GStrv) tokens = NULL; GStrv next; - if (!fgets(line, sizeof(line), fp)) - break; - /* The line we're interested in is marked differently depending * on the architecture, so check possible prefixes */ if (!STRPREFIX(line, "flags") && @@ -129,11 +127,9 @@ virBitmap *virHostValidateGetCPUFlags(void) !STRPREFIX(line, "facilities")) continue; - /* fgets() includes the trailing newline in the output buffer, - * so we need to clean that up ourselves. We can safely access - * line[strlen(line) - 1] because the checks above would cause - * us to skip empty strings */ - line[strlen(line) - 1] = '\0'; + /* getline() may include the trailing newline in the output + * buffer, so we need to clean that up ourselves. */ + virStringTrimOptionalNewline(line); /* Skip to the separator */ if (!(start = strchr(line, ':'))) @@ -153,7 +149,7 @@ virBitmap *virHostValidateGetCPUFlags(void) if ((value = virHostValidateCPUFlagTypeFromString(*next)) >= 0) ignore_value(virBitmapSetBit(flags, value)); } - } while (1); + } VIR_FORCE_FCLOSE(fp); -- 2.44.2

2 1

[PATCH] virt-host-validate: Allow longer list of CPU flags
by Michal Privoznik 23 Jul '24

23 Jul '24

On various occasions, virt-host-validate parses /proc/cpuinfo to learn about CPU flags (see virHostValidateGetCPUFlags()). It does so, by reading the file line by line until the line with CPU flags is reached. Then the line is split into individual flags (using space as a delimiter) and the list of flags is then iterated over. This works, except for cases when the line with CPU flags is too long. Problem is - the line is capped at 1024 bytes and on newer CPUs (and newer kernels), the line can be significantly longer. I've seen a line that's ~1200 characters long (with 164 flags reported). Switch to unbounded read from the file (getline()). Resolves: https://issues.redhat.com/browse/RHEL-39969 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate-common.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 591143c24d..e5fa6606d2 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -106,21 +106,19 @@ virBitmap *virHostValidateGetCPUFlags(void) { FILE *fp; virBitmap *flags = NULL; + g_autofree char *line = NULL; + size_t linelen = 0; if (!(fp = fopen("/proc/cpuinfo", "r"))) return NULL; flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST); - do { - char line[1024]; + while (getline(&line, &linelen, fp) > 0) { char *start; g_auto(GStrv) tokens = NULL; GStrv next; - if (!fgets(line, sizeof(line), fp)) - break; - /* The line we're interested in is marked differently depending * on the architecture, so check possible prefixes */ if (!STRPREFIX(line, "flags") && @@ -129,12 +127,6 @@ virBitmap *virHostValidateGetCPUFlags(void) !STRPREFIX(line, "facilities")) continue; - /* fgets() includes the trailing newline in the output buffer, - * so we need to clean that up ourselves. We can safely access - * line[strlen(line) - 1] because the checks above would cause - * us to skip empty strings */ - line[strlen(line) - 1] = '\0'; - /* Skip to the separator */ if (!(start = strchr(line, ':'))) continue; @@ -153,7 +145,7 @@ virBitmap *virHostValidateGetCPUFlags(void) if ((value = virHostValidateCPUFlagTypeFromString(*next)) >= 0) ignore_value(virBitmapSetBit(flags, value)); } - } while (1); + } VIR_FORCE_FCLOSE(fp); -- 2.44.2

3 2

[PATCH] rpc: report error from filing to add timer
by Daniel P. Berrangé 22 Jul '24

22 Jul '24

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/rpc/virnetclientstream.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 98034d737d..380b785869 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -725,6 +725,8 @@ int virNetClientStreamEventAddCallback(virNetClientStream *st, virNetClientStreamEventTimer, st, virObjectUnref)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to add timer to event loop")); virObjectUnref(st); goto cleanup; } -- 2.45.2

2 1

cpugroups for hyperv hypervisor
by Praveen K Paladugu 22 Jul '24

22 Jul '24

Hey folks, My team is working on exposing `cpugroups` to Libvirt while using 'hyperv' hypervisor with cloud-hypervisor(VMM). cpugroups are relevant in a specific configuration of hyperv called 'minroot'. In Minroot configuration, hypervisor artificially restricts Dom0 to run on a subset of cpus (Logical Processors). The rest of the cpus can be assigned to guests. cpugroups manage the CPUs assigned to guests and their scheduling properties. Initially this looks similar to `cpuset` (in cgroups), but the controls available with cpugroups don't map easily to those in cgroups. For example: * "IdleLPs" are the number of Logical Processors in a cpugroup, that should be reserved to a guest even if they are idle * "SchedulingPriority", the priority(values between 0..7) with which to schedule CPUs in a cpugroup. As controls like above don't easily map to anything in cgroups, using a driver specific element in Domain xml, to configure cpugroups seems like a right approach. For example: <ch:cpugroups> <idle_lps value='4'/> <scheduling_priority value='6'/> </ch:cpugroups> As cpugroups is only relevant while using minroot configuration on hyperv, I don't see any value in generalizing this setting. So, having some "ch" driver specific settings seems like a good approach to implement this feature. Question1: Do you see any concerns with this approach? The cpugroup settings can be applied/modified using sysfs interface or using a cmdline tool on the host. I see Libvirt uses both these mechanisms for various use cases. But, given a choice, sysfs based interface seems like a simpler approach to me. With sysfs interface Libvirt does not have to take install time dependencies on new tools. Question2: Of "sysfs" vs "cmdline tool" which is preferred, given a choice? Early feedback from the community will help us invest in the preferred choices sooner than later. Thanks for your consideration. References: * https://learn.microsoft.com/en-us/windows-server/virtualization/hyper-v/man… -- Regards, Praveen

2 2

[PATCH] conf: virtiofs: add rlimit_nofile element
by Adam Julis 22 Jul '24

22 Jul '24

Add an element to configure the rlimit nofile size: ... <binary> <rlimit_nofile size='122333'/> </binary> ... Non-positive values are forbidden in 'domaincommon.rng'. Added separate test file, created by modifying the 'vhost-user-fs-fd-memory.xml'. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/485 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- docs/formatdomain.rst | 4 ++ src/conf/domain_conf.c | 12 +++++ src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 9 ++++ ...vhost-user-fs-fd-rlimit.x86_64-latest.args | 34 +++++++++++++ .../vhost-user-fs-fd-rlimit.x86_64-latest.xml | 50 +++++++++++++++++++ .../vhost-user-fs-fd-rlimit.xml | 50 +++++++++++++++++++ tests/qemuxmlconftest.c | 1 + 8 files changed, 161 insertions(+) create mode 100644 tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index ca33a2a570..10584dfe83 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3734,6 +3734,10 @@ A directory on the host that can be accessed directly from the guest. The thread pool helps increase the number of requests in flight when used with storage that has a higher latency. However, it has an overhead, and so for fast, low latency filesystems, it may be best to turn it off. ( :since:`Since 8.5.0` ) + Element ``rlimit_profile`` accepts one attribute ``size`` which defines the + maximum number of file descriptors. Non-positive values are forbidden. + Although numbers greater than 1M are allowed, the virtiofsd documentation + states that in this case its set by virtiofsd to the 1M. ( :since:`Since 10.6.0` ) ``source`` The resource on the host that is being accessed in the guest. The ``name`` attribute must be used with ``type='template'``, and the ``dir`` attribute diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 32df908d95..6733857a3a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8868,6 +8868,7 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *queue_size = virXPathString("string(./driver/@queue)", ctxt); g_autofree char *binary = virXPathString("string(./binary/@path)", ctxt); g_autofree char *thread_pool_size = virXPathString("string(./binary/thread_pool/@size)", ctxt); + g_autofree char *rlimit_nofile = virXPathString("string(./binary/rlimit_nofile/@size)", ctxt); xmlNodePtr binary_node = virXPathNode("./binary", ctxt); xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt); xmlNodePtr binary_cache_node = virXPathNode("./binary/cache", ctxt); @@ -8891,6 +8892,14 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, goto error; } + if (rlimit_nofile && + virStrToLong_ull(rlimit_nofile, NULL, 10, &def->rlimit_nofile) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse rlimit_nofile '%1$s' for virtiofs"), + rlimit_nofile); + goto error; + } + if (binary) def->binary = virFileSanitizePath(binary); @@ -23415,6 +23424,9 @@ virDomainFSDefFormat(virBuffer *buf, if (def->thread_pool_size >= 0) virBufferAsprintf(&binaryBuf, "<thread_pool size='%d'/>\n", def->thread_pool_size); + if (def->rlimit_nofile > 0) + virBufferAsprintf(&binaryBuf, "<rlimit_nofile size='%lld'/>\n", def->rlimit_nofile); + } virDomainVirtioOptionsFormat(&driverAttrBuf, def->virtio); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0fcc4f1f9b..8283493dfc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -890,6 +890,7 @@ struct _virDomainFSDef { bool symlinksResolved; char *binary; unsigned long long queue_size; + unsigned long long rlimit_nofile; virTristateSwitch xattr; virDomainFSCacheMode cache; virTristateSwitch posix_lock; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 2d23fcf123..ab5374d5f0 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3380,6 +3380,15 @@ </optional> </element> </optional> + <optional> + <element name="rlimit_nofile"> + <optional> + <attribute name="size"> + <data type="positiveInteger"/> + </attribute> + </optional> + </element> + </optional> </interleave> </element> </define> diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args new file mode 100644 index 0000000000..b4c2e3fe98 --- /dev/null +++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args @@ -0,0 +1,34 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,acpi=off \ +-accel kvm \ +-cpu qemu64 \ +-m size=14680064k \ +-overcommit mem-lock=off \ +-smp 2,sockets=2,cores=1,threads=1 \ +-object '{"qom-type":"memory-backend-file","id":"ram-node0","mem-path":"/var/lib/libvirt/qemu/ram/-1-guest/ram-node0","share":true,"size":15032385536}' \ +-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \ +-uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-chardev socket,id=chr-vu-fs0,path=/var/lib/libvirt/qemu/domain--1-guest/fs0-fs.sock \ +-device '{"driver":"vhost-user-fs-pci","id":"fs0","chardev":"chr-vu-fs0","queue-size":1027,"tag":"mount_tag","bus":"pci.0","addr":"0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml new file mode 100644 index 0000000000..2983d3f275 --- /dev/null +++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml @@ -0,0 +1,50 @@ +<domain type='kvm'> + <name>guest</name> + <uuid>126f2720-6f8e-45ab-a886-ec9277079a67</uuid> + <memory unit='KiB'>14680064</memory> + <currentMemory unit='KiB'>14680064</currentMemory> + <memoryBacking> + <source type='file'/> + <access mode='shared'/> + </memoryBacking> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + <numa> + <cell id='0' cpus='0-1' memory='14680064' unit='KiB' memAccess='shared'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pci-root'/> + <filesystem type='mount' accessmode='passthrough'> + <driver type='virtiofs' queue='1027'/> + <binary xattr='on'> + <cache mode='always'/> + <sandbox mode='chroot'/> + <thread_pool size='16'/> + <rlimit_nofile size='122333'/> + </binary> + <idmap> + <uid start='0' target='100000' count='65535'/> + <gid start='0' target='100000' count='65535'/> + </idmap> + <source dir='/path'/> + <target dir='mount_tag'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </filesystem> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml new file mode 100644 index 0000000000..2983d3f275 --- /dev/null +++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml @@ -0,0 +1,50 @@ +<domain type='kvm'> + <name>guest</name> + <uuid>126f2720-6f8e-45ab-a886-ec9277079a67</uuid> + <memory unit='KiB'>14680064</memory> + <currentMemory unit='KiB'>14680064</currentMemory> + <memoryBacking> + <source type='file'/> + <access mode='shared'/> + </memoryBacking> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + <numa> + <cell id='0' cpus='0-1' memory='14680064' unit='KiB' memAccess='shared'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pci-root'/> + <filesystem type='mount' accessmode='passthrough'> + <driver type='virtiofs' queue='1027'/> + <binary xattr='on'> + <cache mode='always'/> + <sandbox mode='chroot'/> + <thread_pool size='16'/> + <rlimit_nofile size='122333'/> + </binary> + <idmap> + <uid start='0' target='100000' count='65535'/> + <gid start='0' target='100000' count='65535'/> + </idmap> + <source dir='/path'/> + <target dir='mount_tag'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </filesystem> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 389d31800b..2aaf6103db 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2814,6 +2814,7 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("launch-security-s390-pv", "s390x"); DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory"); + DO_TEST_CAPS_LATEST("vhost-user-fs-fd-rlimit"); DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages"); DO_TEST_CAPS_LATEST_PARSE_ERROR("vhost-user-fs-readonly"); -- 2.45.2

3 2

[PATCH v2 0/1] Expose availability of SEV-ES
by Takashi Kajinami 22 Jul '24

22 Jul '24

This introduces the new "model" field in sev elements so that clients can check whether SEV-ES, the 2nd generation of AMD SEV, is available in the taget hyprvisor. There is the maxESGuests field (along with the maxGuests field) but this field does not explain whether SEV-ES is actually enabled in KVM. Takashi Kajinami (1): Expose available AMD SEV models in domain capabilities Changes since v1: * Fixed one code path where available models are not added * Fixed missing update of "report" flag * Updated the documentation to explain the new model field in addition to the existing but undocumanted cpu0Id field Takashi Kajinami (1): Expose available AMD SEV models in domain capabilities docs/formatdomaincaps.rst | 5 ++ src/conf/domain_capabilities.c | 2 + src/conf/domain_capabilities.h | 1 + src/conf/domain_conf.c | 7 +++ src/conf/domain_conf.h | 8 ++++ src/qemu/qemu_capabilities.c | 84 +++++++++++++++++++++++++--------- 6 files changed, 85 insertions(+), 22 deletions(-) -- 2.43.0

2 3

[PATCH 0/2] Add Granite Rapids cpu model
by Tim Wiederhake 22 Jul '24

22 Jul '24

Also, update the sync tool with new features introduced in qemu. Tim Wiederhake (2): sync_qemu_models_i386.py: Add missing features cpu_map: Add GraniteRapids CPU model src/cpu_map/index.xml | 1 + src/cpu_map/meson.build | 1 + src/cpu_map/sync_qemu_models_i386.py | 5 + src/cpu_map/x86_GraniteRapids.xml | 200 ++++++++++++++++++ .../domaincapsdata/qemu_8.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_8.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_8.1.0.x86_64.xml | 1 + .../domaincapsdata/qemu_8.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_8.2.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_8.2.0.x86_64.xml | 1 + .../domaincapsdata/qemu_9.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_9.0.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_9.0.0.x86_64.xml | 1 + .../domaincapsdata/qemu_9.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_9.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_9.1.0.x86_64.xml | 1 + 16 files changed, 219 insertions(+) create mode 100644 src/cpu_map/x86_GraniteRapids.xml -- 2.43.0

2 3

[PATCH] cpu_map: Add libcpuinfo as optional data source
by Tim Wiederhake 22 Jul '24

22 Jul '24

This adds an option to use libcpuinfo [1] as data source for libvirt's list of x86 cpu features. This is purely optional and does not change the script's behavior if libcpuinfo is not installed. libcpuinfo is a cross-vendor, cross-architecture source for CPU related information that has the capability to replace libvirt's dependence on qemu's cpu feature list. [1] https://gitlab.com/twiederh/libcpuinfo Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com> --- src/cpu_map/libcpuinfo_aliases.xml | 75 +++++++++++++++++++++++++ src/cpu_map/sync_qemu_features_i386.py | 77 +++++++++++++++++++++++--- 2 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 src/cpu_map/libcpuinfo_aliases.xml diff --git a/src/cpu_map/libcpuinfo_aliases.xml b/src/cpu_map/libcpuinfo_aliases.xml new file mode 100644 index 0000000000..75d243fead --- /dev/null +++ b/src/cpu_map/libcpuinfo_aliases.xml @@ -0,0 +1,75 @@ + + +<external_aliases> + <external_alias> + <type>feature</type> + <canonical>pclmulqdq</canonical> + <name>pclmuldq</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>ds-cpl</canonical> + <name>ds_cpl</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>sse4-1</canonical> + <name>sse4.1</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>sse4-2</canonical> + <name>sse4.2</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>tsc-adjust</canonical> + <name>tsc_adjust</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>lahf-lm</canonical> + <name>lahf_lm</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>cmp-legacy</canonical> + <name>cmp_legacy</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>nodeid-msr</canonical> + <name>nodeid_msr</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>perfctr-core</canonical> + <name>perfctr_core</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>perfctr-nb</canonical> + <name>perfctr_nb</name> + <domain>libvirt</domain> + </external_alias> + <external_alias> + <type>feature</type> + <canonical>fxsr-opt</canonical> + <name>fxsr_opt</name> + <domain>libvirt</domain> + </external_alias> +</external_aliases> diff --git a/src/cpu_map/sync_qemu_features_i386.py b/src/cpu_map/sync_qemu_features_i386.py index e4b1f7275a..3b3ad5a643 100755 --- a/src/cpu_map/sync_qemu_features_i386.py +++ b/src/cpu_map/sync_qemu_features_i386.py @@ -4,6 +4,11 @@ import argparse import os import re +try: + import pycpuinfo +except ImportError: + pycpuinfo = None + # features in qemu that we do not want in libvirt FEATURES_IGNORE = ( @@ -22,6 +27,7 @@ FEATURES_IGNORE = ( "kvm-steal-time", "kvmclock", "kvmclock-stable-bit", + "kvmclock2", "xstore", "xstore-en", @@ -295,6 +301,53 @@ def add_feature_qemu(query, data): add_feature_cpuid(eax, ecx, reg, bit, name) +def add_features_cpuinfo(): + def decode_bit(value): + for i in range(0, 64): + if value == (1 << i): + return i + + def decode_cpuid(v): + if v[0] != 0 and v[1] == 0 and v[2] == 0 and v[3] == 0: + reg, val = "eax", v[0] + if v[0] == 0 and v[1] != 0 and v[2] == 0 and v[3] == 0: + reg, val = "ebx", v[1] + if v[0] == 0 and v[1] == 0 and v[2] != 0 and v[3] == 0: + reg, val = "ecx", v[2] + if v[0] == 0 and v[1] == 0 and v[2] == 0 and v[3] != 0: + reg, val = "edx", v[3] + + return reg, decode_bit(val) + + x86 = pycpuinfo.Family.find("x86", "") + + for feature in pycpuinfo.features(): + if feature.family() != x86: + continue + + if list(feature.features()): + continue + + name = feature.name("libvirt") + if name in FEATURES_IGNORE: + continue + + cpuid = feature.extra_x86_cpuid() + if cpuid: + eax = cpuid[0] + ecx = cpuid[1] + if ecx == pycpuinfo.x86.CPUINFO_X86_CPUID_ECX_NONE: + ecx = None + reg, bit = decode_cpuid(cpuid[2:]) + add_feature_cpuid(eax, ecx, reg, bit, name) + + msr = feature.extra_x86_msr() + if msr: + index = msr[0] + bit = decode_bit(msr[1] | (msr[2] << 32)) + add_feature_msr(index, bit, name) + + # read the `feature_word_info` struct from qemu's cpu.c into a list of strings def read_cpu_c(path): pattern_comment = re.compile("/\\*.*?\\*/") @@ -450,6 +503,12 @@ def main(): nargs="?", type=os.path.realpath, ) + if pycpuinfo: + parser.add_argument( + "--libcpuinfo", + help="Use libcpuinfo as data source instead", + action="store_true", + ) parser.add_argument( "--output", "-o", @@ -459,14 +518,18 @@ def main(): ) args = parser.parse_args() - if not os.path.isdir(args.qemu): - parser.print_help() - exit("qemu source directory not found") + if pycpuinfo and args.libcpuinfo: + add_features_cpuinfo() + else: + if not os.path.isdir(args.qemu): + parser.print_help() + exit("qemu source directory not found") + + read_headers(args.qemu) + lines = read_cpu_c(args.qemu) + parse_feature_words(lines) + add_extra_features() - read_headers(args.qemu) - lines = read_cpu_c(args.qemu) - parse_feature_words(lines) - add_extra_features() write_output(args.output) print( -- 2.43.0

2 2