September 2016 - Devel - libvirt List Archives

[libvirt] [PATCH 0/4] Introduce aliases for virt-admin's srv-* commands
by Erik Skultety 14 Sep '16

14 Sep '16

So, as suggested by Martin in [1], this series tweaks the command alias handling logic, so that when creating alias, it is not necessary to duplicate the original command's structure, thus creating a bit more noise. Instead, the new format for an alias structure hints very clearly that it's just an alias for a different command, while linking the alias with the original command's data by using new element '.alias'. [1] https://www.redhat.com/archives/libvir-list/2016-September/msg00129.html Erik Skultety (4): tests: fix incorrect status handling by virsh-self-test virt-admin: Tweak command parsing logic so that aliases point to new commands virt-admin: Add some command aliases to provide syntax sugar over ugly commands virt-admin: Replace the (now) aliases with new command names in the man page tests/virsh-self-test | 2 +- tools/virsh-nodedev.c | 6 ++---- tools/virsh.c | 3 ++- tools/virt-admin.c | 24 ++++++++++++++++++++++++ tools/virt-admin.pod | 30 +++++++++++++++--------------- tools/vsh.c | 6 ++++++ tools/vsh.h | 1 + 7 files changed, 51 insertions(+), 21 deletions(-) -- 2.5.5

2 9

[libvirt] [PATCH] configure: Check for major() more strictly
by Michal Privoznik 13 Sep '16

13 Sep '16

Thing is, in f3f15cc24 I'm trying to adapt libvirt to the newest glibc where major()/minor()/makedev() are moved from sys/types.h to sys/sysmacros.h. However, my commit back then expect autoconf to be fixed too as we already use AC_HEADER_MAJOR to determine which header file the functions are in, but because the header files just trigger a warning and not a compile error, the autoconf macro detects the bad header file. This is just a workaround until autoconf macro is fixed. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- configure.ac | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure.ac b/configure.ac index f6076bd..f322c4a 100644 --- a/configure.ac +++ b/configure.ac @@ -144,7 +144,10 @@ AC_TYPE_UID_T dnl Support building Win32 DLLs (must appear *before* AM_PROG_LIBTOOL) AC_LIBTOOL_WIN32_DLL +old_CFLAGS=$CFLAGS +CFLAGS="$CFLAGS -Werror" AC_HEADER_MAJOR +CFLAGS=$old_CFLAGS m4_ifndef([LT_INIT], [ AM_PROG_LIBTOOL -- 2.8.4

2 1

[libvirt] qemu: migration: shall we abort migration while the guest is rebooting？
by Zhangbo (Oscar) 13 Sep '16

13 Sep '16

Hi all: Here's the steps we produce the problem: 1 reboot guest with the flag of VIR_DOMAIN_REBOOT_ACPI_POWER_BTN 2 sleep 1 second (so that the guest is still rebooting, although the API already returned.) 3 migrate the guest The problem is that : the guest failed to migrate to the dest, and crashed on source side. We don't bother to dig further into the problem, the root cause we think is that we migrate a guest while it's rebooting. So, shall we 1 make the reboot JOB LOCK longer enough until the guest has already rebooted(got monitor message from qemu)? 2 let openstack do the mutex work ? (has openstack already done this?) (I checked openstack codes, and found that openstack uses shutdown and create APIs rather than REBOOT to do the reboot work. So it seems that openstack doesn't take care of this problem nowadays. Am I right? --- see: nova/virt/libvirt/driver.py: _soft_reboot) Thanks in advance. Zhang Bo (Oscar)

2 4

[libvirt] [PATCH] qemu: Add missing 'p' to qemuCgrouEmulatorAllNodesRestore
by Peter Krempa 13 Sep '16

13 Sep '16

