April 2016 - Devel - libvirt List Archives

[libvirt] [PATCH] virsh: don't override connection URI from argv
by Roman Bogorodskiy 21 Apr '16

21 Apr '16

Currently, if a connection URI was specified on the command line by the '-c' switch, virsh connects to it, but after connecting overrides its value with the one it tries to obtain from the VIRSH_DEFAULT_CONNECT_URI environment variable. This makes virsh connecting to the wrong URI if it disconnects from the hypervisor and then tries to reconnect, and also leaks the original connname. Fix by calling virGetEnvBlockSUID() before virshParseArgv(). --- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 0d8ec5c..e14410b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -962,14 +962,14 @@ main(int argc, char **argv) if (!vshInit(ctl, cmdGroups, NULL)) exit(EXIT_FAILURE); + ctl->connname = vshStrdup(ctl, virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")); + if (!virshParseArgv(ctl, argc, argv) || !virshInit(ctl)) { virshDeinit(ctl); exit(EXIT_FAILURE); } - ctl->connname = vshStrdup(ctl, virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")); - if (!ctl->imode) { ret = vshCommandRun(ctl, ctl->cmd); } else { -- 2.7.4

3 4

[libvirt] [PATCH] virsh: blockcopy: force an absolute path
by Cole Robinson 21 Apr '16

21 Apr '16

virsh doesn't reject or absolutify a passed relative path, which can give unexpected results. Convert a relative path to an absolute one before calling the API https://bugzilla.redhat.com/show_bug.cgi?id=1300177 --- tools/virsh-domain.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a1d4a75..7074ded 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2256,8 +2256,9 @@ static bool cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; - const char *dest = NULL; + const char *dest_cli = NULL; const char *format = NULL; + char *dest = NULL; unsigned long bandwidth = 0; unsigned int granularity = 0; unsigned long long buf_size = 0; @@ -2281,7 +2282,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "dest", &dest) < 0) + if (vshCommandOptStringReq(ctl, cmd, "dest", &dest_cli) < 0) return false; if (vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0) return false; @@ -2308,6 +2309,11 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) if (async) abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; + if (dest_cli && virFileAbsPath(dest_cli, &dest) < 0) { + vshError(ctl, _("failed to build absolute path for %s"), dest_cli); + return false; + } + VSH_EXCLUSIVE_OPTIONS_VAR(dest, xml); VSH_EXCLUSIVE_OPTIONS_VAR(format, xml); VSH_EXCLUSIVE_OPTIONS_VAR(blockdev, xml); @@ -2462,6 +2468,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: + VIR_FREE(dest); VIR_FREE(xmlstr); virTypedParamsFree(params, nparams); virDomainFree(dom); -- 2.7.3

4 3

[libvirt] [PATCH 0/4] build: Fix symbolic links usage
by Andrea Bolognani 21 Apr '16

21 Apr '16

This series fixes the build failures currently affecting FreeBSD on ci.centos.org[1] and also gets rid of some behavior deemed dangerous by the autoconf manual[2]. Tested on Fedora 23 and FreeBSD 10.3-RELEASE. [1] https://ci.centos.org/view/libvirt-project/job/libvirt-freebsd/617/console [2] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Partic… Andrea Bolognani (4): build: Always use $(LN_S) and $(MKDIR_P) build: Add AC_PROG_LN_S to configure build: Overwrite existing symbolic links build: Use $(LN_S) safely Makefile.am | 2 +- configure.ac | 1 + src/Makefile.am | 5 ++--- tools/Makefile.am | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) -- 2.5.5

2 6

[libvirt] [PATCH 1/2] qemu: process: split out shmem startup warning
by Cole Robinson 21 Apr '16

21 Apr '16

Now we can return early and save some indentation --- src/qemu/qemu_process.c | 95 ++++++++++++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c087300..10e1b5a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4497,6 +4497,56 @@ qemuProcessMakeDir(virQEMUDriverPtr driver, } +static void +qemuProcessStartWarnShmem(virDomainObjPtr vm) +{ + size_t i; + bool check_shmem = false; + bool shmem = vm->def->nshmems; + + /* + * For vhost-user to work, the domain has to have some type of + * shared memory configured. We're not the proper ones to judge + * whether shared hugepages or shm are enough and will be in the + * future, so we'll just warn in case neither is configured. + * Moreover failing would give the false illusion that libvirt is + * really checking that everything works before running the domain + * and not only we are unable to do that, but it's also not our + * aim to do so. + */ + for (i = 0; i < vm->def->nnets; i++) { + if (virDomainNetGetActualType(vm->def->nets[i]) == + VIR_DOMAIN_NET_TYPE_VHOSTUSER) { + check_shmem = true; + break; + } + } + + if (!check_shmem) + return; + + /* + * This check is by no means complete. We merely check + * whether there are *some* hugepages enabled and *some* NUMA + * nodes with shared memory access. + */ + if (!shmem && vm->def->mem.nhugepages) { + for (i = 0; i < virDomainNumaGetNodeCount(vm->def->numa); i++) { + if (virDomainNumaGetNodeMemoryAccessMode(vm->def->numa, i) == + VIR_NUMA_MEM_ACCESS_SHARED) { + shmem = true; + break; + } + } + } + + if (!shmem) { + VIR_WARN("Detected vhost-user interface without any shared memory, " + "the interface might not be operational"); + } +} + + /** * qemuProcessStartValidate: * @vm: domain object @@ -4517,9 +4567,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, bool snapshot, unsigned int flags) { - bool check_shmem = false; - size_t i; - if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) { if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) { VIR_DEBUG("Checking for KVM availability"); @@ -4559,47 +4606,7 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, VIR_DEBUG("Checking for any possible (non-fatal) issues"); - /* - * For vhost-user to work, the domain has to have some type of - * shared memory configured. We're not the proper ones to judge - * whether shared hugepages or shm are enough and will be in the - * future, so we'll just warn in case neither is configured. - * Moreover failing would give the false illusion that libvirt is - * really checking that everything works before running the domain - * and not only we are unable to do that, but it's also not our - * aim to do so. - */ - for (i = 0; i < vm->def->nnets; i++) { - if (virDomainNetGetActualType(vm->def->nets[i]) == - VIR_DOMAIN_NET_TYPE_VHOSTUSER) { - check_shmem = true; - break; - } - } - - if (check_shmem) { - bool shmem = vm->def->nshmems; - - /* - * This check is by no means complete. We merely check - * whether there are *some* hugepages enabled and *some* NUMA - * nodes with shared memory access. - */ - if (!shmem && vm->def->mem.nhugepages) { - for (i = 0; i < virDomainNumaGetNodeCount(vm->def->numa); i++) { - if (virDomainNumaGetNodeMemoryAccessMode(vm->def->numa, i) == - VIR_NUMA_MEM_ACCESS_SHARED) { - shmem = true; - break; - } - } - } - - if (!shmem) { - VIR_WARN("Detected vhost-user interface without any shared memory, " - "the interface might not be operational"); - } - } + qemuProcessStartWarnShmem(vm); return 0; } -- 2.7.3

