October 2019 - Devel - libvirt List Archives

[libvirt] [PATCH v2 0/5] Fix up some issues from x and y resolution patches
by Jonathon Jongsma 14 Nov '19

14 Nov '19

This is a follow-up series responding to some comments from Jan Tomko. Most importantly, the fact that the errors are not propagated up to the caller, they're only logged. To fix this, the function signatures were changed to return a error status. The patch series was also re-ordered slightly to improve readability (I hope). Jonathon Jongsma (5): qemu: fix documentation for video resolution conf: remove unnecessary NULL checks conf: use glib allocation when parsing video props conf: report errors when parsing video resolution conf: report errors when parsing video acceleration docs/formatdomain.html.in | 12 +++-- src/conf/domain_conf.c | 108 ++++++++++++++++++++++---------------- 2 files changed, 71 insertions(+), 49 deletions(-) -- 2.21.0

2 9

[libvirt] [RFC] default video device type
by Pavel Mores 14 Nov '19

14 Nov '19

Hi, I'm looking into fixing https://bugzilla.redhat.com/show_bug.cgi?id=1668141 (as a short summary, if a graphics device is added to XML that has no video device, libvirt automatically adds a video device which is always of type 'cirrus' - even if the underlying qemu doesn't support cirrus). I'm able to affect the behaviour in question by using qemu capabilities in qemuDomainDeviceVideoDefPostParse(), see proof-of-concept change in [1]. I have a couple of questions though: 1) is this a proper place and approach to fix the bug? 2) what would be the full specification of expected behaviour? The bug report only states that the video type shouldn't be cirrus but doesn't say what it should be. [2] gives some information about the order of preference of video device types but I was wondering if there are any opinions about this on this list? Cheers, pvl [1] diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b4175a846e..0de491b79f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7821,7 +7821,8 @@ qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net, static int qemuDomainDeviceVideoDefPostParse(virDomainVideoDefPtr video, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { if (video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { if (ARCH_IS_PPC64(def->os.arch)) @@ -7830,8 +7831,16 @@ qemuDomainDeviceVideoDefPostParse(virDomainVideoDefPtr video, qemuDomainIsRISCVVirt(def) || ARCH_IS_S390(def->os.arch)) video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO; - else - video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; + else { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE) + && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) { + video->type = VIR_DOMAIN_VIDEO_TYPE_QXL; + video->vgamem = QEMU_QXL_VGAMEM_DEFAULT; + } else { + video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; + } + } } if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && @@ -7926,7 +7935,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, break; case VIR_DOMAIN_DEVICE_VIDEO: - ret = qemuDomainDeviceVideoDefPostParse(dev->data.video, def); + ret = qemuDomainDeviceVideoDefPostParse(dev->data.video, def, qemuCaps); break; case VIR_DOMAIN_DEVICE_PANIC: [2] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/

3 3

[libvirt] [PATCH] virhostuptime: Add linux stub for musl
by Michal Privoznik 13 Nov '19

13 Nov '19