--- Pushed as trivial. src/qemu/qemu_cgroup.c | 6 +++--- src/qemu/qemu_cgroup.h | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index e2b5bab..fe94613 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1149,7 +1149,7 @@ qemuCgroupEmulatorAllNodesDataFree(qemuCgroupEmulatorAllNodesDataPtr data) * Allows all NUMA nodes for the qemu emulator thread temporarily. This is * necessary when hotplugging cpus since it requires memory allocated in the * DMA region. Afterwards the operation can be reverted by - * qemuCgrouEmulatorAllNodesRestore. + * qemuCgroupEmulatorAllNodesRestore. * * Returns 0 on success -1 on error */ @@ -1196,14 +1196,14 @@ qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, /** - * qemuCgrouEmulatorAllNodesRestore: + * qemuCgroupEmulatorAllNodesRestore: * @data: data structure created by qemuCgroupEmulatorAllNodesAllow * * Rolls back the setting done by qemuCgroupEmulatorAllNodesAllow and frees the * associated data. */ void -qemuCgrouEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data) +qemuCgroupEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data) { virErrorPtr err; diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index e6ebae0..623823e 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -66,6 +66,6 @@ struct _qemuCgroupEmulatorAllNodesData { int qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, qemuCgroupEmulatorAllNodesDataPtr *data); -void qemuCgrouEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data); +void qemuCgroupEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data); #endif /* __QEMU_CGROUP_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 12b84c4..dda82d3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4891,7 +4891,7 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuCgrouEmulatorAllNodesRestore(emulatorCgroup); + qemuCgroupEmulatorAllNodesRestore(emulatorCgroup); virBitmapFree(vcpumap); return ret; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7596579..cecd321 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4925,7 +4925,7 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuCgrouEmulatorAllNodesRestore(emulatorCgroup); + qemuCgroupEmulatorAllNodesRestore(emulatorCgroup); VIR_FREE(bootHotplug); virJSONValueFree(vcpuprops); return ret; -- 2.10.0

1 0

[libvirt] [PATCH] Make sure sys/types.h is included after sys/sysmacros.h
by Michal Privoznik 13 Sep '16

13 Sep '16

In the latest glibc, major() and minor() functions are marked as deprecated (glibc commit dbab6577): CC util/libvirt_util_la-vircgroup.lo util/vircgroup.c: In function 'virCgroupGetBlockDevString': util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated: In the GNU C Library, `major' is defined by <sys/sysmacros.h>. For historical compatibility, it is currently defined by <sys/types.h> as well, but we plan to remove this soon. To use `major', include <sys/sysmacros.h> directly. If you did not intend to use a system-defined macro `major', you should #undef it after including <sys/types.h>. [-Werror=deprecated-declarations] if (virAsprintf(&ret, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0) ^~ In file included from /usr/include/features.h:397:0, from /usr/include/bits/libc-header-start.h:33, from /usr/include/stdio.h:28, from ../gnulib/lib/stdio.h:43, from util/vircgroup.c:26: /usr/include/sys/sysmacros.h:87:1: note: declared here __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL) ^ Moreover, in the glibc commit, there's suggestion to keep ordering of including of header files as implemented here. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- This still won't solve the build issue completely as AC_HEADER_MAJOR still reports that major() is defined by sys/types.h instead of sys/sysmacros.h. But once they fix it, we are good too. Or we can use the following workaround in configure.ac: +old_CFLAGS=$CFLAGS +CFLAGS+=" -Werror " AC_HEADER_MAJOR +CFLAGS=$old_CFLAGS src/conf/domain_audit.c | 3 ++- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/util/vircgroup.c | 2 +- src/util/virutil.c | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 53a58ac..52dea02 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -24,7 +24,6 @@ #include <config.h> #include <sys/stat.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -32,6 +31,8 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> + #include "domain_audit.h" #include "viraudit.h" #include "viruuid.h" diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 825b4d4..8c581df 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -27,7 +27,6 @@ #include <sys/epoll.h> #include <sys/wait.h> #include <sys/socket.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -35,6 +34,7 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> #include <sys/un.h> #include <sys/personality.h> #include <unistd.h> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index da98b38..24025d1 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -28,7 +28,6 @@ #include <sched.h> #include <sys/utsname.h> #include <string.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -36,6 +35,7 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> #include <sys/socket.h> #include <sys/stat.h> #include <sys/un.h> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index f2477d5..8b52816 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -34,7 +34,6 @@ #include <errno.h> #include <stdlib.h> #include <sys/stat.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -42,6 +41,7 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> #include <signal.h> #include <dirent.h> #include <unistd.h> diff --git a/src/util/virutil.c b/src/util/virutil.c index 170dd59..b57a195 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -35,7 +35,6 @@ #include <errno.h> #include <poll.h> #include <sys/stat.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -43,6 +42,7 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> #include <sys/ioctl.h> #include <string.h> #include <termios.h> -- 2.8.4

3 5

[libvirt] [PATCH 0/3] qemu: fix startup of NUMA pinned VM with hotpluggable vcpus
by Peter Krempa 13 Sep '16

13 Sep '16

See patch 2 for explanation. Peter Krempa (3): qemu: cgroup: Extract temporary relaxing of cgroup setting for vcpu hotplug qemu: process: Fix start with unpluggable vcpus with NUMA pinning qemu: driver: Remove unnecessary condition src/qemu/qemu_cgroup.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_cgroup.h | 11 ++++++ src/qemu/qemu_driver.c | 38 +++------------------ src/qemu/qemu_process.c | 6 ++++ 4 files changed, 112 insertions(+), 33 deletions(-) -- 2.10.0

3 5

