[PATCH] meson: disable bogus warnings from sanitizers on Fedora
by Daniel P. Berrangé
When building with sanitizers on Fedora we get a wierd error
message
In file included from /usr/include/string.h:519,
from ../src/internal.h:28,
from ../src/util/virsocket.h:21,
from ../src/util/virsocketaddr.h:21,
from ../src/util/virnetdevip.h:21,
from ../src/util/virnetdevip.c:21:
In function ‘memcpy’,
inlined from ‘virNetDevGetifaddrsAddress’ at ../src/util/virnetdevip.c:702:13,
inlined from ‘virNetDevIPAddrGet’ at ../src/util/virnetdevip.c:754:16:
/usr/include/bits/string_fortified.h:29:10: error: ‘__builtin_memcpy’ offset [2, 27] from the object at ‘addr’ is out of the bounds of referenced subobject ‘ss_family’ with type ‘short unsigned int’ at offset 0 [-Werror=array-bounds]
29 | return __builtin___memcpy_chk (__dest, __src, __len,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
30 | __glibc_objsize0 (__dest));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/bits/socket.h:175,
from /usr/include/sys/socket.h:33,
from ../src/util/virsocket.h:66,
from ../src/util/virsocketaddr.h:21,
from ../src/util/virnetdevip.h:21,
from ../src/util/virnetdevip.c:21:
../src/util/virnetdevip.c: In function ‘virNetDevIPAddrGet’:
/usr/include/bits/socket.h:193:5: note: subobject ‘ss_family’ declared here
193 | __SOCKADDR_COMMON (ss_); /* Address family, etc. */
| ^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
The code is correct, and this only happens when building at -O2.
The docs for -Warray-bounds say that a value of "2" is known to
be liable to generate false positives. Rather than downgrade the
check everywhere, we do it selectively for sanitizers.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
meson.build | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index ca4291e37a..d4c142eebb 100644
--- a/meson.build
+++ b/meson.build
@@ -227,6 +227,11 @@ alloc_max = run_command(
# sanitizer instrumentation may enlarge stack frames
stack_frame_size = get_option('b_sanitize') == 'none' ? 4096 : 8192
+# array_bounds=2 check triggers false positive on some GCC
+# versions when using sanitizers. Seen on Fedora 34 with
+# GCC 11.1.1
+array_bounds = get_option('b_sanitize') == 'none' ? 2 : 1
+
cc_flags += [
'-fasynchronous-unwind-tables',
'-fexceptions',
@@ -238,7 +243,7 @@ cc_flags += [
'-Waggressive-loop-optimizations',
'-Walloc-size-larger-than=@0@'.format(alloc_max.stdout().strip()),
'-Walloca',
- '-Warray-bounds=2',
+ '-Warray-bounds=@0@'.format(array_bounds),
'-Wattribute-alias=2',
'-Wattribute-warning',
'-Wattributes',
--
2.31.1
3 years, 4 months
[PATCH v1] virtqemud: remove sysconfig file
by Olaf Hering
sysconfig files are owned by the admin of the host. He has the liberty
to put anything he wants into these files. This makes it difficult to
provide different defaults.
Remove the sysconfig file and place the current desired default into
the service file.
Local customizations can now go either into /etc/sysconfig/virtqemud
or /etc/systemd/system/virtqemud.service.d/my-knobs.conf
Signed-off-by: Olaf Hering <olaf(a)aepfle.de>
---
Untested, just to demonstrate how it could be done for every sysconfig file.
libvirt.spec.in | 1 -
src/qemu/meson.build | 5 -----
src/qemu/virtqemud.service.in | 1 +
src/qemu/virtqemud.sysconf | 12 ------------
4 files changed, 1 insertion(+), 18 deletions(-)
delete mode 100644 src/qemu/virtqemud.sysconf
diff --git a/libvirt.spec.in b/libvirt.spec.in
index cb48dd0be0..2cc3e61e3c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1689,7 +1689,6 @@ exit 0
%if %{with_qemu}
%files daemon-driver-qemu
-%config(noreplace) %{_sysconfdir}/sysconfig/virtqemud
%config(noreplace) %{_sysconfdir}/libvirt/virtqemud.conf
%{_datadir}/augeas/lenses/virtqemud.aug
%{_datadir}/augeas/lenses/tests/test_virtqemud.aug
diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index 3898d23877..6b4dd58309 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -165,11 +165,6 @@ if conf.has('WITH_QEMU')
'in_file': files('virtqemud.init.in'),
}
- sysconf_files += {
- 'name': 'virtqemud',
- 'file': files('virtqemud.sysconf'),
- }
-
virt_install_dirs += [
localstatedir / 'lib' / 'libvirt' / 'qemu',
runstatedir / 'libvirt' / 'qemu',
diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in
index 8abc9d3a7f..a5cb145668 100644
--- a/src/qemu/virtqemud.service.in
+++ b/src/qemu/virtqemud.service.in
@@ -18,6 +18,7 @@ Documentation=https://libvirt.org
[Service]
Type=notify
+Environment=VIRTQEMUD_ARGS="--timeout 120"
EnvironmentFile=-@sysconfdir@/sysconfig/virtqemud
ExecStart=@sbindir@/virtqemud $VIRTQEMUD_ARGS
ExecReload=/bin/kill -HUP $MAINPID
diff --git a/src/qemu/virtqemud.sysconf b/src/qemu/virtqemud.sysconf
deleted file mode 100644
index 87b626e3ed..0000000000
--- a/src/qemu/virtqemud.sysconf
+++ /dev/null
@@ -1,12 +0,0 @@
-# Customizations for the virtqemud.service systemd unit
-
-VIRTQEMUD_ARGS="--timeout 120"
-
-# Override the QEMU/SDL default audio driver probing when
-# starting virtual machines using SDL graphics
-#
-# NB these have no effect for VMs using VNC, unless vnc_allow_host_audio
-# is enabled in /etc/libvirt/qemu.conf
-#QEMU_AUDIO_DRV=sdl
-#
-#SDL_AUDIODRIVER=pulse
3 years, 4 months
[libvirt PATCH] qemu: Signal domain condition in qemuProcessStop a bit later
by Jiri Denemark
Signaling the condition before vm->def->id is reset to -1 is dangerous:
in case a waiting thread wakes up, it does not see anything interesting
(the domain is still marked as running) and just enters virDomainObjWait
where it waits forever because the condition will never be signalled
again.
Originally it was impossible to get into such situation because the vm
object was locked all the time between signaling the condition and
resetting vm->def->id, but after commit 860a999802 released in 6.8.0,
qemuDomainObjStopWorker called in qemuProcessStop between
virDomainObjBroadcast and setting vm->def->id to -1 unlocks the vm
object giving other threads a chance to wake up and possibly hang.
In real world, this can be easily reproduced by killing, destroying, or
just shutting down (from the guest OS) a domain while it is being
migrated somewhere else. The migration job would never finish.
We can't fix this by reseting vm->def->id earlier because other
functions (such as qemuProcessKill) do nothing when the domain is
already marked as inactive. So let's make sure we delay signaling the
domain condition to the point when a woken up thread can detect the
domain is not active anymore.
https://bugzilla.redhat.com/show_bug.cgi?id=1949869
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_process.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c972c90801..914f936e45 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7852,9 +7852,6 @@ void qemuProcessStop(virQEMUDriver *driver,
if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback)
driver->inhibitCallback(false, driver->inhibitOpaque);
- /* Wake up anything waiting on domain condition */
- virDomainObjBroadcast(vm);
-
if ((timestamp = virTimeStringNow()) != NULL) {
qemuDomainLogAppendMessage(driver, vm, "%s: shutting down, reason=%s\n",
timestamp,
@@ -7925,6 +7922,9 @@ void qemuProcessStop(virQEMUDriver *driver,
vm->def->id = -1;
+ /* Wake up anything waiting on domain condition */
+ virDomainObjBroadcast(vm);
+
virFileDeleteTree(priv->libDir);
virFileDeleteTree(priv->channelTargetDir);
--
2.32.0
3 years, 4 months
[PATCH 13/13] docs/deprecated: deprecate passing plugin args through `arg=`
by Mahmoud Mandour
Signed-off-by: Mahmoud Mandour <ma.mandourr(a)gmail.com>
---
docs/system/deprecated.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index e2e0090878..aaf0ee5777 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -126,6 +126,12 @@ other options have been processed. This will either have no effect (if
if they were not given. The property is therefore useless and should not be
specified.
+Plugin argument passing through ``arg=<string>`` (since 6.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Passing 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. ``arg="arg_name=arg_value"``. Therefore, the usage of ``arg`` is redundant.
QEMU Machine Protocol (QMP) commands
------------------------------------
--
2.25.1
3 years, 4 months
[PATCH 0/5] virsh: avoid local enum helpers with 'vir' prefix
by Peter Krempa
Follow up to:
https://listman.redhat.com/archives/libvir-list/2021-July/msg00281.html
Peter Krempa (5):
virsh-host: Avoid 'vir' prefix for locally declared VIR_ENUM* helpers
virsh-domain: Avoid 'vir' prefix for locally declared VIR_ENUM*
helpers
virsh-network: Avoid 'vir' prefix for locally declared VIR_ENUM*
helpers
virsh-volume: Avoid 'vir' prefix for locally declared VIR_ENUM*
helpers
syntax-check: Prohibit 'vir' prefix for enum implementations in virsh
build-aux/syntax-check.mk | 7 +++++++
tools/virsh-completer-domain.c | 8 ++++----
tools/virsh-completer-host.c | 2 +-
tools/virsh-domain.c | 18 +++++++++---------
tools/virsh-domain.h | 8 ++++----
tools/virsh-host.c | 4 ++--
tools/virsh-host.h | 2 +-
tools/virsh-network.c | 12 ++++++------
tools/virsh-volume.c | 6 +++---
9 files changed, 37 insertions(+), 30 deletions(-)
--
2.31.1
3 years, 4 months
[PATCH 0/2] Add support for 'id' attribute for 'cachetune' element
by Kristina Hanicova
Kristina Hanicova (2):
docs: Allow 'id' attribute for 'cachetune' element
tests: Modify to test output value of <cachetune id=''
docs/formatdomain.rst | 2 +-
docs/schemas/domaincommon.rng | 5 ++++
tests/genericxml2xmlindata/cachetune.xml | 8 +++---
tests/genericxml2xmloutdata/cachetune.xml | 34 -----------------------
tests/genericxml2xmltest.c | 2 +-
5 files changed, 11 insertions(+), 40 deletions(-)
delete mode 100644 tests/genericxml2xmloutdata/cachetune.xml
--
2.31.1
3 years, 4 months
[PATCH] test_driver: Implement virDomainGetControlInfo and add test
by Luke Yue
As test driver won't have real background job running, in order to get
all possible states, the time is used here to decide which state to be
returned. The default time will get `ok` as return value.
Note that using `virsh domtime fc4 200` won't take effect for the test
driver, to get other states, you have to enter virsh interactive
terminal and set time.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 63 ++++++++++++++++++++++++++++++++++++++++++
tests/virshtest.c | 11 ++++++++
2 files changed, 74 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ef0ddab54d..892dc978f2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2080,6 +2080,68 @@ testDomainGetState(virDomainPtr domain,
return 0;
}
+static int
+testDomainGetControlInfo(virDomainPtr dom,
+ virDomainControlInfoPtr info,
+ unsigned int flags)
+{
+ virDomainObj *vm;
+ testDomainObjPrivate *priv;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ memset(info, 0, sizeof(*info));
+
+ if (priv->seconds > 0 && priv->seconds < 10000) {
+ info->state = VIR_DOMAIN_CONTROL_JOB;
+ info->stateTime = priv->seconds;
+ } else if (priv->seconds < 30000 && priv->seconds >= 10000) {
+ info->state = VIR_DOMAIN_CONTROL_OCCUPIED;
+ info->stateTime = priv->seconds - 10000;
+ } else if (priv->seconds < 60000 && priv->seconds >= 30000) {
+ info->state = VIR_DOMAIN_CONTROL_ERROR;
+ switch (priv->seconds % 4) {
+ case 0:
+ info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_NONE;
+ break;
+
+ case 1:
+ info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_UNKNOWN;
+ break;
+
+ case 2:
+ info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_MONITOR;
+ break;
+
+ case 3:
+ info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_INTERNAL;
+ break;
+
+ default:
+ info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_NONE;
+ break;
+ }
+ info->stateTime = priv->seconds - 30000;
+ } else {
+ info->state = VIR_DOMAIN_CONTROL_OK;
+ }
+
+ ret = 0;
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
static int
testDomainGetTime(virDomainPtr dom,
long long *seconds,
@@ -9335,6 +9397,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetHostname = testDomainGetHostname, /* 5.5.0 */
.domainGetInfo = testDomainGetInfo, /* 0.1.1 */
.domainGetState = testDomainGetState, /* 0.9.2 */
+ .domainGetControlInfo = testDomainGetControlInfo, /* 7.6.0 */
.domainGetTime = testDomainGetTime, /* 5.4.0 */
.domainSetTime = testDomainSetTime, /* 5.7.0 */
.domainSave = testDomainSave, /* 0.3.2 */
diff --git a/tests/virshtest.c b/tests/virshtest.c
index c1974c46cb..fe0c420958 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -250,6 +250,13 @@ static int testCompareDomstateByName(const void *data G_GNUC_UNUSED)
return testCompareOutputLit(exp, NULL, argv);
}
+static int testCompareDomControlInfoByName(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "domcontrol", "fc4", NULL };
+ const char *exp = "ok\n\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
struct testInfo {
const char *const *argv;
const char *result;
@@ -334,6 +341,10 @@ mymain(void)
testCompareDomstateByName, NULL) != 0)
ret = -1;
+ if (virTestRun("virsh domcontrol (by name)",
+ testCompareDomControlInfoByName, NULL) != 0)
+ ret = -1;
+
/* It's a bit awkward listing result before argument, but that's a
* limitation of C99 vararg macros. */
# define DO_TEST(i, result, ...) \
--
2.32.0
3 years, 4 months
[libvirt PATCH 1/2] nodedev: fix parent device of inactive mdevs
by Jonathon Jongsma
Inactive mdevs were simply formatting their parent name as the value
received from mdevctl rather than looking up the libvirt nodedev name of
the parent device. This resulted in a parent value of e.g.
'0000:5b:00.0' instead of 'pci_0000_5b_00_0'. This prevented defining a
new mdev device from the output of nodedev-dumpxml.
Unfortunately, it's not simple to fix this comprehensively due to the
fact that mdevctl supports defining (inactive) mdevs for parent devices
that do not actually exist on the host (yet). So for those persistent
mdev definitions that do not have a valid parent in the device list, the
parent device will be set to the root "computer" device.
Unfortunately, because the value of the 'parent' field now depends on
the configuration of the host, the mdevctl parsing test will output
'computer' for all test devices. Fixing this would require a more
extensive mock test environment.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1979761
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
fixup
---
src/node_device/node_device_driver.c | 20 ++++++++++++++++++-
.../mdevctl-list-multiple.out.xml | 8 ++++----
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index b4dd57e5f4..26b0c2f032 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1051,6 +1051,7 @@ nodeDeviceParseMdevctlChildDevice(const char *parent,
virJSONValue *props;
virJSONValue *attrs;
g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1);
+ g_autofree char *parent_sysfs_path = NULL;
/* the child object should have a single key equal to its uuid.
* The value is an object describing the properties of the mdev */
@@ -1060,7 +1061,24 @@ nodeDeviceParseMdevctlChildDevice(const char *parent,
uuid = virJSONValueObjectGetKey(json, 0);
props = virJSONValueObjectGetValue(json, 0);
- child->parent = g_strdup(parent);
+ /* Look up id of parent device. mdevctl supports defining mdevs for parent
+ * devices that are not present on the system (to support starting mdevs on
+ * hotplug, etc) so the parent may not actually exist. */
+ parent_sysfs_path = g_strdup_printf("/sys/class/mdev_bus/%s", parent);
+ if (virFileExists(parent_sysfs_path)) {
+ g_autofree char *canon_syspath = realpath(parent_sysfs_path, NULL);
+ virNodeDeviceObj *parentobj = NULL;
+ if ((parentobj = virNodeDeviceObjListFindBySysfsPath(driver->devs,
+ canon_syspath))) {
+ virNodeDeviceDef *parentdef = virNodeDeviceObjGetDef(parentobj);
+ child->parent = g_strdup(parentdef->name);
+ virNodeDeviceObjEndAPI(&parentobj);
+
+ child->parent_sysfs_path = g_steal_pointer(&canon_syspath);
+ }
+ }
+ if (!child->parent)
+ child->parent = g_strdup("computer");
child->caps = g_new0(virNodeDevCapsDef, 1);
child->caps->data.type = VIR_NODE_DEV_CAP_MDEV;
diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
index cf7e966256..eead6f2a40 100644
--- a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
+++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
@@ -1,6 +1,6 @@
<device>
<name>mdev_200f228a_c80a_4d50_bfb7_f5a0e4e34045</name>
- <parent>0000:00:02.0</parent>
+ <parent>computer</parent>
<capability type='mdev'>
<type id='i915-GVTg_V5_4'/>
<uuid>200f228a-c80a-4d50-bfb7-f5a0e4e34045</uuid>
@@ -9,7 +9,7 @@
</device>
<device>
<name>mdev_de807ffc_1923_4d5f_b6c9_b20ecebc6d4b</name>
- <parent>0000:00:02.0</parent>
+ <parent>computer</parent>
<capability type='mdev'>
<type id='i915-GVTg_V5_4'/>
<uuid>de807ffc-1923-4d5f-b6c9-b20ecebc6d4b</uuid>
@@ -18,7 +18,7 @@
</device>
<device>
<name>mdev_435722ea_5f43_468a_874f_da34f1217f13</name>
- <parent>0000:00:02.0</parent>
+ <parent>computer</parent>
<capability type='mdev'>
<type id='i915-GVTg_V5_8'/>
<uuid>435722ea-5f43-468a-874f-da34f1217f13</uuid>
@@ -28,7 +28,7 @@
</device>
<device>
<name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name>
- <parent>matrix</parent>
+ <parent>computer</parent>
<capability type='mdev'>
<type id='vfio_ap-passthrough'/>
<uuid>783e6dbb-ea0e-411f-94e2-717eaad438bf</uuid>
--
2.31.1
3 years, 4 months
Question about skipping virDomainDiskDefAssignAddress
by Motohiro Kawahito
Hi, is there any existing way for skipping virDomainDiskDefAssignAddress
for disk configuration?
I want to send the following XML to libvirt, but I got the error
"virDomainDiskDefAssignAddress:7642 : XML error: Unknown disk name '0A80'
and no address specified".
<disk type='file'>
<source file='/volumes_qcow2/DMRES1.qcow2'/>
<target dev='0A80' />
</disk>
According to the source code, target device must be one of {"fd", "hd",
"vd", "sd", "xvd", "ubd"} + alphabet (+ number). Our guest OS is not
Linux, so I'd like to skip this limitation. Actually, I want to specify
4-digits hexadecimal number in target device.
Is there any hypervisor driver for Windows? I guess that it also has the
same problem.
Motohiro Kawahito, Commercial Systems, IBM Research - Tokyo
19-21 Nihonbashi, Hakozaki-cho Chuo-ku, Tokyo 103-8510, Japan
3 years, 4 months