2 2

[libvirt] [PATCH python] spec: Don't pull in dependencies for example scripts
by Cole Robinson 21 Apr '16

21 Apr '16

If the scripts are marked as executable, RPM magic will scan them for dependencies, which can pull in python2 for the python3 package --- Taken from this commit by a fedora packager: http://pkgs.fedoraproject.org/cgit/rpms/libvirt-python.git/commit/?id=78763… libvirt-python.spec.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in index 8a21003..3021ebd 100644 --- a/libvirt-python.spec.in +++ b/libvirt-python.spec.in @@ -49,6 +49,11 @@ of recent versions of Linux (and other OSes). %prep %setup -q +# Unset execute bit for example scripts; it can introduce spurious +# RPM dependencies, like /usr/bin/python which can pull in python2 +# for the -python3 package +find examples -type f -exec chmod 0644 \{\} \; + %build CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build %if %{with_python3} -- 2.7.3

2 1

[libvirt] [PATCH] virsh: Don't clear old connection if 'connect $uri' fails
by Cole Robinson 21 Apr '16

21 Apr '16

virsh # list --all Id Name State ---------------------------------------------------- 1 test running virsh # connect frob error: Failed to connect to the hypervisor error: no connection driver available for frob virsh # list --all error: failed to connect to the hypervisor error: no valid connection error: no connection driver available for frob Seems sensible IMO to just not clear out the old connection state until the new virConnectOpen succeeds. https://bugzilla.redhat.com/show_bug.cgi?id=829160 --- There was an older discussion here: https://www.redhat.com/archives/libvir-list/2012-June/msg00270.html That seemed to go off into the weeds a bit, talking about caching old URIs... This approach seems fine for me to address the bug report but maybe I missed something tools/virsh.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 0d8ec5c..353a5d8 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -292,6 +292,17 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) bool ro = vshCommandOptBool(cmd, "readonly"); const char *name = NULL; virshControlPtr priv = ctl->privData; + virConnectPtr conn; + + if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) + return false; + + conn = virshConnect(ctl, name, ro); + + if (!conn) { + vshError(ctl, "%s", _("Failed to connect to the hypervisor")); + return false; + } if (priv->conn) { int ret; @@ -303,13 +314,10 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) else if (ret > 0) vshError(ctl, "%s", _("One or more references were leaked after " "disconnect from the hypervisor")); - priv->conn = NULL; } + priv->conn = conn; VIR_FREE(ctl->connname); - if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) - return false; - ctl->connname = vshStrdup(ctl, name); priv->useGetInfo = false; @@ -317,13 +325,6 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) priv->blockJobNoBytes = false; priv->readonly = ro; - priv->conn = virshConnect(ctl, ctl->connname, priv->readonly); - - if (!priv->conn) { - vshError(ctl, "%s", _("Failed to connect to the hypervisor")); - return false; - } - if (virConnectRegisterCloseCallback(priv->conn, virshCatchDisconnect, ctl, NULL) < 0) vshError(ctl, "%s", _("Unable to register disconnect callback")); -- 2.7.3

2 1

[libvirt] [PATCH] libvirt: Fix crash on URI without scheme
by Cole Robinson 21 Apr '16

