July 2015 - Devel - libvirt List Archives

[libvirt] [PATCH 0/4] Improve migration errors
by Jiri Denemark 10 Jul '15

10 Jul '15

When QEMU exits on destination during migration, the source reports either success (if the failure happened at the very end) or unhelpful "unexpectedly failed" error message, only the destination host knows more about the error, but usually there's no way to get the error back to the source because the domain is already gone at the time virDomainMigrateFinish* is called. This series changes libvirt to remember incoming migration errors so that they can be sent back to a source host. https://bugzilla.redhat.com/show_bug.cgi?id=1090093 Jiri Denemark (4): Introduce virHashLockable qemu: Remember incoming migration errors qemu: Don't report false error from MigrateFinish qemu: Use error from Finish instead of "unexpectedly failed" include/libvirt/virterror.h | 1 + src/libvirt-domain.c | 21 ++++++++++- src/libvirt_private.syms | 3 ++ src/qemu/qemu_conf.h | 3 ++ src/qemu/qemu_driver.c | 31 +++++++++++---- src/qemu/qemu_migration.c | 91 +++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_migration.h | 7 ++++ src/qemu/qemu_monitor.c | 19 ++++++++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_process.c | 4 ++ src/util/virerror.c | 3 ++ src/util/virhash.c | 81 ++++++++++++++++++++++++++++++++++++++++ src/util/virhash.h | 10 +++++ 13 files changed, 263 insertions(+), 13 deletions(-) -- 2.4.5

2 10

[libvirt] [libvirt-snmp][PATCH] LIBVIRT-MIB: Comply with formatting rules
by Michal Privoznik 10 Jul '15

10 Jul '15

So, I've came along this web site [1] which tries to validate a MIB file. So I fed it with LIBVIRT-MIB and found some errors. This commit fixes them so that the test passes again. This should be no functional change, rather than some formatting change. 1: http://www.simpleweb.org/ietf/mibs/validate/ Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/LIBVIRT-MIB.txt | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/LIBVIRT-MIB.txt b/src/LIBVIRT-MIB.txt index d83db49..a8195ff 100644 --- a/src/LIBVIRT-MIB.txt +++ b/src/LIBVIRT-MIB.txt @@ -6,12 +6,10 @@ LIBVIRT-MIB DEFINITIONS ::= BEGIN IMPORTS MODULE-IDENTITY, OBJECT-TYPE, enterprises, Unsigned32, Gauge32, - Counter64 - FROM SNMPv2-SMI - OBJECT-GROUP, MODULE-COMPLIANCE - FROM SNMPv2-CONF - TEXTUAL-CONVENTION, RowStatus - FROM SNMPv2-TC + Counter64 FROM SNMPv2-SMI + OBJECT-GROUP, MODULE-COMPLIANCE FROM SNMPv2-CONF + TEXTUAL-CONVENTION, RowStatus FROM SNMPv2-TC + NOTIFICATION-TYPE FROM SNMPv2-SMI ; @@ -78,7 +76,7 @@ LibvirtGuestEntry ::= SEQUENCE { libvirtGuestUUID OBJECT-TYPE SYNTAX UUID - MAX-ACCESS not-accessible + MAX-ACCESS accessible-for-notify STATUS current DESCRIPTION "The UUID of the virtual guest." @@ -202,11 +200,11 @@ libvirtGuestGroup OBJECT-GROUP ::= { libvirtGroups 1 } libvirtGuestNotif NOTIFICATION-TYPE - STATUS current OBJECTS { libvirtGuestName, libvirtGuestUUID, libvirtGuestState, libvirtGuestRowStatus } + STATUS current DESCRIPTION "Guest lifecycle notification." ::= { libvirtNotifications 1 } -- 2.3.6

1 0

[libvirt] [PATCH 0/2] Fix timerActive logic in virNetDaemon
by Martin Kletzander 10 Jul '15

10 Jul '15

Nowadays the daemon can quit even if there's a client connected, so let's fix that. If ACK'd, I'll backport this to v1.2.17-maint as well as v1.2.17 is the only affected release. Martin Kletzander (2): rpc: Add virNetDaemonHasClients rpc: Rework timerActive logic in daemon src/libvirt_remote.syms | 1 + src/rpc/virnetdaemon.c | 37 ++++++++++++++++++++++--------------- src/rpc/virnetdaemon.h | 2 ++ 3 files changed, 25 insertions(+), 15 deletions(-) -- 2.4.5

2 3

