September 2021 - Devel - libvirt List Archives

[PATCH] qemu: remove unnecessary strlen for LINE_ENDING
by renlei1998＠163.com 08 Sep '21

08 Sep '21

From: Ren Lei <ren.lei4(a)zte.com.cn> the length of LINE_ENDING is static, it's a waste to call strlen every time. Signed-off-by: Ren Lei <ren.lei4(a)zte.com.cn> --- src/qemu/qemu_agent.c | 5 +++-- src/qemu/qemu_monitor_json.c | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5f421be6f6..3a453b4c58 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -47,6 +47,7 @@ VIR_LOG_INIT("qemu.qemu_agent"); #define LINE_ENDING "\n" +#define LINE_ENDING_LENGTH 1 #define DEBUG_IO 0 #define DEBUG_RAW_IO 0 @@ -341,11 +342,11 @@ static int qemuAgentIOProcessData(qemuAgent *agent, if (nl) { int got = nl - (data + used); - for (i = 0; i < strlen(LINE_ENDING); i++) + for (i = 0; i < LINE_ENDING_LENGTH; i++) data[used + got + i] = '\0'; if (qemuAgentIOProcessLine(agent, data + used, msg) < 0) return -1; - used += got + strlen(LINE_ENDING); + used += got + LINE_ENDING_LENGTH; } else { break; } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8d3c4031a6..8b77b3cdaa 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -51,6 +51,7 @@ VIR_LOG_INIT("qemu.qemu_monitor_json"); #define QOM_CPU_PATH "/machine/unattached/device[0]" #define LINE_ENDING "\r\n" +#define LINE_ENDING_LENGTH 2 VIR_ENUM_IMPL(qemuMonitorJob, QEMU_MONITOR_JOB_TYPE_LAST, @@ -271,7 +272,7 @@ int qemuMonitorJSONIOProcess(qemuMonitor *mon, int got = nl - (data + used); char *line; line = g_strndup(data + used, got); - used += got + strlen(LINE_ENDING); + used += got + LINE_ENDING_LENGTH; line[got] = '\0'; /* kill \n */ if (qemuMonitorJSONIOProcessLine(mon, line, msg) < 0) { VIR_FREE(line); -- 2.27.0

2 1

[PATCH 0/6] qemu: Fix crash when validating a XML without disk's <target>
by Peter Krempa 08 Sep '21

08 Sep '21

Apart from fixing the crash the validation code is fixed to do the correct thing and a test case is added. Peter Krempa (6): qemuDomainDefValidateDiskLunSource: Unbreak error messages conf: validate: Move qemu-specific LUN disk validation to global validation conf: validate: Run global device definition validation before callbacks conf: Don't call 'virDomainDiskDefAssignAddress' when disk->dst is NULL virDomainDiskDefValidate: Move validation of disk target qemuxml2argvtest: Add test case for missing disk '<target>' src/conf/domain_conf.c | 1 + src/conf/domain_validate.c | 94 ++++++++++++++----- src/conf/domain_validate.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 50 ---------- src/qemu/qemu_domain.h | 3 - src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_validate.c | 3 - .../default-video-type-x86_64-caps-test-0.err | 2 +- .../disk-fdc-incompatible-address.err | 2 +- .../disk-ide-incompatible-address.err | 2 +- .../disk-missing-target-invalid.err | 1 + .../disk-missing-target-invalid.xml | 22 +++++ .../disk-sata-incompatible-address.err | 2 +- .../disk-scsi-incompatible-address.err | 2 +- .../pseries-default-phb-numa-node.err | 2 +- .../pseries-phb-invalid-target-index-1.err | 2 +- .../pseries-phb-invalid-target-index-2.err | 2 +- .../pseries-phb-invalid-target-index-3.err | 2 +- .../video-invalid-multiple-devices.err | 2 +- ...splay-device-pci-address.x86_64-latest.err | 2 +- tests/qemuxml2argvtest.c | 1 + 22 files changed, 109 insertions(+), 93 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-missing-target-invalid.err create mode 100644 tests/qemuxml2argvdata/disk-missing-target-invalid.xml -- 2.31.1

2 7

[PATCH v2 0/3] support for virtio-blk-pci discard option
by yuxiating＠huawei.com 08 Sep '21

08 Sep '21