21 Apr '16

My commit 0d1579572 crashes on a URI without a scheme, like via 'virsh --connect frob' Add a check on uri->server too while we are at it, and centralize them all --- src/libvirt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 749089b..114e88c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -935,6 +935,9 @@ virConnectGetDefaultURI(virConfPtr conf, static int virConnectCheckURIMissingSlash(const char *uristr, virURIPtr uri) { + if (!uri->scheme || !uri->path || !uri->server) + return 0; + /* To avoid false positives, only check drivers that mandate a path component in the URI, like /system or /session */ if (STRNEQ(uri->scheme, "qemu") && @@ -942,9 +945,6 @@ virConnectCheckURIMissingSlash(const char *uristr, virURIPtr uri) STRNEQ(uri->scheme, "vz")) return 0; - if (uri->path != NULL) - return 0; - if (STREQ(uri->server, "session") || STREQ(uri->server, "system")) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.7.3

2 1

[libvirt] RFC: virtio-rng and /dev/urandom
by Cole Robinson 21 Apr '16

21 Apr '16

Libvirt currently rejects using host /dev/urandom as an input source for a virtio-rng device. The only accepted sources are /dev/random and /dev/hwrng. This is the result of discussions on qemu-devel around when the feature was first added (2013). Examples: http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg02387.html https://lists.gnu.org/archive/html/qemu-devel/2013-03/threads.html#00023 libvirt's rejection of /dev/urandom has generated some complaints from users: https://bugzilla.redhat.com/show_bug.cgi?id=1074464 * cited: http://www.2uo.de/myths-about-urandom/ http://www.redhat.com/archives/libvir-list/2016-March/msg01062.html http://www.redhat.com/archives/libvir-list/2016-April/msg00186.html I think it's worth having another discussion about this, at least with a recent argument in one place so we can put it to bed. I'm CCing a bunch of people. I think the questions are: 1) is the original recommendation to never use virtio-rng+/dev/urandom correct? 2) regardless of #1, should we continue to reject that config in libvirt? Thanks, Cole

9 21

[libvirt] [PATCH 0/4] libxl: Add support for UEFI using OVMF
by Jim Fehlig 20 Apr '16

20 Apr '16

This series adds support for UEFI, via OVMF, to the libxl driver. Patch1 moves the capabilities code from libxl_conf.{ch} to the new files libxl_capabilities.{ch}. Patch2 implements connectGetDomainCapabilities, allowing advertisement of the default OVMF firmware path. Patch3 implements conversion of xl.cfg <-> libvirt domXML. And patch4 finally maps the relevant virDomainLoaderDef fields to libxl_domain_build_info. Jim Fehlig (4): libxl: introduce libxl_capabilities.{ch} libxl: implement connectGetDomainCapabilities xenconfig: support bios=ovmf xl.cfg libxl: Add support for ovmf firmware po/POTFILES.in | 1 + src/Makefile.am | 11 +- src/libxl/libxl_capabilities.c | 557 +++++++++++++++++++++ src/libxl/libxl_capabilities.h | 54 ++ src/libxl/libxl_conf.c | 423 +--------------- src/libxl/libxl_conf.h | 18 +- src/libxl/libxl_domain.c | 1 + src/libxl/libxl_driver.c | 69 +++ src/xenconfig/xen_xl.c | 50 +- tests/Makefile.am | 5 + tests/domaincapsschemadata/domaincaps-xenfv.xml | 51 ++ tests/domaincapsschemadata/domaincaps-xenpv.xml | 44 ++ tests/domaincapstest.c | 33 ++ tests/testutilsxen.h | 1 + tests/xlconfigdata/test-fullvirt-ovmf-override.cfg | 27 + tests/xlconfigdata/test-fullvirt-ovmf-override.xml | 58 +++ tests/xlconfigdata/test-fullvirt-ovmf.cfg | 26 + tests/xlconfigdata/test-fullvirt-ovmf.xml | 58 +++ tests/xlconfigtest.c | 2 + 19 files changed, 1046 insertions(+), 443 deletions(-) create mode 100644 src/libxl/libxl_capabilities.c create mode 100644 src/libxl/libxl_capabilities.h create mode 100644 tests/domaincapsschemadata/domaincaps-xenfv.xml create mode 100644 tests/domaincapsschemadata/domaincaps-xenpv.xml create mode 100644 tests/xlconfigdata/test-fullvirt-ovmf-override.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-ovmf-override.xml create mode 100644 tests/xlconfigdata/test-fullvirt-ovmf.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-ovmf.xml -- 2.1.4

1 4

[libvirt] [PATCH] qemu: process: comment on min_guarantee validation
by Cole Robinson 20 Apr '16

20 Apr '16

Explain why we check it at process startup time, and not parse time where most other XML validation checks are performed --- src/qemu/qemu_process.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c087300..628b4b6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4550,6 +4550,8 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) return -1; + /* Previously we silently accepted this parameter; we can't reject + it at parse time without breaking those configs, so check it here */ if (vm->def->mem.min_guarantee) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Parameter 'min_guarantee' " -- 2.7.3

2 4