[libvirt] [PATCH 0/3] add option to keep nvram file on undefine
by Nikolay Shirokovskiy 13 Sep '16

13 Sep '16

There is already a patch [1] on this topic with a different approach - keep nvram file by default. There is also some discussion there. To sum up keeping nvram on undefine could be useful in some usecases so there should be an option to do it. On the other hand there is a danger of leaving domain assets after its undefine and unsing them unintentionally on defining domain with the same name. AFAIU keeping nvram by default was motivated by domain disks behaviour. I think there is a difference as libvirt never create disks for domain as opposed to nvram and managed save and without disks domain will not start so user is quite aware of disks files. On the other hand one can start using nvram file solely putting <nvram> in config and managed save is created on daemon shutdown. So user is much less aware of nvram and managed save existence. Thus one can easily mess up by unaware define $name/using/undefine/define $name again usecase. Thus I vote for keeping said assets only if it is specified explicitly so user knows what he is doing. Adding option to undefine is best solution I come up with. The other options are add checks on define or start and both are impossible. Such a check should be done without any extra flags for it to be useful but this way we break existing users. As this a proof of concept this series does not add extra flag for managed save. [1] https://www.redhat.com/archives/libvir-list/2015-February/msg00915.html Nikolay Shirokovskiy (3): api: add VIR_DOMAIN_UNDEFINE_KEEP_NVRAM flag qemu: add VIR_DOMAIN_UNDEFINE_KEEP_NVRAM support virsh: add --keep-nvram option to undefine command include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 26 +++++++++++++++++--------- tools/virsh-domain.c | 8 ++++++++ tools/virsh.pod | 6 +++--- 4 files changed, 29 insertions(+), 12 deletions(-) -- 1.8.3.1

5 10

[libvirt] [PATCH] conf: allow hotplugging "legacy PCI" device to manually addressed PCIe slot
by Laine Stump 13 Sep '16

13 Sep '16

In a full domain config, libvirt allows overriding the normal PCI vs. PCI Express rules when a device address is explicitly provided (so, e.g., you can force a legacy PCI device to plug into a PCIe port, although libvirt would never do that on its own). However, due to a bug libvirt doesn't give this same leeway when hotplugging devices. On top of that, current libvirt assumes that *all* devices are legacy PCI. The result of all this is that it's impossible to hotplug a device into a PCIe port, even if you manually add the PCI address. This can all be traced to the function virDomainPCIAddressEnsureAddr(), and the fact that it calls virDomainPCIaddressReserveSlot() for manually set addresses, and that function hardcodes the argument "fromConfig" to false (meaning "this address was auto-assigned, so it should be subject to stricter validation"). Since virDomainPCIAddressReserveSlot() is just a one line simple wrapper around virDomainPCIAddressReserveAddr() (adding in a hardcoded reserveEntireSlot = true and fromConfig = false), all that's needed to solve the problem with no unwanted side effects is to replace that call for virDomainPCIAddressReserveSlot() with a direct call to virDomainPCIAddressReserveAddr(), but with reserveEntireSlot = true, fromConfig = true. That's what this patch does. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1337490 --- src/conf/domain_addr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 79c024f..93026c2 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -507,7 +507,8 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, addrStr, flags, true)) goto cleanup; - ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags); + ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci, flags, + true, true); } else { ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags); } -- 2.7.4

2 2

[libvirt] [PATCH] audit: Audit information about shmem devices
by Martin Kletzander 13 Sep '16