From: yuxiating <yuxiating(a)huawei.com> v1->v2 add docs/formatdomain.rst for discard_enable add qemuxml2argvtest change virDomainDiskDiscardEnable to virTristateSwitch yuxiating (3): qemu: probe for virtio-blk-pci discard option support conf: support for virtio-blk-pci discard option qemu: command: support for virtio-blk-pci discard option docs/formatdomain.rst | 3 ++ docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 8 +++++ src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 6 ++++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 +++ .../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 + .../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 + .../qemuxml2argvdata/disk-virtio-discard.args | 29 +++++++++++++++ .../qemuxml2argvdata/disk-virtio-discard.xml | 35 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 36 files changed, 125 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-discard.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-discard.xml -- 2.27.0

2 6

[libvirt PATCH 0/6] libxl: remove enum libxlHwcapVersion
by Ján Tomko 08 Sep '21

08 Sep '21

While looking at users of virCPU.*Free, I found some code that is supposed to deal with Xen < 4.7. The minimum version is 4.9 since the following commit: commit 68940b3fb3c43b8aa03cb6fd2f1d00b1737c9b2c Author: Jim Fehlig <jfehlig(a)suse.com> CommitDate: 2021-06-17 10:11:56 -0600 Xen: Bump minimum supported Xen version to 4.9 Remove the code dealing with the old version and use g_auto moer. Ján Tomko (6): libxl: capabilities: assume Xen version >= 4.7 libxl: remove enum libxlHwcapVersion libxl: capsInitCPU: fail if we can't initialize host features libxl: refactor libxlCapsInitCPU libxl: use g_auto in libxlCapsNodeData libxl: use g_auto in libxlDomainGetEmulatorType src/libxl/libxl_capabilities.c | 119 ++++++++++----------------------- 1 file changed, 37 insertions(+), 82 deletions(-) -- 2.31.1

2 7

Re: [PATCH 2/2] qemu: support for virtio-blk-pci discard options
by y005616296 07 Sep '21

07 Sep '21

>On Tue, Aug 31, 2021 at 20:50:04 +0800, yuxiating wrote: >> 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. > >I'm not very persuaded by this commit message, could you please elaborate how the bugs show themselves? > >Specifically this fells like a hack/workaround rather so a proper justification would be great. > Virtio_blk kernel driver has a bug that causes memory corruption in virtblk_setup_discard_write_zeroes(); af822aa68fbd ("block: virtio_blk: fix handling single range discard request") has fix it. However, some operating systems are not fixed and need to disabled on the QEMU side. >> 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(-) > >Please split the conf/* changes from the qemu changes. > >Also you didn't add any documentation to docs/formatdomain.rst which describes the XML which is unacceptable. > >Additionally you are also missing test cases for qemuxml2argvtest. > I will add a description to docs/ formatDomain.rst. qemuxml2argvtest will be added in version V2. >> >> 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", > >NACK to this hunk, 'default' value feels weird, and for the rest we have the virTristateBool type. > > I find virTristateSwitch more appropriate. >> +); >> + >> 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)); > >It even behaves exactly like virTristateBool. > I'll use virTristateSwitch instead >> + >> 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 > >[...] > >> 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. > >NACK to this hunk too, you shouldn't need to modify the header here. > It was my mistake. >> @@ -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)) >> + { > >This capability was also checked in the validator so you don't need to do this here. > >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> + _("virtio-blk discard property isn't supported by this " >> + "QEMU binary")); > > > I'm going to delete this check. >> + return NULL; >> + } >> + virBufferAsprintf(&opt, ",discard=%s", >> + virDomainDiskDiscardEnableTypeToString(disk->discard_enable)); >> + } >> + >> qemuBuildVirtioOptionsStr(&opt, disk->virtio); >> >> if (qemuBuildDeviceAddressStr(&opt, def, &disk->info) < 0) >> -- >> 2.27.0 >> >>

1 0

[PATCH] qemu_security: Set the label of monitor
by Peng Liang 07 Sep '21

07 Sep '21

Signed-off-by: Peng Liang <liangpeng10(a)huawei.com> --- src/qemu/qemu_security.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 19d957dd4b96..96755a62bd2c 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -52,6 +52,12 @@ qemuSecuritySetAllLabel(virQEMUDriver *driver, priv->chardevStdioLogd, migrated) < 0) goto cleanup; + if (priv->monConfig && + virSecurityManagerSetChardevLabel(driver->securityManager, + vm->def, + priv->monConfig, + priv->chardevStdioLogd) < 0) + goto cleanup; if (virSecurityManagerTransactionCommit(driver->securityManager, pid, priv->rememberOwner) < 0) -- 2.31.1

2 4

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

07 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 07 Sep '21

07 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 07 Sep '21

07 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 07 Sep '21

07 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