- Devel - libvirt List Archives

[libvirt PATCH 0/6] tests: refactor some functions (glib chronicles)
by Ján Tomko 06 Sep '21

06 Sep '21

Ján Tomko (6): tests: esxutils: refactor testParseDatastorePath tests: esxutils: reduce variable scope in testEscapeDatastoreItem tests: esxutils: reduce variable scope in testConvertWindows1252ToUTF8 tests: reduce variable scope in testSELinuxCheckLabels tests: refactor testSELinuxLoadDef virnetsockettest: refactor checkProtocols tests/esxutilstest.c | 46 +++++++++----------------------- tests/securityselinuxlabeltest.c | 41 +++++++++------------------- tests/virnetsockettest.c | 26 +++++++----------- 3 files changed, 34 insertions(+), 79 deletions(-) -- 2.31.1

2 7

[libvirt PATCH] secretxml2xmltest: refactor testCompareXMLToXMLFiles (glib chronicles)
by Ján Tomko 06 Sep '21

06 Sep '21

Use g_auto where possible and remove the pointless label. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- tests/secretxml2xmltest.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c index 48f48416b5..99aaa9db15 100644 --- a/tests/secretxml2xmltest.c +++ b/tests/secretxml2xmltest.c @@ -10,25 +10,19 @@ static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { - char *actual = NULL; - int ret = -1; - virSecretDef *secret = NULL; + g_autofree char *actual = NULL; + g_autoptr(virSecretDef) secret = NULL; if (!(secret = virSecretDefParseFile(inxml))) - goto fail; + return -1; if (!(actual = virSecretDefFormat(secret))) - goto fail; + return -1; if (virTestCompareToFile(actual, outxml) < 0) - goto fail; + return -1; - ret = 0; - - fail: - VIR_FREE(actual); - virSecretDefFree(secret); - return ret; + return 0; } struct testInfo { -- 2.31.1

2 1

[libvirt PATCH 0/4] Reinstate -Wunused-but-set-variable
by Ján Tomko 06 Sep '21

06 Sep '21

Personally, I feel like all the cases caught by new Clang's -Wunused-but-set-variable behavior were also unnecessarily difficult to read for humans, we should be more obvious when relying purely on side effects of g_auto and the warning is worth keeping. Even if not, the first two patches should not be controversial. Ján Tomko (4): qemu: remove unused 'cfg' variables util: virIdentitySetCurrent: only unref the old identity on success rpc: mark source returned by virEventGLibAddSocketWatch as unused Revert "meson: avoid bogus warnings from clang and g_autoptr" build-aux/syntax-check.mk | 2 +- meson.build | 21 --------------------- src/qemu/qemu_driver.c | 9 --------- src/rpc/virnetclient.c | 6 +++--- src/util/viridentity.c | 4 +++- 5 files changed, 7 insertions(+), 35 deletions(-) -- 2.31.1

2 5

[libvirt PATCH 0/2] tests: bhyve: minor style tweaks for constants
by Ján Tomko 06 Sep '21

06 Sep '21

I found these lying around in a forgotten branch. Ján Tomko (2): tests: bhyve: remove magic constants tests: bhyve: use bitwise shift when defining flags tests/bhyveargv2xmltest.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) -- 2.31.1

2 3

[PATCH] qemu_driver: Don't check for g_strdup_printf() retval
by Michal Privoznik 06 Sep '21

06 Sep '21

