- Devel - libvirt List Archives

[libvirt] [PATCH v2.1 RESEND 00/11] Rework storage migration
by Michal Privoznik 18 Jan '13

18 Jan '13

This patch set re-implements migration with storage for enough new qemu. Currently, you can migrate a domain to a host without need for shared storage. This is done by setting 'blk' or 'inc' attribute (representing VIR_MIGRATE_NON_SHARED_DISK and VIR_MIGRATE_NON_SHARED_INC flags respectively) of 'migrate' monitor command. However, the qemu implementation is buggy and applications are advised to switch to new impementation which, moreover, offers some nice features, like migrating only explicitly specified disks. The new functionality is controlled via 'nbd-server-*' and 'drive-mirror' commands. The flow is meant to look like this: 1) User invokes libvirt's migrate functionality. 2) libvirt checks that no block jobs are active on the source. 3) libvirt starts the destination QEMU and sets up the NBD server using the nbd-server-start and nbd-server-add commands. 4) libvirt starts drive-mirror with a destination pointing to the remote NBD server, for example nbd:host:port:exportname=diskname (where diskname is the -drive id specified on the destination). 5) once all mirroring jobs reach steady state, libvirt invokes the migrate command. 6) once migration completed, libvirt invokes the nbd-server-stop command on the destination QEMU. If we just skip the 2nd step and there is an active block-job, qemu will fail in step 4. No big deal. Since we try to NOT break migration and keep things compatible, this feature is enabled iff both sides support it. Since there's obvious need for some data transfer between src and dst, I've put it into qemuCookieMigration: 1) src -> dest: (QEMU_MIGRATION_PHASE_BEGIN3 -> QEMU_MIGRATION_PHASE_PREPARE) <nbd> <disk size='17179869184'/> </nbd> Hey destination, I know how to use this cool new feature. Moreover, these are the disks I'll send you. Each one of them is X bytes big. It's one of the prerequisite - the file (disk->src) on dst exists and has at least the same size as on dst. 2) dst -> src: (QEMU_MIGRATION_PHASE_PREPARE -> QEMU_MIGRATION_PHASE_PERFORM3) <nbd port='X'/> Okay, I (destination) support this feature as well. I've created all files as you (src) told me to and you can start rolling data. I am listening on port X. 3) src -> dst: (QEMU_MIGRATION_PHASE_PERFORM3 -> QEMU_MIGRATION_PHASE_FINISH3) <nbd port='-1'/> Migration completed, destination, you may shut the NBD server down. If either src or dst doesn't support NBD, it is not used and whole process fall backs to old implementation. diff to v1: -Eric's and Daniel's suggestions worked in. To point out the bigger ones: don't do NBD style when TUNNELLED requested, added 'b:writable' to 'nbd-server-add' -drop '/qemu-migration/nbd/disk/@src' attribute from migration cookie. As pointed out by Jirka, disk->src can be changed during migration (e.g. by migration hook or by passed xml). So I've tried (as suggested on the list) passing disk alias. However, since qemu hasn't been started on destination yet, the aliases hasn't been generated yet. So we have to rely on ordering completely. diff to v2: -rebase to reflect changes made by offline migration patch -send initial nbd cookie only if needed diff to previous v2.1: -rebase to current HEAD The patches 1,3 and 5 has been ACKed already. Maybe, I'll need to drop 7/11 patch completely, once Dan's patches [1] are in. 1: https://www.redhat.com/archives/libvir-list/2013-January/msg00742.html Michal Privoznik (11): qemu: Introduce NBD_SERVER capability Introduce NBD migration cookie qemu: Introduce nbd-server-start command qemu: Introduce nbd-server-add command qemu: Introduce nbd-server-stop command qemu_migration: Introduce qemuMigrationStartNBDServer qemu_migration: Move port allocation to a separate func qemu_migration: Implement qemuMigrationStartNBDServer() qemu_migration: Implement qemuMigrationDriveMirror qemu_migration: Check size prerequisites qemu_migration: Stop NBD server at Finish phase src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_migration.c | 624 ++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 63 +++++ src/qemu/qemu_monitor.h | 7 + src/qemu/qemu_monitor_json.c | 95 +++++++ src/qemu/qemu_monitor_json.h | 7 + 7 files changed, 768 insertions(+), 33 deletions(-) -- 1.8.0.2

2 23

[libvirt] [RFC] Data in the <topology> element in the capabilities XML
by Peter Krempa 18 Jan '13

18 Jan '13

