June 2024 - Devel - libvirt List Archives

[PATCH RFC 0/9] qemu: Support mapped-ram migration capability
by Jim Fehlig 17 Aug '24

17 Aug '24

This series is a RFC for support of QEMU's mapped-ram migration capability [1] for saving and restoring VMs. It implements the first part of the design approach we discussed for supporting parallel save/restore [2]. In summary, the approach is 1. Add mapped-ram migration capability 2. Steal an element from save header 'unused' for a 'features' variable and bump save version to 3. 3. Add /etc/libvirt/qemu.conf knob for the save format version, defaulting to latest v3 4. Use v3 (aka mapped-ram) by default 5. Use mapped-ram with BYPASS_CACHE for v3, old approach for v2 6. include: Define constants for parallel save/restore 7. qemu: Add support for parallel save. Implies mapped-ram, reject if v2 8. qemu: Add support for parallel restore. Implies mapped-ram. Reject if v2 9. tools: add parallel parameter to virsh save command 10. tools: add parallel parameter to virsh restore command This series implements 1-5, with the BYPASS_CACHE support in patches 8 and 9 being quite hacky. They are included to discuss approaches to make them less hacky. See the patches for details. The QEMU mapped-ram capability currently does not support directio. Fabino is working on that now [3]. This complicates merging support in libvirt. I don't think it's reasonable to enable mapped-ram by default when BYPASS_CACHE cannot be supported. Should we wait until the mapped-ram directio support is merged in QEMU before supporting mapped-ram in libvirt? For the moment, compression is ignored in the new save version. Currently, libvirt connects the output of QEMU's save stream to the specified compression program via a pipe. This approach is incompatible with mapped-ram since the fd provided to QEMU must be seekable. One option is to reopen and compress the saved image after the actual save operation has completed. This has the downside of requiring the iohelper to handle BYPASS_CACHE, which would preclude us from removing it sometime in the future. Other suggestions much welcomed. Note the logical file size of mapped-ram saved images is slightly larger than guest RAM size, so the files are often much larger than the files produced by the existing, sequential format. However, actual blocks written to disk is often lower with mapped-ram saved images. E.g. a saved image from a 30G, freshly booted, idle guest results in the following 'Size' and 'Blocks' values reported by stat(1) Size Blocks sequential 998595770 1950392 mapped-ram 34368584225 1800456 With the same guest running a workload that dirties memory Size Blocks sequential 33173330615 64791672 mapped-ram 34368578210 64706944 Thanks for any comments on this RFC! [1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/map… [2] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/K4B… [3] https://mail.gnu.org/archive/html/qemu-devel/2024-05/msg04432.html Jim Fehlig (9): qemu: Enable mapped-ram migration capability qemu_fd: Add function to retrieve fdset ID qemu: Add function to get migration params for save qemu: Add a 'features' element to save image header and bump version qemu: conf: Add setting for save image version qemu: Add support for mapped-ram on save qemu: Enable mapped-ram on restore qemu: Support O_DIRECT with mapped-ram on save qemu: Support O_DIRECT with mapped-ram on restore src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 6 + src/qemu/qemu_conf.c | 8 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 25 ++-- src/qemu/qemu_fd.c | 18 +++ src/qemu/qemu_fd.h | 3 + src/qemu/qemu_migration.c | 99 ++++++++++++++- src/qemu/qemu_migration.h | 11 +- src/qemu/qemu_migration_params.c | 20 +++ src/qemu/qemu_migration_params.h | 4 + src/qemu/qemu_monitor.c | 40 ++++++ src/qemu/qemu_monitor.h | 5 + src/qemu/qemu_process.c | 63 +++++++--- src/qemu/qemu_process.h | 16 ++- src/qemu/qemu_saveimage.c | 187 +++++++++++++++++++++++------ src/qemu/qemu_saveimage.h | 20 ++- src/qemu/qemu_snapshot.c | 12 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + 19 files changed, 455 insertions(+), 85 deletions(-) -- 2.44.0

3 22

[PATCH v2 0/7] introduce job-change qmp command
by Vladimir Sementsov-Ogievskiy 02 Aug '24

02 Aug '24

Hi all! This is an updated first part of my "[RFC 00/15] block job API" Supersedes: <20240313150907.623462-1-vsementsov(a)yandex-team.ru> v2: - only job-change for now, as a first step - drop "type-based unions", and keep type parameter as is for now (I now doubt that this was good idea, as it makes QAPI protocol dependent on context) 03: improve documentation 06: deprecated only block-job-change for now 07: new Vladimir Sementsov-Ogievskiy (7): qapi: rename BlockJobChangeOptions to JobChangeOptions blockjob: block_job_change_locked(): check job type qapi: block-job-change: make copy-mode parameter optional blockjob: move change action implementation to job from block-job qapi: add job-change qapi/block-core: derpecate block-job-change iotests/mirror-change-copy-mode: switch to job-change command block/mirror.c | 13 +++++--- blockdev.c | 4 +-- blockjob.c | 20 ------------ docs/about/deprecated.rst | 5 +++ include/block/blockjob.h | 11 ------- include/block/blockjob_int.h | 7 ----- include/qemu/job.h | 12 +++++++ job-qmp.c | 15 +++++++++ job.c | 23 ++++++++++++++ qapi/block-core.json | 31 ++++++++++++++----- .../tests/mirror-change-copy-mode | 2 +- 11 files changed, 90 insertions(+), 53 deletions(-) -- 2.34.1

2 15

[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

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

23 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 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] 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

[PATCH] formatstorage: Document qcow2 default version change
by Peter Krempa 22 Jul '24

22 Jul '24

Based on discussion after commit f432114d9c was pushed it was pointed out that the documentation still mentions the older version. Fix the documentation to state the new version and introduce ambiguity for future updates. Fixes: f432114d9cf507a4047aa9dc1344b1c13356db08 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- Posting this documentation update to document what happened rather than introduce (almost pointless) complication in adding a config file which is unlikely to be ever used. docs/formatstorage.rst | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/formatstorage.rst b/docs/formatstorage.rst index 86e167d9cb..9d9a4143eb 100644 --- a/docs/formatstorage.rst +++ b/docs/formatstorage.rst @@ -700,10 +700,15 @@ host filesystem. It can contain the following child elements: Encryption <formatstorageencryption.html>`__ page for more information. ``compat`` Specify compatibility level. So far, this is only used for ``type='qcow2'`` - volumes. Valid values are ``0.10`` and ``1.1`` so far, specifying QEMU - version the images should be compatible with. If the ``feature`` element is - present, 1.1 is used. :since:`Since 1.1.0` If omitted, 0.10 is used. - :since:`Since 1.1.2` + volumes. Valid values are ``0.10`` (QCOW2 v2) and ``1.1`` (QCOW2 v3) so far. + The values were meant to specify QEMU version the images should be compatible + with. + + The default, if the ``feature`` element is present is ``1.1``. :since:`Since 1.1.0` + If ``feature`` is not present, ``0.10`` was used :since:`Since 1.1.2` and + :since:`Since 10.2.0` ``1.1`` is used as it's the default of ``qemu-img``. + + Any tool depending on a specific version should specify this field explicitly. ``nocow`` Turn off COW of the newly created volume. So far, this is only valid for a file image in btrfs file system. It will improve performance when the file -- 2.44.0

2 1

[PATCH] security: AppArmor allow write when os loader readonly=no
by Miroslav Los 19 Jul '24

19 Jul '24

Since libvirt commit 3ef9b51b10e52886e8fe8d75e36d0714957616b7, the pflash storage for the os loader file follows its read-only flag, and qemu tries to open the file for writing if set so. This patches virt-aa-helper to generate the VM's AppArmor rules that allow this, using the same domain definition flag and default. Signed-off-by: Miroslav Los <mirlos(a)cisco.com> --- src/security/virt-aa-helper.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 0374581f07..2f57664a4c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1001,9 +1001,14 @@ get_files(vahControl * ctl) if (vah_add_file(&buf, ctl->def->os.slic_table, "r") != 0) goto cleanup; - if (ctl->def->os.loader && ctl->def->os.loader->path) - if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0) + if (ctl->def->os.loader && ctl->def->os.loader->path) { + bool readonly = false; + virTristateBoolToBool(ctl->def->os.loader->readonly, &readonly); + if (vah_add_file(&buf, + ctl->def->os.loader->path, + readonly ? "rk" : "rwk") != 0) goto cleanup; + } if (ctl->def->os.loader && ctl->def->os.loader->nvram) { if (storage_source_add_files(ctl->def->os.loader->nvram, &buf, 0) < 0) -- 2.25.1

4 8

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

12 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 v1] security_manager: Ensure top lock is acquired before nested locks
by hongmianquan 04 Jul '24

04 Jul '24

We need to ensure top lock is acquired before nested lock. Otherwise deadlock issues may arise. We have the stack security driver, which internally manages other security drivers, we call them "top" and "nested". We call virSecurityStackPreFork() to lock the top one, and it also locks and then unlocks the nested drivers prior to fork. Then in qemuSecurityPostFork(), it unlocks the top one, but not the nested ones. Thus, if one of the nested drivers ("dac" or "selinux") is still locked, it will cause a deadlock. We discovered this case: the nested list obtained through the qemuSecurityGetNested() will be locked for subsequent use, such as in virQEMUDriverCreateCapabilities(), where the nested list is locked using qemuSecurityGetDOI, but the top one is not locked beforehand. The problem stack is as follows: libvirtd thread1 libvirtd thread2 child libvirtd | | | | | | virsh capabilities qemuProcessLanuch | | | | | lock top | | | | lock nested | | | | | | fork------------------->|(held nested lock) | | | | | | unlock nested unlock top unlock top | | qemuSecuritySetSocketLabel | | lock nested (deadlock) In this commit, we ensure that the top lock is acquired before the nested lock, so during fork, it's not possible for another task to acquire the nested lock. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1303031 Signed-off-by: hongmianquan <hongmianquan(a)bytedance.com> --- src/libvirt_private.syms | 3 ++- src/qemu/qemu_conf.c | 9 ++++++++- src/qemu/qemu_driver.c | 16 +++++++++------- src/qemu/qemu_security.h | 2 ++ src/security/security_manager.c | 22 ++++++++++++++++++++++ src/security/security_manager.h | 2 ++ 6 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bac4a8a366..39cdb90772 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1806,7 +1806,8 @@ virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; virSecurityManagerTransactionStart; virSecurityManagerVerify; - +virSecurityManagerStackLock; +virSecurityManagerStackUnlock; # security/security_util.h virSecurityXATTRNamespaceDefined; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4050a82341..21f0739fd5 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1380,6 +1380,9 @@ virCaps *virQEMUDriverCreateCapabilities(virQEMUDriver *driver) return NULL; } + /* Ensure top lock is acquired before nested locks */ + qemuSecurityStackLock(driver->securityManager); + /* access sec drivers and create a sec model for each one */ if (!(sec_managers = qemuSecurityGetNested(driver->securityManager))) return NULL; @@ -1402,8 +1405,10 @@ virCaps *virQEMUDriverCreateCapabilities(virQEMUDriver *driver) lbl = qemuSecurityGetBaseLabel(sec_managers[i], virtTypes[j]); type = virDomainVirtTypeToString(virtTypes[j]); if (lbl && - virCapabilitiesHostSecModelAddBaseLabel(sm, type, lbl) < 0) + virCapabilitiesHostSecModelAddBaseLabel(sm, type, lbl) < 0) { + qemuSecurityStackUnlock(driver->securityManager); return NULL; + } } VIR_DEBUG("Initialized caps for security driver \"%s\" with " @@ -1412,6 +1417,8 @@ virCaps *virQEMUDriverCreateCapabilities(virQEMUDriver *driver) caps->host.numa = virCapabilitiesHostNUMANewHost(); caps->host.cpu = virQEMUDriverGetHostCPU(driver); + + qemuSecurityStackUnlock(driver->securityManager); return g_steal_pointer(&caps); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fc1704f4fc..c980a0990f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -560,7 +560,6 @@ qemuStateInitialize(bool privileged, bool autostart = true; size_t i; const char *defsecmodel = NULL; - g_autofree virSecurityManager **sec_managers = NULL; g_autoptr(virIdentity) identity = virIdentityGetCurrent(); qemu_driver = g_new0(virQEMUDriver, 1); @@ -835,11 +834,8 @@ qemuStateInitialize(bool privileged, if (!qemu_driver->qemuCapsCache) goto error; - if (!(sec_managers = qemuSecurityGetNested(qemu_driver->securityManager))) - goto error; - - if (sec_managers[0] != NULL) - defsecmodel = qemuSecurityGetModel(sec_managers[0]); + if (qemu_driver->securityManager != NULL) + defsecmodel = qemuSecurityGetModel(qemu_driver->securityManager); if (!(qemu_driver->xmlopt = virQEMUDriverCreateXMLConf(qemu_driver, defsecmodel))) @@ -5663,7 +5659,12 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom, ret = 0; } else { int len = 0; - virSecurityManager ** mgrs = qemuSecurityGetNested(driver->securityManager); + virSecurityManager ** mgrs = NULL; + + /* Ensure top lock is acquired before nested locks */ + qemuSecurityStackLock(driver->securityManager); + + mgrs = qemuSecurityGetNested(driver->securityManager); if (!mgrs) goto cleanup; @@ -5688,6 +5689,7 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom, } cleanup: + qemuSecurityStackUnlock(driver->securityManager); virDomainObjEndAPI(&vm); return ret; } diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index 41da33debc..19fcb3c939 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -151,3 +151,5 @@ int qemuSecurityCommandRun(virQEMUDriver *driver, #define qemuSecuritySetTapFDLabel virSecurityManagerSetTapFDLabel #define qemuSecurityStackAddNested virSecurityManagerStackAddNested #define qemuSecurityVerify virSecurityManagerVerify +#define qemuSecurityStackLock virSecurityManagerStackLock +#define qemuSecurityStackUnlock virSecurityManagerStackUnlock \ No newline at end of file diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 24f2f3d3dc..c49c4f708f 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -989,6 +989,28 @@ virSecurityManagerGetNested(virSecurityManager *mgr) return list; } +/* + * Usually called before virSecurityManagerGetNested(). + * We need to ensure locking the stack security manager before + * locking the nested security manager to maintain the correct + * synchronization state. + * It must be followed by a call virSecurityManagerStackUnlock(). + */ +void +virSecurityManagerStackLock(virSecurityManager *mgr) +{ + if (STREQ("stack", mgr->drv->name)) + virObjectLock(mgr); +} + + +void +virSecurityManagerStackUnlock(virSecurityManager *mgr) +{ + if (STREQ("stack", mgr->drv->name)) + virObjectUnlock(mgr); +} + /** * virSecurityManagerDomainSetPathLabel: diff --git a/src/security/security_manager.h b/src/security/security_manager.h index a416af3215..bb6d22bc31 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -158,6 +158,8 @@ int virSecurityManagerSetTapFDLabel(virSecurityManager *mgr, char *virSecurityManagerGetMountOptions(virSecurityManager *mgr, virDomainDef *vm); virSecurityManager ** virSecurityManagerGetNested(virSecurityManager *mgr); +void virSecurityManagerStackLock(virSecurityManager *mgr); +void virSecurityManagerStackUnlock(virSecurityManager *mgr); typedef enum { VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN = 1 << 0, -- 2.11.0

3 4