[libvirt] [PATCH] Document that virNodeGetInfo can return mhz == 0.
by Richard W.M. Jones 10 Jul '15

10 Jul '15

On the s/390x architecture, libvirt may already return 0 in the node_info->mhz field (see src/nodeinfo.c:linuxNodeInfoCPUPopulate). We may also want to return this on aarch64 in future, because calculating the proper value requires SMBIOS, which is not available on non-server-class systems (specifically on systems which don't adhere to the SBSA standard). Therefore this change documents the existing behaviour and provides a valid path for aarch64. Signed-off-by: Richard W.M. Jones <rjones(a)redhat.com> Bug-URL: https://bugzilla.redhat.com/1206353 --- include/libvirt/libvirt-host.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 953366b..070550b 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -354,7 +354,8 @@ struct _virNodeInfo { char model[32]; /* string indicating the CPU model */ unsigned long memory; /* memory size in kilobytes */ unsigned int cpus; /* the number of active CPUs */ - unsigned int mhz; /* expected CPU frequency */ + unsigned int mhz; /* expected CPU frequency, 0 if not known or + on unusual architectures */ unsigned int nodes; /* the number of NUMA cell, 1 for unusual NUMA topologies or uniform memory access; check capabilities XML for the actual NUMA topology */ -- 2.3.1

3 3

[libvirt] [PATCH] qemu: Reject updating unsupported disk information
by Martin Kletzander 10 Jul '15

10 Jul '15

If one calls update-device with information that is not updatable, libvirt reports success even though no data were updated. The example used in the bug linked below uses updating device with <boot order='2'/> which, in my opinion, is a valid thing to request from user's perspective. Mainly since we properly error out if user wants to update such data on a network device for example. And since there are many things that might happen (update-device on disk basically knows just how to change removable media), check for what's changing and moreover, since the function might be usable in other dirvers (updating only disk path is a valid possibility) let's abstract it for any two disks. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1007228 --- src/conf/domain_conf.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 3 ++ 4 files changed, 117 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0219c3c4814d..a6950087d987 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5687,6 +5687,117 @@ virDomainDiskFindByBusAndDst(virDomainDefPtr def, return NULL; } + +/* + * Makes sure the @disk differs from @orig_disk only by the source + * path and nothing else. Fields that are being checked and the + * information whether they are nullable (can be NULL) or is taken + * from the virDomainDiskDefFormat() code. + */ +bool +virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk, + virDomainDiskDefPtr orig_disk) +{ +#define CHECK_EQ(field, field_name, nullable) \ + do { \ + if (nullable && !disk->field) \ + break; \ + if (disk->field != orig_disk->field) { \ + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, \ + _("cannot modify field '%s' of the disk"), \ + field_name); \ + return false; \ + } \ + } while (0) + + CHECK_EQ(device, "device", false); + CHECK_EQ(cachemode, "cache", true); + CHECK_EQ(error_policy, "error_policy", true); + CHECK_EQ(rerror_policy, "rerror_policy", true); + CHECK_EQ(iomode, "io", true); + CHECK_EQ(ioeventfd, "ioeventfd", true); + CHECK_EQ(event_idx, "event_idx", true); + CHECK_EQ(copy_on_read, "copy_on_read", true); + CHECK_EQ(discard, "discard", true); + CHECK_EQ(iothread, "iothread", true); + + if (disk->geometry.cylinders && + disk->geometry.heads && + disk->geometry.sectors) { + CHECK_EQ(geometry.cylinders, "geometry cylinders", false); + CHECK_EQ(geometry.heads, "geometry heads", false); + CHECK_EQ(geometry.sectors, "geometry sectors", false); + CHECK_EQ(geometry.trans, "BIOS-translation-modus", true); + } + + CHECK_EQ(blockio.logical_block_size, + "blockio logical_block_size", false); + CHECK_EQ(blockio.physical_block_size, + "blockio physical_block_size", false); + + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) + CHECK_EQ(removable, "removable", true); + + CHECK_EQ(blkdeviotune.total_bytes_sec, + "blkdeviotune.total_bytes_sec", + true); + CHECK_EQ(blkdeviotune.read_bytes_sec, + "blkdeviotune.read_bytes_sec", + true); + CHECK_EQ(blkdeviotune.write_bytes_sec, + "blkdeviotune.write_bytes_sec", + true); + CHECK_EQ(blkdeviotune.total_iops_sec, + "blkdeviotune.total_iops_sec", + true); + CHECK_EQ(blkdeviotune.read_iops_sec, + "blkdeviotune.read_iops_sec", + true); + CHECK_EQ(blkdeviotune.write_iops_sec, + "blkdeviotune.write_iops_sec", + true); + CHECK_EQ(blkdeviotune.total_bytes_sec_max, + "blkdeviotune.total_bytes_sec_max", + true); + CHECK_EQ(blkdeviotune.read_bytes_sec_max, + "blkdeviotune.read_bytes_sec_max", + true); + CHECK_EQ(blkdeviotune.write_bytes_sec_max, + "blkdeviotune.write_bytes_sec_max", + true); + CHECK_EQ(blkdeviotune.total_iops_sec_max, + "blkdeviotune.total_iops_sec_max", + true); + CHECK_EQ(blkdeviotune.read_iops_sec_max, + "blkdeviotune.read_iops_sec_max", + true); + CHECK_EQ(blkdeviotune.write_iops_sec_max, + "blkdeviotune.write_iops_sec_max", + true); + CHECK_EQ(blkdeviotune.size_iops_sec, + "blkdeviotune.size_iops_sec", + true); + + CHECK_EQ(transient, "transient", true); + CHECK_EQ(serial, "serial", true); + CHECK_EQ(wwn, "wwn", true); + CHECK_EQ(vendor, "vendor", true); + CHECK_EQ(product, "product", true); + + CHECK_EQ(info.bootIndex, "boot order", true); + + if (disk->info.alias && STRNEQ(disk->info.alias, orig_disk->info.alias)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("cannot modify field '%s' of the disk"), + "alias name"); + return false; + } + +#undef CHECK_EQ + + return true; +} + int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, virDomainDiskDefPtr def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 12d945ea7f24..b84640c2de56 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2475,6 +2475,8 @@ int virDomainDeviceFindControllerModel(virDomainDefPtr def, virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def, int bus, char *dst); +bool virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk, + virDomainDiskDefPtr orig_disk); void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainActualNetDefFree(virDomainActualNetDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1566d11e4156..5534dc7ed704 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -249,6 +249,7 @@ virDomainDiskDefFree; virDomainDiskDefNew; virDomainDiskDefSourceParse; virDomainDiskDeviceTypeToString; +virDomainDiskDiffersSourceOnly; virDomainDiskDiscardTypeToString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ba804429a28a..4f1724e92e4a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7916,6 +7916,9 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, goto end; } + if (!virDomainDiskDiffersSourceOnly(disk, orig_disk)) + goto end; + /* Add the new disk src into shared disk hash table */ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) goto end; -- 2.4.5

3 3

[libvirt] [PATCH] qemu: Check duplicate WWNs also for hotplugged disks
by Peter Krempa 10 Jul '15

10 Jul '15

In commit 714b38cb232bcbbd7487af4c058fa6d0999b3326 I tried to avoid having two disks with the same WWN in a VM. I forgot to check the hotplug paths though which make it possible bypass that check. Reinforce the fix by checking the wwn when attaching the disk. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1208009 --- src/conf/domain_conf.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1f7862b..5a9a88d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -22135,6 +22135,34 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } + +/** + * virDomainDefGetDiskByWWN: + * @def: domain definition + * @wwn: wwn of a disk to find + * + * Returns a disk definition pointer corresponding to the given WWN identifier + * or NULL either if @wwn was NULL or if disk with given WWN is not present in + * the domain definition. + */ +static virDomainDiskDefPtr +virDomainDefGetDiskByWWN(virDomainDefPtr def, + const char *wwn) +{ + size_t i; + + if (!wwn) + return NULL; + + for (i = 0; i < def->ndisks; i++) { + if (STREQ_NULLABLE(def->disks[i]->wwn, wwn)) + return def->disks[i]; + } + + return NULL; +} + + int virDomainDefCompatibleDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev, @@ -22178,6 +22206,15 @@ virDomainDefCompatibleDevice(virDomainDefPtr def, } } + if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (!!virDomainDefGetDiskByWWN(def, dev->data.disk->wwn)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Domain already has a disk with wwn '%s'"), + dev->data.disk->wwn); + return -1; + } + } + return 0; } -- 2.4.5

2 2

[libvirt] [PATCH] cpu: Add support for MPX and AVX512 Intel features
by Jiri Denemark 10 Jul '15

10 Jul '15

Corresponding QEMU commits: MPX 79e9ebebbf2a00c46fcedb6dc7dd5e12bbd30216 AVX512 9aecd6f8aef653cea58932f06a2740299dbe5fd3 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/cpu/cpu_map.xml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index b9e95cf..b924bd3 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -317,6 +317,12 @@ <feature name='rtm'> <cpuid function='0x00000007' ebx='0x00000800'/> </feature> + <feature name='mpx'> + <cpuid function='0x00000007' ebx='0x00004000'/> + </feature> + <feature name='avx512f'>  + <cpuid function='0x00000007' ebx='0x00010000'/> + </feature> <feature name='rdseed'> <cpuid function='0x00000007' ebx='0x00040000'/> </feature> @@ -326,6 +332,15 @@ <feature name='smap'> <cpuid function='0x00000007' ebx='0x00100000'/> </feature> + <feature name='avx512pf'>  + <cpuid function='0x00000007' ebx='0x04000000'/> + </feature> + <feature name='avx512er'>  + <cpuid function='0x00000007' ebx='0x08000000'/> + </feature> + <feature name='avx512cd'>  + <cpuid function='0x00000007' ebx='0x10000000'/> + </feature>  <feature name='invtsc' migratable='no'> -- 2.4.5

2 1

[libvirt] [PATCH v3] qemu: Use heads parameter for QXL driver
by Frediano Ziglio 10 Jul '15

10 Jul '15

Allows to specify maximum number of head to QXL driver. The patch to support the "max_outputs" in Qemu is still not merged but I got agreement on the name of the argument. Actually can be a compatiblity problem as heads in the XML configuration was set by default to '1'. Signed-off-by: Frediano Ziglio <fziglio(a)redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 +++++ 3 files changed, 8 insertions(+) Changes from v2: - removed capability tests (Martin Kletzander) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 27686c3..68060cd 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -287,6 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "aarch64-off", "vhost-user-multiqueue", /* 190 */ + "qxl-vga.max_outputs", ); @@ -1649,6 +1650,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxl[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = { { "vgamem_mb", QEMU_CAPS_QXL_VGA_VGAMEM }, + { "max_outputs", QEMU_CAPS_QXL_VGA_MAX_OUTPUTS }, }; struct virQEMUCapsObjectTypeProps { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 30aa504..02f9e81 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -230,6 +230,7 @@ typedef enum { QEMU_CAPS_DEVICE_PCI_SERIAL = 188, /* -device pci-serial */ QEMU_CAPS_CPU_AARCH64_OFF = 189, /* -cpu ...,aarch64=off */ QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */ + QEMU_CAPS_QXL_VGA_MAX_OUTPUTS = 191, /* qxl-vga.max_outputs */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 25a7bc6..59666e7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5661,6 +5661,11 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, /* QEMU accepts mebibytes for vgamem_mb. */ virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vgamem / 1024); } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) && + video->heads > 0) { + virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); + } } else if (video->vram && ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || -- 2.1.0

2 2

[libvirt] [PATCH] qemu: don't use initialized ret in qemuRemoveSharedDevice
by Guido Günther 09 Jul '15

09 Jul '15

This fixes CC qemu/libvirt_driver_qemu_impl_la-qemu_conf.lo qemu/qemu_conf.c: In function 'qemuRemoveSharedDevice': qemu/qemu_conf.c:1384:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] --- This is only catched by Debian Wheezy's gcc 4.7.2, Jessies gcc 4.9.2 doesn't trap on it. src/qemu/qemu_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2ab5494..38d4a86 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1381,7 +1381,7 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver, { char *dev_path = NULL; char *key = NULL; - int ret; + int ret = -1; if (!qemuIsSharedHostdev(hostdev)) return 0; -- 2.1.4

3 3

[libvirt] [PATCH] qemu: Log all arguments of qemuProcessStart
by Jiri Denemark 09 Jul '15

09 Jul '15

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_process.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ba84182..ee522b9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4302,9 +4302,10 @@ int qemuProcessStart(virConnectPtr conn, size_t nnicindexes = 0; int *nicindexes = NULL; - VIR_DEBUG("vm=%p name=%s id=%d pid=%llu", - vm, vm->def->name, vm->def->id, - (unsigned long long)vm->pid); + VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d migrateFrom=%s stdin_fd=%d " + "stdin_path=%s snapshot=%p vmop=%d flags=0x%x", + vm, vm->def->name, vm->def->id, asyncJob, NULLSTR(migrateFrom), + stdin_fd, NULLSTR(stdin_path), snapshot, vmop, flags); /* Okay, these are just internal flags, * but doesn't hurt to check */ -- 2.4.5

2 2