Hi everybody, a while ago there was a discussion about changing the data that is returned in the <topology> sub-element: <capabilities> <host> <cpu> <arch>x86_64</arch> <model>SandyBridge</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='2'/> The data provided here is as of today taken from the nodeinfo detection code and thus is really wrong when the fallback mechanisms are used. To get a useful count, the user has to multiply the data by the number of NUMA nodes in the host. With the fallback detection code used for nodeinfo the NUMA node count used to get the CPU count should be 1 instead of the actual number. As Jiri proposed, I think we should change this output to separate detection code that will not take into account NUMA nodes for this output and will rather provide data as the "lspci" command does. This change will make the data provided by the element standalone and also usable in guest XMLs to mirror host's topology. The meaning of the attributes of that element isn't really documented anywhere, so as an additional precaution we should document that. ( http://libvirt.org/formatcaps.html ) For some additional background, please refer to the original discussion here: https://www.redhat.com/archives/libvir-list/2012-March/msg01123.html Thanks in advance for your comments and suggestions. Peter

3 16

[libvirt] [PATCH v2] Avoid integer wrap on remotePortMax in QEMU driver
by Daniel P. Berrange 17 Jan '13

17 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The QEMU driver default max port is 65535, but it then increments this by 1 to 65536. This maps to 0 in an unsigned short :-( This was apparently done so that for() loops could use "< max" instead of "<= max". Remove this insanity and just make the loop do the right thing. --- src/qemu/qemu_conf.c | 4 ---- src/util/virportallocator.c | 6 +++--- tests/virportallocatortest.c | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 680e8fb..b21392e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -233,10 +233,6 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, filename, QEMU_REMOTE_PORT_MAX); goto cleanup; } - /* increasing the value by 1 makes all the loops going through - the bitmap (i = remotePortMin; i < remotePortMax; i++), work as - expected. */ - driver->remotePortMax++; if (driver->remotePortMin > driver->remotePortMax) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 033aee4..d80347a 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -84,7 +84,7 @@ virPortAllocatorPtr virPortAllocatorNew(unsigned short start, pa->start = start; pa->end = end; - if (!(pa->bitmap = virBitmapNew(end-start))) { + if (!(pa->bitmap = virBitmapNew((end-start)+1))) { virReportOOMError(); virObjectUnref(pa); return NULL; @@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, *port = 0; virObjectLock(pa); - for (i = pa->start ; i < pa->end && !*port; i++) { + for (i = pa->start ; i <= pa->end && !*port; i++) { int reuse = 1; struct sockaddr_in addr; bool used = false; @@ -168,7 +168,7 @@ int virPortAllocatorRelease(virPortAllocatorPtr pa, virObjectLock(pa); if (port < pa->start || - port >= pa->end) { + port > pa->end) { virReportInvalidArg(port, "port %d must be in range (%d, %d)", port, pa->start, pa->end); goto cleanup; diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c index 93577d7..3f6edcc 100644 --- a/tests/virportallocatortest.c +++ b/tests/virportallocatortest.c @@ -59,7 +59,7 @@ int bind(int sockfd ATTRIBUTE_UNUSED, static int testAllocAll(const void *args ATTRIBUTE_UNUSED) { - virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5910); + virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5909); int ret = -1; unsigned short p1, p2, p3, p4, p5, p6, p7; -- 1.8.0.2

3 2

[libvirt] [PATCH] Avoid integer wrap on remotePortMax in QEMU driver
by Daniel P. Berrange 17 Jan '13

17 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The QEMU driver default max port is 65535, but it then increments this by 1 to 65536. This maps to 0 in an unsigned short :-( This was apparently done so that for() loops could use "< max" instead of "<= max". Remove this insanity and just make the loop do the right thing. --- src/qemu/qemu_conf.c | 4 ---- src/util/virportallocator.c | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 680e8fb..b21392e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -233,10 +233,6 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, filename, QEMU_REMOTE_PORT_MAX); goto cleanup; } - /* increasing the value by 1 makes all the loops going through - the bitmap (i = remotePortMin; i < remotePortMax; i++), work as - expected. */ - driver->remotePortMax++; if (driver->remotePortMin > driver->remotePortMax) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 033aee4..5d07dd0 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -84,7 +84,7 @@ virPortAllocatorPtr virPortAllocatorNew(unsigned short start, pa->start = start; pa->end = end; - if (!(pa->bitmap = virBitmapNew(end-start))) { + if (!(pa->bitmap = virBitmapNew((end-start)+1))) { virReportOOMError(); virObjectUnref(pa); return NULL; @@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, *port = 0; virObjectLock(pa); - for (i = pa->start ; i < pa->end && !*port; i++) { + for (i = pa->start ; i <= pa->end && !*port; i++) { int reuse = 1; struct sockaddr_in addr; bool used = false; -- 1.8.0.2

3 2

[libvirt] [PATCH 0/5] Resolve REVERSE_INULL errors found by Coverity
by John Ferlan 17 Jan '13

17 Jan '13

This patch will resolve a set of "REVERSE_INULL (CWE-476):" errors found by Coverity. John Ferlan (5): network: Resolve some issues around vlan copying xen: Remove extraneous checks in xenStoreGetDomainInfo() interface: Need to check ifacedef->mac not just ifacedef after strdup() xen: Need to check validity of domain->conn before deref privateData openvz: Need to check 'vm' first before dereferencing 'def' src/interface/interface_backend_udev.c | 2 +- src/network/bridge_driver.c | 16 +++++++++++++--- src/openvz/openvz_driver.c | 2 +- src/xen/xen_hypervisor.c | 3 ++- src/xen/xs_internal.c | 2 +- 5 files changed, 18 insertions(+), 7 deletions(-) -- 1.7.11.7

5 15

[libvirt] [PATCH] network: use bandwidth from portgroup when appropriate
by Laine Stump 17 Jan '13

17 Jan '13

The bandwidth plug and unplug functions were assuming that an interface's bandwidth setting was always specified directly in the domain's <interface> definition, but that's not necessarily true - it could have been obtained from a <portgroup> definition in the network definition. This patch fixes those functions to use virDomainNetGetActualBandwidth(), which gets the bandwidth pointer from iface->data.network.actual if it exists, otherwise returns iface->bandwidth. --- src/network/bridge_driver.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c3cb63d..6f3c839 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4518,7 +4518,7 @@ networkCheckBandwidth(virNetworkObjPtr net, { int ret = -1; virNetDevBandwidthPtr netBand = net->def->bandwidth; - virNetDevBandwidthPtr ifaceBand = iface->bandwidth; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); unsigned long long tmp_floor_sum = net->floor_sum; unsigned long long tmp_new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; @@ -4597,6 +4597,7 @@ networkPlugBandwidth(virNetworkObjPtr net, unsigned long long new_rate = 0; ssize_t class_id = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); if ((plug_ret = networkCheckBandwidth(net, iface, &new_rate)) < 0) { /* helper reported error */ @@ -4625,11 +4626,8 @@ networkPlugBandwidth(virNetworkObjPtr net, goto cleanup; } - plug_ret = virNetDevBandwidthPlug(net->def->bridge, - net->def->bandwidth, - &iface->mac, - iface->bandwidth, - class_id); + plug_ret = virNetDevBandwidthPlug(net->def->bridge, net->def->bandwidth, + &iface->mac, ifaceBand, class_id); if (plug_ret < 0) { ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id)); goto cleanup; @@ -4638,11 +4636,11 @@ networkPlugBandwidth(virNetworkObjPtr net, /* QoS was set, generate new class ID */ iface->data.network.actual->class_id = class_id; /* update sum of 'floor'-s of attached NICs */ - net->floor_sum += iface->bandwidth->in->floor; + net->floor_sum += ifaceBand->in->floor; /* update status file */ if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) { ignore_value(virBitmapClearBit(net->class_id, class_id)); - net->floor_sum -= iface->bandwidth->in->floor; + net->floor_sum -= ifaceBand->in->floor; iface->data.network.actual->class_id = 0; ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id)); goto cleanup; @@ -4666,6 +4664,7 @@ networkUnplugBandwidth(virNetworkObjPtr net, { int ret = 0; unsigned long long new_rate; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); if (iface->data.network.actual && iface->data.network.actual->class_id) { @@ -4680,13 +4679,13 @@ networkUnplugBandwidth(virNetworkObjPtr net, if (ret < 0) goto cleanup; /* update sum of 'floor'-s of attached NICs */ - net->floor_sum -= iface->bandwidth->in->floor; + net->floor_sum -= ifaceBand->in->floor; /* return class ID */ ignore_value(virBitmapClearBit(net->class_id, iface->data.network.actual->class_id)); /* update status file */ if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) { - net->floor_sum += iface->bandwidth->in->floor; + net->floor_sum += ifaceBand->in->floor; ignore_value(virBitmapSetBit(net->class_id, iface->data.network.actual->class_id)); goto cleanup; -- 1.7.11.7

2 2

[libvirt] [PATCH] conf: don't fail to parse <boot> element when parsing a single device
by Laine Stump 17 Jan '13

17 Jan '13

This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=895294 The symptom was that attempts to modify a network device using virDomainUpdateDeviceFlags() would fail if the original device had a <boot> element (e.g. "<boot order='1'/>"), even if the updated device had the same <boot> element. Instead, the following error would be logged: cannot modify network device boot index setting It's true that it's not possible to change boot order (internally known as bootIndex) of a live device; qemuDomainChangeNet checks for that, but the problem was that the information it was checking was incorrect. Explanation: When a complete domain is parsed, a global (to the domain) "bootMap" is passed down to the parse for each device; the bootMap is used to make sure that devices don't have conflicting settings for their boot orders. When a single device is parsed by itself (as in the case of virDomainUpdateDeviceFlags), there is no global bootMap that would be appropriate to send, so NULL is sent instead. However, although the lowest level function that parses just the boot order *does* simply skip the sanity check in that case, the next higher level "virDomainDeviceInfoParseXML" function refuses to call down to the lower "virDomainDeviceBootParseXML" if bootMap is NULL. So, the boot order is never set in the "new" device object, and when it is compared to the original (which does have a boot order), they don't match. The fix is to patch virDomainDeviceInfoParseXML to not care about bootMap, and just always call virDomainDeviceInfoBootParseXML whenever there is a <boot> element. When we are only parsing a single device, we don't care whether or not any specified boot order is consistent with the rest of the domain; we will always do this check later (in the current case, we do it by verifying that the net bootIndex exactly matches the old bootIndex). --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 06b9c25..e4babaf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1,7 +1,7 @@ /* * domain_conf.c: domain XML processing * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -2624,7 +2624,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, } else if (master == NULL && xmlStrEqual(cur->name, BAD_CAST "master")) { master = cur; - } else if (boot == NULL && bootMap && + } else if (boot == NULL && (flags & VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) && xmlStrEqual(cur->name, BAD_CAST "boot")) { boot = cur; -- 1.7.11.7

2 2

[libvirt] [PATCH] lock_daemon: Simplify error handling
by Peter Krempa 17 Jan '13

17 Jan '13

No need to check the run_dir variable twice for NULL. --- src/locking/lock_daemon.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index f0eba48..97e5d74 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1311,19 +1311,16 @@ int main(int argc, char **argv) { /* Ensure the rundir exists (on tmpfs on some systems) */ if (privileged) { - run_dir = strdup(LOCALSTATEDIR "/run/libvirt"); + if (!(run_dir = strdup(LOCALSTATEDIR "/run/libvirt"))) { + virReportOOMError(); + goto cleanup; + } } else { - run_dir = virGetUserRuntimeDirectory(); - - if (!run_dir) { + if (!(run_dir = virGetUserRuntimeDirectory())) { VIR_ERROR(_("Can't determine user directory")); goto cleanup; } } - if (!run_dir) { - virReportOOMError(); - goto cleanup; - } if (privileged) old_umask = umask(022); -- 1.8.1.1

2 2

[libvirt] [PATCH 0/2 v2] Colorize HTML documentation
by Claudio Bley 17 Jan '13

17 Jan '13

The only difference to v1 is that I fixed the CSS pseudo elements adding some vital whitespaces. (I tried to be extra smart and removed some whitespace without further testing, just to discover that it did not work anymore *after* sending the patcheset). Claudio Bley (2): docs: Assign classes to documentation elements docs: Add some style and color to the HTML documentation docs/generic.css | 4 ++ docs/libvirt.css | 56 +++++++++++++++- docs/newapi.xsl | 187 +++++++++++++++++++++++++++++++----------------------- 3 files changed, 166 insertions(+), 81 deletions(-) -- 1.7.9.5

2 6

[libvirt] [PATCH 00/12] virsh: Partially convert to c99 struct initialization
by Michal Privoznik 17 Jan '13

17 Jan '13

For now, we have to suffice with vshCmdOptDef. Others coming soon. Changes are pretty mechanical, so if you find a mistake there, blame my vim macro :) Michal Privoznik (12): virsh.c: Switch to C99 initialization of vshCmdOptDef virsh-domain.c: Switch to C99 initialization of vshCmdOptDef virsh-domain-monitor.c: Switch to C99 initialization of vshCmdOptDef virsh-host.c: Switch to C99 initialization of vshCmdOptDef virsh-interface.c: Switch to C99 initialization of vshCmdOptDef virsh-network.c: Switch to C99 initialization of vshCmdOptDef virsh-nodedev.c: Switch to C99 initialization of vshCmdOptDef virsh-nwfilter.c: Switch to C99 initialization of vshCmdOptDef virsh-pool.c: Switch to C99 initialization of vshCmdOptDef virsh-secret.c: Switch to C99 initialization of vshCmdOptDef virsh-snapshot.c: Switch to C99 initialization of vshCmdOptDef virsh-volume.c: Switch to C99 initialization of vshCmdOptDef tools/virsh-domain-monitor.c | 279 +++-- tools/virsh-domain.c | 2299 +++++++++++++++++++++++++++++++++--------- tools/virsh-host.c | 102 +- tools/virsh-interface.c | 138 ++- tools/virsh-network.c | 193 +++- tools/virsh-nodedev.c | 64 +- tools/virsh-nwfilter.c | 34 +- tools/virsh-pool.c | 286 ++++-- tools/virsh-secret.c | 72 +- tools/virsh-snapshot.c | 452 +++++++-- tools/virsh-volume.c | 368 +++++-- tools/virsh.c | 62 +- 12 files changed, 3406 insertions(+), 943 deletions(-) -- 1.8.0.2

3 28