13 Sep '16

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1218603 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- docs/auditlog.html.in | 19 +++++++++++++++++++ src/conf/domain_audit.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_audit.h | 4 ++++ src/libvirt_private.syms | 1 + 4 files changed, 67 insertions(+) diff --git a/docs/auditlog.html.in b/docs/auditlog.html.in index 012d0680a87b..7348948feb7a 100644 --- a/docs/auditlog.html.in +++ b/docs/auditlog.html.in @@ -352,5 +352,24 @@ <dd>The name of the cgroup controller</dd> </dl> + + <h4><a name="typeresourceshmem">console/serial/parallel/channel</a></h4> + <p> + The <code>msg</code> field will include the following sub-fields + </p> + + <dl> + <dt><code>resrc</code></dt> + <dd>The type of resource assigned. Set to <code>shmem</code></dd> + <dt><code>reason</code></dt> + <dd>The reason which caused the resource to be assigned to happen</dd> + <dt><code>size</code></dt> + <dd>The size of the shared memory region</dd> + <dt><code>shmem</code></dt> + <dd>Name of the shared memory region</dd> + <dt><code>source</code></dt> + <dd>Path of the backing character device for given emulated device</dd> + </dl> + </body> </html> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 53a58ac4c88c..cf9c03dee749 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -889,6 +889,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) if (vm->def->tpm) virDomainAuditTPM(vm, vm->def->tpm, "start", true); + for (i = 0; i < vm->def->nshmems; i++) + virDomainAuditShmem(vm, vm->def->shmems[i], "start", true); + virDomainAuditMemory(vm, 0, virDomainDefGetMemoryTotal(vm->def), "start", true); virDomainAuditVcpu(vm, 0, virDomainDefGetVcpus(vm->def), "start", true); @@ -963,3 +966,43 @@ virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success) VIR_FREE(vmname); } + +void +virDomainAuditShmem(virDomainObjPtr vm, + virDomainShmemDefPtr def, + const char *reason, bool success) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *vmname = virAuditEncode("vm", vm->def->name); + const char *srcpath = virDomainAuditChardevPath(&def->server.chr); + char *src = virAuditEncode("server", VIR_AUDIT_STR(srcpath)); + char *shmem = virAuditEncode("shmem", VIR_AUDIT_STR(def->name)); + const char *virt = virDomainVirtTypeToString(vm->def->virtType); + char *size = NULL; + + ignore_value(virAsprintfQuiet(&size, "%llu", def->size)); + virUUIDFormat(vm->def->uuid, uuidstr); + if (!vmname || !src || !size || !shmem || !size) + goto no_memory; + + if (!virt) { + VIR_WARN("Unexpected virt type %d while encoding audit message", + vm->def->virtType); + virt = "?"; + } + + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, + "virt=%s resrc=shmem reason=%s %s uuid=%s size=%s %s %s", + virt, reason, vmname, uuidstr, size ?: "?", shmem, src); + + cleanup: + VIR_FREE(vmname); + VIR_FREE(src); + VIR_FREE(size); + VIR_FREE(shmem); + return; + + no_memory: + VIR_WARN("OOM while encoding audit message"); + goto cleanup; +} diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 97dadca5aae7..8cb585dc7d96 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -129,6 +129,10 @@ void virDomainAuditRNG(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditShmem(virDomainObjPtr vm, + virDomainShmemDefPtr def, + const char *reason, bool success) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); #endif /* __VIR_DOMAIN_AUDIT_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a5fa30562f0c..1555f60ffda4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -146,6 +146,7 @@ virDomainAuditNetDevice; virDomainAuditRedirdev; virDomainAuditRNG; virDomainAuditSecurityLabel; +virDomainAuditShmem; virDomainAuditStart; virDomainAuditStop; virDomainAuditVcpu; -- 2.10.0

2 2

[libvirt] [PATCH] qemu: fix improper initialization of cgroupControllers bitmap
by Laine Stump 13 Sep '16

13 Sep '16

virQEMUDriverConfigNew() always initializes the bitmap in its cgroupControllers member to -1 (i.e. all 1's). Prior to commit a9331394, if qemu.conf had a line with "cgroup_controllers", cgroupControllers would get reset to 0 before going through a loop setting a bit for each named cgroup controller. commit a9331394 left out the "reset to 0" part, so cgroupControllers would always be -1; if you didn't want a controller included, there was no longer a way to make that happen. This was discovered by users who were using qemu commandline passthrough to use the "input-linux" method of directing keyboard/mouse input to a virtual machine: https://www.redhat.com/archives/vfio-users/2016-April/msg00105.html Here's the first report I found of the problem encountered after upgrading libvirt beyond v2.0.0: https://www.redhat.com/archives/vfio-users/2016-August/msg00053.html Thanks to sL1pKn07 SpinFlo <sl1pkn07(a)gmail.com> for bringing the problem up in IRC, and then taking the time to do a git bisect and find the patch that started the problem. --- src/qemu/qemu_conf.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e7b2d8d..dad8334 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -613,15 +613,18 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, &controllers) < 0) goto cleanup; - for (i = 0; controllers != NULL && controllers[i] != NULL; i++) { - int ctl; - if ((ctl = virCgroupControllerTypeFromString(controllers[i])) < 0) { - virReportError(VIR_ERR_CONF_SYNTAX, - _("Unknown cgroup controller '%s'"), - controllers[i]); - goto cleanup; + if (controllers) { + cfg-> cgroupControllers = 0; + for (i = 0; controllers[i] != NULL; i++) { + int ctl; + if ((ctl = virCgroupControllerTypeFromString(controllers[i])) < 0) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("Unknown cgroup controller '%s'"), + controllers[i]); + goto cleanup; + } + cfg->cgroupControllers |= (1 << ctl); } - cfg->cgroupControllers |= (1 << ctl); } if (virConfGetValueStringList(conf, "cgroup_device_acl", false, -- 2.7.4

2 1