When we want to know the boot timestamp of the host, we can call virHostGetBootTime(). Under the hood, it uses getutxid() which is defined by POSIX and properly check for in configure. However, musl took a path where it declares the function but instead of providing any useful implementation it returns NULL meaning "no record found". If that's the case, use our second best option - /proc/uptime and a bit of maths. https://bugzilla.redhat.com/show_bug.cgi?id=1760885 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virhostuptime.c | 64 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/src/util/virhostuptime.c b/src/util/virhostuptime.c index 62b781acd5..3189074d3d 100644 --- a/src/util/virhostuptime.c +++ b/src/util/virhostuptime.c @@ -25,16 +25,68 @@ #endif #include "virhostuptime.h" +#include "viralloc.h" +#include "virfile.h" +#include "virlog.h" +#include "virstring.h" +#include "virtime.h" #include "virthread.h" +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("util.virhostuptime"); + static unsigned long long bootTime; static int bootTimeErrno; static virOnceControl virHostGetBootTimeOnce = VIR_ONCE_CONTROL_INITIALIZER; -#ifdef HAVE_GETUTXID +#if defined(__linux__) +# define UPTIME_FILE "/proc/uptime" +static int +virHostGetBootTimeProcfs(unsigned long long *btime) +{ + unsigned long long now; + double up; + g_autofree char *buf = NULL; + char *tmp; + + if (virTimeMillisNow(&now) < 0) + return -errno; + + /* 1KiB limit is more than enough. */ + if (virFileReadAll(UPTIME_FILE, 1024, &buf) < 0) + return -errno; + + /* buf contains two doubles now: + * $uptime $idle_time + * We're interested only in the first one */ + if (!(tmp = strchr(buf, ' '))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("uptime file has unexpected format '%s'"), + buf); + return -EINVAL; + } + + *tmp = '\0'; + + if (virStrToDouble(buf, NULL, &up) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse sched info value '%s'"), + buf); + return -EINVAL; + } + + *btime = now / 1000 - up + 0.5; + + return 0; +} +#endif /* defined(__linux__) */ + +#if defined(HAVE_GETUTXID) || defined(__linux__) static void virHostGetBootTimeOnceInit(void) { +# ifdef HAVE_GETUTXID struct utmpx id = {.ut_type = BOOT_TIME}; struct utmpx *res = NULL; @@ -45,16 +97,22 @@ virHostGetBootTimeOnceInit(void) } endutxent(); +# endif /* HAVE_GETUTXID */ + + if (bootTimeErrno == ENODATA || + (bootTime == 0 && bootTimeErrno == 0)) { + bootTimeErrno = -virHostGetBootTimeProcfs(&bootTime); + } } -#else /* !HAVE_GETUTXID */ +#else /* !defined(HAVE_GETUTXID) && !defined(__linux__) */ static void virHostGetBootTimeOnceInit(void) { bootTimeErrno = ENOSYS; } -#endif /* HAVE_GETUTXID */ +#endif /* !defined(HAVE_GETUTXID) && !defined(__linux__) */ /** * virHostGetBootTime: -- 2.21.0

2 1

[libvirt] [PATCH v2 0/2] qemu: fix type of default video device
by Pavel Mores 13 Nov '19

13 Nov '19

This new version mostly integrates Cole's comments about the first version. Pavel Mores (2): qemu: fix type of default video device qemu: add tests of the default video type selection algorithm src/qemu/qemu_domain.c | 39 +++++++++++------ .../default-video-type-aarch64.xml | 16 +++++++ .../default-video-type-ppc64.xml | 16 +++++++ .../default-video-type-riscv64.xml | 16 +++++++ .../default-video-type-s390x.xml | 16 +++++++ .../default-video-type-x86_64-caps-test-0.xml | 17 ++++++++ .../default-video-type-x86_64-caps-test-1.xml | 17 ++++++++ tests/qemuxml2argvtest.c | 1 + ...ault-video-type-aarch64.aarch64-latest.xml | 42 +++++++++++++++++++ .../default-video-type-ppc64.ppc64-latest.xml | 31 ++++++++++++++ ...ault-video-type-riscv64.riscv64-latest.xml | 39 +++++++++++++++++ .../default-video-type-s390x.s390x-latest.xml | 32 ++++++++++++++ .../default-video-type-x86_64-caps-test-0.xml | 31 ++++++++++++++ .../default-video-type-x86_64-caps-test-1.xml | 31 ++++++++++++++ tests/qemuxml2xmltest.c | 10 ++++- 15 files changed, 341 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/default-video-type-aarch64.xml create mode 100644 tests/qemuxml2argvdata/default-video-type-ppc64.xml create mode 100644 tests/qemuxml2argvdata/default-video-type-riscv64.xml create mode 100644 tests/qemuxml2argvdata/default-video-type-s390x.xml create mode 100644 tests/qemuxml2argvdata/default-video-type-x86_64-caps-test-0.xml create mode 100644 tests/qemuxml2argvdata/default-video-type-x86_64-caps-test-1.xml create mode 100644 tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/default-video-type-riscv64.riscv64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml create mode 100644 tests/qemuxml2xmloutdata/default-video-type-x86_64-caps-test-0.xml create mode 100644 tests/qemuxml2xmloutdata/default-video-type-x86_64-caps-test-1.xml -- 2.21.0

2 4

[libvirt] [PATCH v3 0/3] unplug timeout changes for PPC64
by Daniel Henrique Barboza 13 Nov '19

13 Nov '19

changes in v3: - redesigned patch 1 based on Cole Robinson feedback v2: https://www.redhat.com/archives/libvir-list/2019-September/msg00447.html v1: https://www.redhat.com/archives/libvir-list/2019-August/msg00698.html Daniel Henrique Barboza (3): qemu_hotplug.c: adding qemuDomainGetUnplugTimeout qemu: Remove qemu_hotplugpriv.h and qemuDomainRemoveDeviceWaitTime qemu_hotplug.c: user-friendlier setvcpus timeout error message src/qemu/Makefile.inc.am | 1 - src/qemu/qemu_hotplug.c | 28 ++++++++++++++----- src/qemu/qemu_hotplug.h | 2 ++ tests/Makefile.am | 13 ++++++++- .../qemuhotplugmock.c | 27 +++++++++--------- tests/qemuhotplugtest.c | 7 ++--- 6 files changed, 51 insertions(+), 27 deletions(-) rename src/qemu/qemu_hotplugpriv.h => tests/qemuhotplugmock.c (61%) -- 2.21.0

2 4

[libvirt] [PATCH 0/3] Update rebootTimeout range to 0..0xffff
by Han Han 13 Nov '19

13 Nov '19

Since qemu v4.0.0(commit ee5d0f89de3), reboot-timeout valid range was set 0~0xffff. Update libvirt codes, docs, tests for that change. I am not sure if po files should be updated. If so, please point it out. Han Han (3): conf: Set rebootTimeout valid range to 0..0xffff tests: Remove test reboot-timeout-disabled docs: Update docs on rebootTimeout valid value range docs/formatdomain.html.in | 5 ++-- src/conf/domain_conf.c | 6 ++-- .../reboot-timeout-disabled.args | 28 ------------------- .../reboot-timeout-disabled.xml | 24 ---------------- .../reboot-timeout-disabled.xml | 26 ----------------- 5 files changed, 6 insertions(+), 83 deletions(-) delete mode 100644 tests/qemuxml2argvdata/reboot-timeout-disabled.args delete mode 100644 tests/qemuxml2argvdata/reboot-timeout-disabled.xml delete mode 100644 tests/qemuxml2xmloutdata/reboot-timeout-disabled.xml -- 2.20.1

3 11

[libvirt] [PATCH] qemu: hotplug: ensure address generation for vfio-ccw
by Bjoern Walk 12 Nov '19

12 Nov '19

When attaching a mediated host device of model vfio-ccw without specifying a guest-address, none is generated by libvirt. Let's fix this and make sure to generate a device address during live-hotplug. Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk(a)linux.ibm.com> --- src/qemu/qemu_hotplug.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fd4bafef..7b775159 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2790,6 +2790,8 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver, { int ret = -1; g_autofree char *devstr = NULL; + char *devName = NULL; + bool releaseaddr = false; bool added = false; bool teardowncgroup = false; bool teardownlabel = false; @@ -2805,6 +2807,10 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver, return -1; break; case VIR_MDEV_MODEL_TYPE_VFIO_CCW: + devName = hostdev->source.subsys.u.mdev.uuidstr; + if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev, devName) < 0) + return -1; + break; case VIR_MDEV_MODEL_TYPE_LAST: break; } -- 2.21.0

3 7

[libvirt] [PATCH v1 00/21] remove 'cleanup: return' labels
by Daniel Henrique Barboza 12 Nov '19

12 Nov '19

Hi, This series is focused on removing most of the unused 'cleanup' labels that we have in the code so far (as of master commit 2cff65e4c6). I say 'most' because there's a handful of instances left that are being used by macros and other machinery that I judged it wasn't worth to deal with in the scope of this work. The motivations for this change are (1) less code to deal with and (2) a bit more clarity, since the presence of a 'cleanup' label should indicate a cleanup procedure (virObjectUnref and so on). The vast majority of the cases being handled here are variations of the following pattern: ----- (function header) int ret = -1; ( nothing is done with 'ret' in the function body, 'cleanup' calls are made on error conditions) ret = 0; cleanup: return ret; (end of function) ----- This pattern can be replaced by removing 'cleanup:' and 'ret', replacing it with 'return -1' on error conditions and 'return 0' on success. Other patterns were handled as well, but this is the most frequent one. I made an effort to *not* change or refactor the function logic in the attempt of removing labels. Patches were split by directory. If only one file in a dir got changed, the file is mentioned in the commit instead. There were not preparatory patches for these changes because they are intended to be somewhat simple. The one exception is patch 09. I chose to handle that case in a separated patch to not clutter the review of patch 10, but I expect it to be merged with the 'qemu' patch once it is reviewed. All these changes are available in [1]. [1] https://github.com/danielhb/libvirt/tree/clean_trivial_labels Daniel Henrique Barboza (21): conf: remove unneeded cleanup labels storage: remove unneeded cleanup labels nwfilter: remove unneeded cleanup labels remote: remove unneeded cleanup labels lxc: remove unneeded cleanup labels node_device: remove unneeded cleanup labels secret_driver.c: remove unneeded cleanup label openvz_conf.c: remove unneeded cleanup label qemu_monitor_json.c: remove unneeded cleanup label qemu: remove unneeded cleanup labels rpc: remove unneeded cleanup labels vz_sdk.c: remove unneeded cleanup label vbox: remove unneeded cleanup labels util: remove unneeded cleanup labels libvirt.c: remove unneeded cleanup label libxl_driver.c: remove unneeded cleanup label bridge_driver.c: remove unneeded cleanup labels suspend.c: remove unneeded cleanup label tools: remove unneeded cleanup labels tests: remove unneeded cleanup labels bhyve_device.c: remove unneeded cleanup labels examples/c/domain/suspend.c | 9 +- src/bhyve/bhyve_device.c | 18 +- src/conf/domain_addr.c | 23 +- src/conf/domain_capabilities.c | 21 +- src/conf/domain_conf.c | 334 ++++++++-------------- src/conf/netdev_bandwidth_conf.c | 17 +- src/conf/network_conf.c | 25 +- src/conf/node_device_conf.c | 28 +- src/conf/numa_conf.c | 25 +- src/conf/nwfilter_conf.c | 4 +- src/conf/storage_conf.c | 7 +- src/conf/virnetworkobj.c | 14 +- src/conf/virsecretobj.c | 5 +- src/libvirt.c | 7 +- src/libxl/libxl_driver.c | 16 +- src/lxc/lxc_cgroup.c | 70 ++--- src/lxc/lxc_container.c | 17 +- src/lxc/lxc_controller.c | 44 ++- src/lxc/lxc_driver.c | 39 +-- src/lxc/lxc_process.c | 34 +-- src/network/bridge_driver.c | 172 +++++------ src/node_device/node_device_driver.c | 11 +- src/node_device/node_device_udev.c | 8 +- src/nwfilter/nwfilter_ebiptables_driver.c | 132 ++++----- src/openvz/openvz_conf.c | 3 +- src/qemu/qemu_cgroup.c | 15 +- src/qemu/qemu_domain.c | 92 +++--- src/qemu/qemu_domain_address.c | 31 +- src/qemu/qemu_driver.c | 140 ++++----- src/qemu/qemu_hotplug.c | 45 ++- src/qemu/qemu_interface.c | 26 +- src/qemu/qemu_migration.c | 38 +-- src/qemu/qemu_monitor_json.c | 15 +- src/qemu/qemu_process.c | 77 ++--- src/qemu/qemu_tpm.c | 20 +- src/remote/remote_daemon.c | 38 ++- src/remote/remote_driver.c | 15 +- src/rpc/virnetserver.c | 15 +- src/rpc/virnetserverprogram.c | 13 +- src/rpc/virnetsocket.c | 9 +- src/rpc/virnettlscontext.c | 20 +- src/secret/secret_driver.c | 11 +- src/storage/storage_backend_fs.c | 13 +- src/storage/storage_backend_rbd.c | 58 ++-- src/storage/storage_backend_zfs.c | 21 +- src/storage/storage_driver.c | 38 +-- src/storage/storage_util.c | 18 +- src/util/vircgroupv1.c | 18 +- src/util/vircommand.c | 11 +- src/util/virdbus.c | 30 +- src/util/virfile.c | 19 +- src/util/virhash.c | 6 +- src/util/virhostcpu.c | 20 +- src/util/virhostdev.c | 7 +- src/util/virhostmem.c | 55 ++-- src/util/virjson.c | 30 +- src/util/virmacmap.c | 10 +- src/util/virnetdevbridge.c | 21 +- src/util/virnuma.c | 27 +- src/util/virpci.c | 7 +- src/util/virprocess.c | 19 +- src/util/virresctrl.c | 7 +- src/util/virstoragefile.c | 46 +-- src/util/virutil.c | 19 +- src/vbox/vbox_common.c | 12 +- src/vbox/vbox_snapshot_conf.c | 75 ++--- src/vz/vz_sdk.c | 8 +- tests/commandtest.c | 17 +- tests/domainconftest.c | 5 +- tests/networkxml2firewalltest.c | 13 +- tests/nsstest.c | 27 +- tests/nwfilterebiptablestest.c | 4 +- tests/nwfilterxml2firewalltest.c | 16 +- tests/qemuhotplugtest.c | 19 +- tests/qemuxml2argvtest.c | 11 +- tests/storagebackendsheepdogtest.c | 5 +- tests/virauthconfigtest.c | 11 +- tests/vircgroupmock.c | 11 +- tests/virendiantest.c | 58 ++-- tests/virkeycodetest.c | 14 +- tests/virmacmaptest.c | 5 +- tests/virnetdevtest.c | 11 +- tests/virpcimock.c | 31 +- tests/virpcitest.c | 3 +- tests/virpolkittest.c | 65 ++--- tests/virstringtest.c | 18 +- tools/virsh-completer-network.c | 8 +- tools/virsh-domain.c | 41 +-- tools/vsh.c | 14 +- 89 files changed, 983 insertions(+), 1722 deletions(-) -- 2.21.0

2 44

[libvirt] [PATCH 0/4] consolidate cgroup code that is duplicated between
by Mao Zhongyi 12 Nov '19

12 Nov '19

Both QEMU and LXC drivers have some duplicate code when dealing with cgroups, this serials mainly consolidate the same chunk of src/qemu/qemu_cgroup.c and src/lxc/lxc_cgroup.c into src/util/cgroup.c. Mao Zhongyi (4): lxc: remove duplicate header files qemu: remove duplicate header files util: consolidate QEMU and LXC block io related duplicate code util: consolidate QEMU and LXC memory related duplicate code src/libvirt_private.syms | 2 + src/lxc/lxc_cgroup.c | 85 +------------------------------------ src/qemu/qemu_cgroup.c | 62 +-------------------------- src/util/vircgroup.c | 91 ++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 4 ++ 5 files changed, 101 insertions(+), 143 deletions(-) -- 2.17.1

2 7

[libvirt] [dbus PATCH] meson: fix construction of data directories
by Pavel Hrdina 12 Nov '19

12 Nov '19

We need to use prefix for all directories and there is no need to check if the user provided path starts with '/' as that is done automatically by the join_paths() function or '/' operator. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- meson.build | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/meson.build b/meson.build index 472e4e2..b293b8c 100644 --- a/meson.build +++ b/meson.build @@ -9,12 +9,30 @@ project( ], ) +prefix = get_option('prefix') +datadir = prefix / get_option('datadir') +sbindir = prefix / get_option('sbindir') + +opt_dirs = [ + 'dbus_interfaces', + 'dbus_services', + 'dbus_system_services', + 'dbus_system_policies', + 'polkit_rules', +] + +foreach opt_dir : opt_dirs + value = get_option(opt_dir) + varname = '@0@_dir'.format(opt_dir) + set_variable(varname, datadir / value) +endforeach + conf = configuration_data() conf.set('MESON_VERSION', '0.49.0') conf.set('PACKAGE', meson.project_name()) conf.set('VERSION', meson.project_version()) conf.set('build_root', meson.build_root()) -conf.set('sbindir', get_option('sbindir')) +conf.set('sbindir', sbindir) conf.set('source_root', meson.source_root()) @@ -40,24 +58,6 @@ git = run_command('test', '-d', '.git').returncode() == 0 conf.set('SYSTEM_USER', get_option('system_user')) -opt_dirs = [ - 'dbus_interfaces', - 'dbus_services', - 'dbus_system_services', - 'dbus_system_policies', - 'polkit_rules', -] - -foreach opt_dir : opt_dirs - value = get_option(opt_dir) - varname = '@0@_dir'.format(opt_dir) - if opt_dir.startswith('/') - set_variable(varname, value) - else - set_variable(varname, join_paths(get_option('datadir'), value)) - endif -endforeach - # Compile flags -- 2.21.0

2 1