The g_strdup_printf() function can't fail really. There's no need to check for its return value. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_driver.c | 49 +++++++++--------------------------------- 1 file changed, 10 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cbd48b1849..2ea67b941e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -357,13 +357,7 @@ qemuDomainSnapshotLoad(virDomainObj *vm, priv = vm->privateData; - if (!(snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to allocate memory for " - "snapshot directory for domain %s"), - vm->def->name); - goto cleanup; - } + snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name); VIR_INFO("Scanning for snapshots for domain %s in %s", vm->def->name, snapDir); @@ -379,11 +373,7 @@ qemuDomainSnapshotLoad(virDomainObj *vm, kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name); - if (!(fullpath = g_strdup_printf("%s/%s", snapDir, entry->d_name))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to allocate memory for path")); - continue; - } + fullpath = g_strdup_printf("%s/%s", snapDir, entry->d_name); if (virFileReadAll(fullpath, 1024*1024*1, &xmlStr) < 0) { /* Nothing we can do here, skip this one */ @@ -464,13 +454,7 @@ qemuDomainCheckpointLoad(virDomainObj *vm, virObjectLock(vm); priv = vm->privateData; - if (!(chkDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to allocate memory for " - "checkpoint directory for domain %s"), - vm->def->name); - goto cleanup; - } + chkDir = g_strdup_printf("%s/%s", baseDir, vm->def->name); VIR_INFO("Scanning for checkpoints for domain %s in %s", vm->def->name, chkDir); @@ -486,11 +470,7 @@ qemuDomainCheckpointLoad(virDomainObj *vm, kill the whole process */ VIR_INFO("Loading checkpoint file '%s'", entry->d_name); - if (!(fullpath = g_strdup_printf("%s/%s", chkDir, entry->d_name))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to allocate memory for path")); - continue; - } + fullpath = g_strdup_printf("%s/%s", chkDir, entry->d_name); if (virFileReadAll(fullpath, 1024*1024*1, &xmlStr) < 0) { /* Nothing we can do here, skip this one */ @@ -626,8 +606,7 @@ qemuStateInitialize(bool privileged, if (!(qemu_driver->config = cfg = virQEMUDriverConfigNew(privileged, root))) goto error; - if (!(driverConf = g_strdup_printf("%s/qemu.conf", cfg->configBaseDir))) - goto error; + driverConf = g_strdup_printf("%s/qemu.conf", cfg->configBaseDir); if (virQEMUDriverConfigLoadFile(cfg, driverConf, privileged) < 0) goto error; @@ -2953,13 +2932,9 @@ qemuDomainSave(virDomainPtr dom, const char *path) static char * qemuDomainManagedSavePath(virQEMUDriver *driver, virDomainObj *vm) { - char *ret; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - if (!(ret = g_strdup_printf("%s/%s.save", cfg->saveDir, vm->def->name))) - return NULL; - - return ret; + return g_strdup_printf("%s/%s.save", cfg->saveDir, vm->def->name); } static int @@ -3461,8 +3436,7 @@ qemuDomainScreenshot(virDomainPtr dom, } } - if (!(tmp = g_strdup_printf("%s/qemu.screendump.XXXXXX", cfg->cacheDir))) - goto endjob; + tmp = g_strdup_printf("%s/qemu.screendump.XXXXXX", cfg->cacheDir); if ((tmp_fd = g_mkstemp_full(tmp, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) { virReportSystemError(errno, _("g_mkstemp(\"%s\") failed"), tmp); @@ -5296,8 +5270,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriver *driver, bool threadAdded = false; bool objectAdded = false; - if (!(alias = g_strdup_printf("iothread%u", iothread_id))) - return -1; + alias = g_strdup_printf("iothread%u", iothread_id); if (qemuMonitorCreateObjectProps(&props, "iothread", alias, NULL) < 0) goto cleanup; @@ -5429,8 +5402,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriver *driver, int new_niothreads = 0; qemuMonitorIOThreadInfo **new_iothreads = NULL; - if (!(alias = g_strdup_printf("iothread%u", iothread_id))) - return -1; + alias = g_strdup_printf("iothread%u", iothread_id); qemuDomainObjEnterMonitor(driver, vm); @@ -10716,8 +10688,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, if (virDomainObjCheckActive(vm) < 0) goto endjob; - if (!(tmp = g_strdup_printf("%s/qemu.mem.XXXXXX", cfg->cacheDir))) - goto endjob; + tmp = g_strdup_printf("%s/qemu.mem.XXXXXX", cfg->cacheDir); /* Create a temporary filename. */ if ((fd = g_mkstemp_full(tmp, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) { -- 2.31.1

2 2

[PATCH 0/1] qemu: add virtio-blk queue-size option
by Hiroki Narukawa 06 Sep '21

06 Sep '21

Hello, The option "queue-size" in virtio-blk was added in qemu-2.12.0, and default value increased from qemu-5.0.0. However, increasing this value may lead to drop of random access performance. This is configurable value, so we want to use it via libvirt. Sorry, my previous e-mail had wrong CC address that is rejected, please reply to this message and not previous one. Could you review this patch? Hiroki Narukawa (1): qemu: add virtio-blk queue-size option docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 6 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 5 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 3 ++ src/qemu/qemu_validate.c | 7 ++++ .../caps_2.12.0.aarch64.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + .../caps_3.0.0.riscv32.xml | 1 + .../caps_3.0.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + .../caps_3.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 + .../caps_3.1.0.x86_64.xml | 1 + .../caps_4.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml | 1 + .../caps_4.0.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../caps_5.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml | 1 + .../caps_5.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../caps_6.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 1 + .../disk-virtio-queue-size.args | 29 +++++++++++++++ .../disk-virtio-queue-size.xml | 35 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ .../disk-virtio-queue-size.xml | 35 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 47 files changed, 165 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.xml create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-queue-size.xml -- 2.17.1

2 3

[PATCH 2/2] qemu: support for virtio-blk-pci discard options
by yuxiating 06 Sep '21

06 Sep '21

DISCARD and WRITE_ZEROES features for machine type >= 4.0 is enabled by default since commit 5c81161f804144b146607f890e84613a4cbad95c virtio-blk: add "discard" and "write-zeroes" properties Sometimes guestos has bugs DISCARD need to be disabled. Signed-off-by: yuxiating <yuxiating(a)huawei.com> --- src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 9 +++++++++ src/conf/domain_validate.c | 6 ++++++ src/libvirt_private.syms | 3 ++- src/qemu/qemu_command.c | 11 +++++++++++ 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6127513117..bfe4721e60 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1278,6 +1278,13 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, "ignore", ); +VIR_ENUM_IMPL(virDomainDiskDiscardEnable, + VIR_DOMAIN_DISK_DISCARD_ENABLE_LAST, + "default", + "off", + "on", +); + VIR_ENUM_IMPL(virDomainDiskDetectZeroes, VIR_DOMAIN_DISK_DETECT_ZEROES_LAST, "default", @@ -8930,6 +8937,10 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def, if (virXMLPropUInt(cur, "queues", 10, VIR_XML_PROP_NONE, &def->queues) < 0) return -1; + if (virXMLPropEnum(cur, "discard_enable", virDomainDiskDiscardEnableTypeFromString, + VIR_XML_PROP_NONZERO, &def->discard_enable) < 0) + return -1; + return 0; } @@ -23416,6 +23427,10 @@ virDomainDiskDefFormatDriver(virBuffer *buf, if (disk->queues) virBufferAsprintf(&attrBuf, " queues='%u'", disk->queues); + if (disk->discard_enable) + virBufferAsprintf(&attrBuf, " discard_enable='%s'", + virDomainDiskDiscardEnableTypeToString(disk->discard_enable)); + virDomainVirtioOptionsFormat(&attrBuf, disk->virtio); if (disk->src->metadataCacheMaxSize > 0) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c7e6df7981..c39694a19e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -457,6 +457,13 @@ typedef enum { VIR_DOMAIN_DISK_DISCARD_LAST } virDomainDiskDiscard; +typedef enum { + VIR_DOMAIN_DISK_DISCARD_ENABLE_DEFAULT = 0, + VIR_DOMAIN_DISK_DISCARD_ENABLE_OFF, + VIR_DOMAIN_DISK_DISCARD_ENABLE_ON, + VIR_DOMAIN_DISK_DISCARD_ENABLE_LAST +} virDomainDiskDiscardEnable; + typedef enum { VIR_DOMAIN_DISK_DETECT_ZEROES_DEFAULT = 0, VIR_DOMAIN_DISK_DETECT_ZEROES_OFF, @@ -589,6 +596,7 @@ struct _virDomainDiskDef { bool diskElementAuth; bool diskElementEnc; + virDomainDiskDiscardEnable discard_enable; }; @@ -3838,6 +3846,7 @@ VIR_ENUM_DECL(virDomainDiskIo); VIR_ENUM_DECL(virDomainDeviceSGIO); VIR_ENUM_DECL(virDomainDiskTray); VIR_ENUM_DECL(virDomainDiskDiscard); +VIR_ENUM_DECL(virDomainDiskDiscardEnable); VIR_ENUM_DECL(virDomainDiskDetectZeroes); VIR_ENUM_DECL(virDomainDiskModel); VIR_ENUM_DECL(virDomainDiskMirrorState); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 60f7ccdddd..6eb346916a 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -612,6 +612,12 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } + if (disk->discard_enable) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("discard_enable attribute in disk driver element is only supported by virtio-blk")); + return -1; + } + if (disk->event_idx != VIR_TRISTATE_SWITCH_ABSENT) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("disk event_idx mode supported only for virtio bus")); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ab8a6c00c3..52a74dd2d5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1,5 +1,5 @@ -# # General private symbols. Add symbols here, and see src/meson.build for +# mainDiskDeviceTypeToString # more details. # # Keep this file sorted by header name, then by symbols with each header. @@ -377,6 +377,7 @@ virDomainDiskDefParseSource; virDomainDiskDetectZeroesTypeFromString; virDomainDiskDetectZeroesTypeToString; virDomainDiskDeviceTypeToString; +virDomainDiskDiscardEnableTypeToString; virDomainDiskDiscardTypeToString; virDomainDiskEmptySource; virDomainDiskErrorPolicyTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b230314f7f..894c8b17b9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1739,6 +1739,17 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, virBufferAsprintf(&opt, ",num-queues=%u", disk->queues); } + if (disk->discard_enable) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DISCARD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio-blk discard property isn't supported by this " + "QEMU binary")); + return NULL; + } + virBufferAsprintf(&opt, ",discard=%s", + virDomainDiskDiscardEnableTypeToString(disk->discard_enable)); + } + qemuBuildVirtioOptionsStr(&opt, disk->virtio); if (qemuBuildDeviceAddressStr(&opt, def, &disk->info) < 0) -- 2.27.0

2 1

RBD encryption support in libvirt
by Or Ozeri 05 Sep '21

05 Sep '21

Hi, I wanted to get your advice on a patch I'm preparing for libvirt. It touches the code-path that allows using LUKS encryption on top of an RBD image. We recently added LUKS and LUKS2 encryption support in Ceph's librbd. We exposed this in qemu in a recent patch by adding new optional "encrypt" member to BlockdevOptionsRbd. This patch was included in the recent release of qemu 6.1. To enable libvirt users to use librbd encryption, we need libvirt to use this new "encrypt" when it builds the blockdev options for RBD. The interesting question is how to define the libvirt XML syntax that will trigger the use of librbd encryption. My thought was to use the already existing <encryption> tag. In that case, we just need to add a new format VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2 to the enum virStorageEncryptionFormatType. This type will be checked in qemuBlockStorageSourceGetRBDProps. The problem with this approach is that it only works for LUKS2. librbd encryption also supports LUKS1. We want to allow the user to choose between the qemu LUKS implementation and the librbd one. One reason to keep support both is that on the one hand librbd only supports XTS mode. On the other hand, qemu implementation will not support a chain of uniquely encrypted RBD images (each serving as a backing store for the previous one). So we need a way in the XML API to support both implementations. Our current thought is to add a new "engine" attribute to the encryption tag. By default, encryption will use the QEMU LUKS implementation, unless <encryption engine='rbd' ...> is specified. To make this more general, we can have engine='backend' instead of engine='rbd' to denote that the encryption is to be delegated to the backend storage properties (instead of the format properties). This way, the encryption tag will only be parsed in the flow of qemuBlockStorageSourceGetBackendProps, instead of in qemuBlockStorageSourceGetBlockdevProps We'll appreciate any feedback you have on this. Thanks, Or

3 4

[PULL 21/22] docs/deprecated: deprecate passing plugin args through `arg=`
by Alex Bennée 03 Sep '21

03 Sep '21

From: Mahmoud Mandour <ma.mandourr(a)gmail.com> Signed-off-by: Mahmoud Mandour <ma.mandourr(a)gmail.com> Message-Id: <20210802134414.52037-1-ma.mandourr(a)gmail.com> Reviewed-by: Alex Bennée <alex.bennee(a)linaro.org> [AJB: fixed up move of deprecated.rst] Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 1e1a5e96ad..6e88a84bba 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -139,6 +139,18 @@ The ``-no-quit`` is a synonym for ``-display ...,window-close=off`` which should be used instead. +Plugin argument passing through ``arg=<string>`` (since 6.1) +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Passing TCG plugins arguments through ``arg=`` is redundant is makes the +command-line less readable, especially when the argument itself consist of a +name and a value, e.g. ``-plugin plugin_name,arg="arg_name=arg_value"``. +Therefore, the usage of ``arg`` is redundant. Single-word arguments are treated +as short-form boolean values, and passed to plugins as ``arg_name=on``. +However, short-form booleans are deprecated and full explicit ``arg_name=on`` +form is preferred. + + QEMU Machine Protocol (QMP) commands ------------------------------------ -- 2.30.2

1 0

[PATCH 0/6] network create: add support for validation against schema
by Kristina Hanicova 02 Sep '21

02 Sep '21

Kristina Hanicova (6): api: add public virNetworkCreateXMLFlags() and remote protocol src: add driver support for networkCreateXMLFlags() api: add virNetworkCreateFlags network_conf: add validation against schema in create network: allow VIR_NETWORK_CREATE_VALIDATE flag virsh: add support for '--validate' option in create network docs/manpages/virsh.rst | 4 ++- include/libvirt/libvirt-network.h | 7 ++++++ src/conf/network_conf.c | 3 ++- src/driver-network.h | 6 +++++ src/libvirt-network.c | 41 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/network/bridge_driver.c | 20 ++++++++++++--- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 18 +++++++++++++- src/remote_protocol-structs | 8 ++++++ src/test/test_driver.c | 15 +++++++++-- src/vbox/vbox_network.c | 10 +++++++- tools/virsh-network.c | 13 +++++++++- 13 files changed, 140 insertions(+), 11 deletions(-) -- 2.31.1

3 12