September 2023 - Devel - libvirt List Archives

[PATCH 0/3] A couple of virtio-mem/virtio-pmem address improvements
by Michal Privoznik 22 Sep '23

22 Sep '23

*** BLURB HERE *** Michal Prívozník (3): domain_validate: Validate VIRTIO_PMEM address alignment virDomainMemoryDefValidate: Fix VIRTIO_MEM alignment check virDomainMemoryDefValidate: Check for overlapping memory devices src/conf/domain_validate.c | 59 ++++++++++++++++++- ...rtio-mem-overlap-address.x86_64-latest.err | 1 + ...ory-hotplug-virtio-mem-overlap-address.xml | 50 ++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem-overlap-address.x86_64-latest.err create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem-overlap-address.xml -- 2.41.0

2 4

[PATCH] test_driver: Implement virConnectGetDomainCapabilities()
by Michal Privoznik 21 Sep '23

21 Sep '23

Our test driver lacks implementation for virConnectGetDomainCapabilities(). Provide one, though a trivial one. Mostly so that something else than VIR_ERR_NO_SUPPORT error is returned. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/test/test_driver.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c1d4d71b78..a8705a8da2 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -10000,6 +10000,41 @@ testNetworkSetMetadata(virNetworkPtr net, return ret; } +static char * +testConnectGetDomainCapabilities(virConnectPtr conn G_GNUC_UNUSED, + const char *emulatorbin, + const char *arch_str, + const char *machine, + const char *virttype_str, + unsigned int flags) +{ + g_autoptr(virDomainCaps) domCaps = NULL; + virArch arch = VIR_ARCH_NONE; + int virttype = VIR_DOMAIN_VIRT_NONE; + + virCheckFlags(0, NULL); + + if (arch_str && + (arch = virArchFromString(arch_str)) == VIR_ARCH_NONE) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown architecture: %1$s"), arch_str); + return NULL; + } + + if (virttype_str && + (virttype = virDomainVirtTypeFromString(virttype_str)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown virttype: %1$s"), virttype_str); + return NULL; + } + + if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, virttype))) + return NULL; + + return virDomainCapsFormat(domCaps); +} + + /* * Test driver */ @@ -10167,6 +10202,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainCheckpointGetParent = testDomainCheckpointGetParent, /* 5.6.0 */ .domainCheckpointDelete = testDomainCheckpointDelete, /* 5.6.0 */ .domainGetMessages = testDomainGetMessages, /* 7.6.0 */ + .connectGetDomainCapabilities = testConnectGetDomainCapabilities, /* 9.8.0 */ }; static virNetworkDriver testNetworkDriver = { -- 2.41.0

2 1

[libvirt PATCH] qemu_nbdkit: fix possible null dereference
by Pavel Hrdina 20 Sep '23

20 Sep '23

Function virGetConnectSecret() can return NULL so we need to check it since in virSecretGetSecretString() it gets dereferenced. Reported-by: coverity Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/qemu/qemu_nbdkit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 66b09cd240..17819ca992 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -971,6 +971,9 @@ qemuNbdkitProcessBuildCommandAuth(virStorageAuthDef *authdef, } conn = virGetConnectSecret(); + if (!conn) + return -1; + if (virSecretGetSecretString(conn, &authdef->seclookupdef, secrettype, -- 2.41.0

2 1

[PATCH 0/4] virerror: Make virReportEnumRangeError() check for type mismatch
by Michal Privoznik 20 Sep '23

20 Sep '23

There are few cases where virReportEnumRangeError() is passed to a mismatching enum. With patch 4/4 we can check for this at compile time (assuming -Wenum-compare is available). For instance with clang I get: FAILED: src/conf/libvirt_conf.a.p/domain_validate.c.o clang ... ../src/conf/domain_validate.c:184:9: error: comparison of different enumeration types ('virDomainVideoBackendType' and 'virDomainInputType') [-Werror,-Wenum-compare] virReportEnumRangeError(virDomainInputType, video->backend); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/util/virerror.h:175:47: note: expanded from macro 'virReportEnumRangeError' (__typeof__(value))1 == (typname)1 && sizeof((typname)1) != 0 ? #typname : #typname) ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~ Whereas with gcc I get more obscure error: FAILED: src/conf/libvirt_conf.a.p/domain_validate.c.o cc ... In file included from ../src/util/virthread.h:25, from ../src/util/virobject.h:25, from ../src/conf/capabilities.h:28, from ../src/conf/domain_conf.h:31, from ../src/conf/domain_validate.h:25, from ../src/conf/domain_validate.c:23: ../src/conf/domain_validate.c: In function ‘virDomainVideoDefValidate’: ../src/util/virerror.h:175:47: error: comparison between ‘enum <anonymous>’ and ‘enum <anonymous>’ [-Werror=enum-compare] 175 | (__typeof__(value))1 == (typname)1 && sizeof((typname)1) != 0 ? #typname : #typname) | ^~ ../src/conf/domain_validate.c:184:9: note: in expansion of macro ‘virReportEnumRangeError’ 184 | virReportEnumRangeError(virDomainInputType, video->backend); | ^~~~~~~~~~~~~~~~~~~~~~~ But I guess this is also okay-ish - it reports there is a problem with code. NB this only works with variables that are of a certain enum types. It does not work with those generic 'int var /* someEnum */' declarations, understandably. But as we use virXMLPropEnum() more, we can declare variables of their true type (enum) and get more and more calls to virReportEnumRangeError() checked. Michal Prívozník (4): virnetdevvportprofile: Turn virNetDevVPortProfileLinkOp enum into a proper typedef virNetDevVPortProfileOp8021Qbh: Use proper type in virReportEnumRangeError() virDomainVideoDefValidate: Use proper type in virReportEnumRangeError() virerror: Make virReportEnumRangeError() check for type mismatch src/conf/domain_validate.c | 2 +- src/util/virerror.h | 14 ++++++++------ src/util/virnetdevvportprofile.c | 10 +++++----- 3 files changed, 14 insertions(+), 12 deletions(-) -- 2.41.0

2 5

[PATCH] libxl: Fix Domain-0 ballooning logic
by Jim Fehlig 20 Sep '23

20 Sep '23

When Domain-0 autoballooning is enabled, it's possible that memory may need to be ballooned down in Domain-0 to accommodate the needs of another virtual machine. libxlDomainFreeMemory handles this task, but due to a logic bug is underflowing the variable containing Domain-0 new target memory. The resulting huge numbers are filtered by libxlSetMemoryTargetWrapper and memory is not changed. Under the covers, libxlDomainFreeMemory uses Xen's libxl_set_memory_target API, which includes a 'relative' parameter for specifying how to set the target. If true, the target is an increment/decrement value over the current memory, otherwise target is taken as an absolute value. libxlDomainFreeMemory sets 'relative' to true, but never allows for negative values by declaring the target memory variable as an unsigned. Fix by declaring the variable as signed, which also requried adjusting libxlSetMemoryTargetWrapper. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/libxl/libxl_api_wrapper.h | 16 ++++++---------- src/libxl/libxl_domain.c | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/libxl/libxl_api_wrapper.h b/src/libxl/libxl_api_wrapper.h index a9627f7983..c9582294cc 100644 --- a/src/libxl/libxl_api_wrapper.h +++ b/src/libxl/libxl_api_wrapper.h @@ -193,24 +193,20 @@ libxlSendTriggerWrapper(libxl_ctx *ctx, static inline int libxlSetMemoryTargetWrapper(libxl_ctx *ctx, uint32_t domid, - uint64_t target_memkb, + int64_t target_memkb, int relative, int enforce) { int ret = -1; - /* Technically this guard could be LIBXL_HAVE_MEMKB_64BITS */ -#if LIBXL_API_VERSION < 0x040800 - if (target_memkb < UINT_MAX) { - uint32_t val32 = target_memkb; +#ifdef LIBXL_HAVE_MEMKB_64BITS + ret = libxl_set_memory_target(ctx, domid, target_memkb, relative, enforce); +#else + if (target_memkb < INT_MAX) { + int32_t val32 = target_memkb; ret = libxl_set_memory_target(ctx, domid, val32, relative, enforce); } -#else - if (target_memkb < LLONG_MAX) { - int64_t val64 = target_memkb; - ret = libxl_set_memory_target(ctx, domid, val64, relative, enforce); - } #endif return ret; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 6c167df63e..0c4beffd6a 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -926,7 +926,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config) { uint64_t needed_mem; uint64_t free_mem; - uint64_t target_mem; + int64_t target_mem; int tries = 3; int wait_secs = 10; -- 2.42.0

2 2

Share qemuInterfacexxxxConnect methods with ch
by Praveen Paladugu 20 Sep '23

20 Sep '23

Folks, I am working on upstreaming network support for ch driver. Like qemu driver,ch driver invokes steps in qemuInterfaceEthernetConnect, qemuInterfaceBridgeConnect methods to connect tap devices to appropriate host backends. Current implementation clones aboves methods to ch_interface files and uses them. I'd like to drop driver specific args from above methods and move them to a common place so that qemu and ch drivers can share above methods. int virInterfaceEthernetConnect(virDomainDef *def, virDomainNetDef *net, ebtablesContext *ebtables, bool macFilter, bool privileged, int *tapfd, size_t tapfdSize) I started with `qemuInterfaceEthernetConnect` and modified the signature as shown above. I initially tried putting `virInterfaceEthernetConnect` in 'src/util' but that caused a lot of cross inclusions. Above definition pulls in domain_conf.h, domain_nwfilter.h, domain_audit.h and more headers, into utils which causes cross inclusion. I later created interface/interface_connect.{c,h} files and moved the methods to these files. This would require driver code to include interface/interface_connect.h, which seemed better than above cross inclusion scenario. Do you see any issues with moving above methods to interface/interface_connect.x files? Any other ideas on how to reorg above methods so that they can be shared by qemu and ch drivers? Praveen

2 2

[PATCH 00/16] nwfilter: Add support for user defined metadata
by K Shiva Kiran 19 Sep '23

19 Sep '23

This patchset adds support for the following user defined metadata fields for network filters. - title: A short description of the filter. - description: Any documentation that the user wants to store. - metadata: Other metadata in XML form. K Shiva Kiran (16): xml: Add <title>, <description> and <metadata> to nwfilter xml schema conf: Add parser logic for nwfilter metadata fields nwfilter: Add enum to operate on user defined metadata nwfilter: Add error code and message for missing metadata nwfilter: Introduce public API to modify user metadata nwfilter: Introduce public API to retrieve user-defined metadata nwfilter: Implement RPC virsh: Add helper method to retrieve xml from NWFilter def virsh: Add new command `nwfilter-desc` virsh: Add new command `nwfilter-metadata` virsh: Add option --title for nwfilter-list docs: Document nwfilter metadata related commands virnwfilterobj: Add virNWFilterObjGetMetadata() virnwfilterobj: Add virNWFilterObjSetMetadata() nwfilter_driver: Add Driver implementation for metadata NEWS: Introduce user-defined metadata fields for NWFilter object NEWS.rst | 18 ++ docs/formatnwfilter.rst | 31 +++ docs/manpages/virsh.rst | 98 +++++++- include/libvirt/libvirt-nwfilter.h | 27 ++ include/libvirt/virterror.h | 1 + src/conf/nwfilter_conf.c | 30 +++ src/conf/nwfilter_conf.h | 5 + src/conf/schemas/nwfilter.rng | 9 + src/conf/virnwfilterobj.c | 148 +++++++++++ src/conf/virnwfilterobj.h | 13 + src/driver-nwfilter.h | 15 ++ src/libvirt-nwfilter.c | 154 ++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 6 + src/nwfilter/nwfilter_driver.c | 61 +++++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 34 ++- src/remote_protocol-structs | 19 ++ src/util/virerror.c | 3 + tools/virsh-nwfilter.c | 387 ++++++++++++++++++++++++++++- tools/virsh-util.c | 25 ++ tools/virsh-util.h | 9 + 22 files changed, 1089 insertions(+), 8 deletions(-) -- 2.42.0

2 25

[libvirt PATCH v3 00/12] Extract the integration job commands to a shell script
by Erik Skultety 19 Sep '23

19 Sep '23

v1 here: https://listman.redhat.com/archives/libvir-list/2023-September/242121.html Since v1: - introduced a new run_cmd_quiet helper function instead of using eval to deal with Shell redirections - added more info from the POSIX standard on test's -a,-o options - added commit references to the vendor locking of Avocado to v98.0 in the past Erik Skultety (12): syntax-check: Drop the shell's 'check for minus' rule ci: integration: Extract the integration CI main recipe to jobs.sh ci: integration: Adjust the check for CentOS Stream version ci: integration: Drop the 'install-deps' hidden job and reference ci: jobs.sh: Drop comment about the need for Avocado 98.0 ci: jobs.sh: integration: Use --quiet with virsh ci: jobs.sh: run_integration: Add/Rewrite/Reformat commentaries ci: jobs.sh: run_integration: Make POSIX-compliant ci: jobs.sh: Introduce a quiet version of run_cmd ci: jobs.sh: integration: Execute commands via 'run_cmd[_quiet]' helpers ci: jobs.sh: run_integration: Print DAEMONS variable for debugging ci: jobs.sh: Define and create SCRATCH_DIR for local executions build-aux/syntax-check.mk | 9 ------- ci/integration-template.yml | 42 +++-------------------------- ci/jobs.sh | 54 +++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 48 deletions(-) -- 2.41.0

2 14

[PATCH] fix uint64 underflow
by Dmitry Frolov 19 Sep '23

19 Sep '23

According to previous statement, 'free_mem' is less than 'needed_mem'. So, the subtraction 'free_mem - needed_mem' is negative, and will raise uint64 underflow. Signed-off-by: Dmitry Frolov <frolov(a)swemel.ru> --- src/libxl/libxl_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 6c167df63e..36be042971 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -940,7 +940,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config) if (free_mem >= needed_mem) return 0; - target_mem = free_mem - needed_mem; + target_mem = needed_mem - free_mem; if (libxlSetMemoryTargetWrapper(ctx, 0, target_mem, /* relative */ 1, 0) < 0) goto error; -- 2.34.1

4 3

[PATCH] src: Avoid needless checks before calling g_strdup()
by Michal Privoznik 18 Sep '23

18 Sep '23

There are few places where the following pattern occurs: if (var) other = g_strdup(var); where @other wasn't initialized before g_strdup(). Checking for var != NULL is useless in this case, as that's exactly what g_strdup() does (in which case it returns NULL). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/domain_event.c | 3 +-- src/libxl/libxl_conf.c | 6 ++---- src/lxc/lxc_native.c | 3 +-- src/qemu/qemu_monitor_json.c | 3 +-- src/util/virconf.c | 3 +-- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 75603a933a..09f3368064 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1561,8 +1561,7 @@ virDomainEventMetadataChangeNew(int id, return NULL; ev->type = type; - if (nsuri) - ev->nsuri = g_strdup(nsuri); + ev->nsuri = g_strdup(nsuri); return (virObjectEvent *)ev; } diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 3ffb46fddd..4582126d19 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -583,8 +583,7 @@ libxlMakeDomBuildInfo(virDomainDef *def, #endif /* copy SLIC table path to acpi_firmware */ - if (def->os.slic_table) - b_info->u.hvm.acpi_firmware = g_strdup(def->os.slic_table); + b_info->u.hvm.acpi_firmware = g_strdup(def->os.slic_table); if (def->nsounds > 0) { /* @@ -1198,8 +1197,7 @@ libxlMakeDisk(virDomainDiskDef *l_disk, libxl_device_disk *x_disk) return -1; } - if (l_disk->domain_name) - x_disk->backend_domname = g_strdup(l_disk->domain_name); + x_disk->backend_domname = g_strdup(l_disk->domain_name); return 0; } diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 5a63efc35f..c0011e0600 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -67,8 +67,7 @@ lxcCreateFSDef(int type, def->type = type; def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH; - if (src) - def->src->path = g_strdup(src); + def->src->path = g_strdup(src); def->dst = g_strdup(dst); def->readonly = readonly; def->usage = usage; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 137cb4e293..8152eea9a0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2960,8 +2960,7 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValue *reply, case QEMU_MONITOR_MIGRATION_STATUS_ERROR: if (error) { tmp = virJSONValueObjectGetString(ret, "error-desc"); - if (tmp) - *error = g_strdup(tmp); + *error = g_strdup(tmp); } break; diff --git a/src/util/virconf.c b/src/util/virconf.c index 934632a35f..8fdf40e9d0 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -939,8 +939,7 @@ int virConfGetValueStringList(virConf *conf, case VIR_CONF_STRING: if (compatString) { *values = g_new0(char *, cval->str ? 2 : 1); - if (cval->str) - (*values)[0] = g_strdup(cval->str); + (*values)[0] = g_strdup(cval->str); break; } G_GNUC_FALLTHROUGH; -- 2.41.0

2 1