- Devel - libvirt List Archives

[libvirt] [PATCH v3 RESEND 00/12] Rework storage migration
by Michal Privoznik 20 Feb '13

20 Feb '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 target='vda' size='15032385536'/> <disk target='vdb' size='11534336'/> <disk target='vdc' size='13631488'/> </nbd> The source is telling the destination it supports the NBD feature. Moreover, which disks are to be transferred and what are their sizes. That's because disks needs to be fully allocated (even qcow) for successful transfer. 2) dst -> src: (QEMU_MIGRATION_PHASE_PREPARE -> QEMU_MIGRATION_PHASE_PERFORM3) <nbd port='5901'/> The destination is confirming it does support NBD as well. All disks were pre-created and NBD server is listening on given port (5901 in this case). If either src or dst doesn't support NBD, it is not used and whole process falls back 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 v2.1: -nbd cookie reworked -don't rely on disk ordering in the cookie, but use disk target for that -adapt to virPortAllocator -unlink pre-created storage on migration fail -other of Jirka's suggestions worked in "diff" to v3: -just rebase & adapt to new qemu code after dropping QDL (Qemu Driver Lock) Michal Privoznik (12): 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: Introduce qemuMigrationDriveMirror qemu_domain: Introduce qemuDomainGetDiskBlockInfo qemu_migration: Check size prerequisites qemu_migration: Stop NBD server at Finish phase qemu_migration: Cancel running jobs on failed migration qemu_migration: Unlink pre-created storage on error src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 157 ++++++++- src/qemu/qemu_domain.h | 7 + src/qemu/qemu_driver.c | 124 +------ src/qemu/qemu_migration.c | 767 ++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_monitor.c | 63 ++++ src/qemu/qemu_monitor.h | 7 + src/qemu/qemu_monitor_json.c | 102 ++++++ src/qemu/qemu_monitor_json.h | 7 + src/qemu/qemu_process.c | 13 + 11 files changed, 1116 insertions(+), 136 deletions(-) -- 1.8.0.2

2 25

[libvirt] [PATCHv2] qemu: enable direct migration over IPv6
by Ján Tomko 20 Feb '13

20 Feb '13

Use virURIParse in qemuMigrationPrepareDirect to allow parsing IPv6 addresses, which would cause an 'incorrect :port' error message before. To be able to migrate over IPv6, QEMU needs to listen on [::] instead of 0.0.0.0. This patch adds a call to getaddrinfo and sets the listen address based on the result. This will break migration if a hostname that can only be resolved on the source machine is passed in the migration URI, or if it does not resolve to the same address family on both sides. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013 --- Diff to V1: * initialize uri_str * reuse STRSKIP("tcp:") result instead of doing strlen on it * print a warning instead of failing when the hostname can't be resolved src/qemu/qemu_migration.c | 64 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 36e55d2..62e9260 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -22,7 +22,10 @@ #include <config.h> +#include <netdb.h> +#include <sys/socket.h> #include <sys/time.h> +#include <sys/types.h> #ifdef WITH_GNUTLS # include <gnutls/gnutls.h> # include <gnutls/x509.h> @@ -1836,7 +1839,11 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, char *hostname = NULL; char migrateFrom [64]; const char *p; + char *uri_str = NULL; int ret = -1; + bool ipv6 = false; + struct addrinfo *info; + virURIPtr uri; VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, " @@ -1885,16 +1892,39 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, * URI when passing it to the qemu monitor, so bad * characters in hostname part don't matter. */ - if (!STRPREFIX(uri_in, "tcp:")) { + if (!(p = STRSKIP(uri_in, "tcp:"))) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("only tcp URIs are supported for KVM/QEMU" " migrations")); goto cleanup; } - /* Get the port number. */ - p = strrchr(uri_in, ':'); - if (p == strchr(uri_in, ':')) { + /* Convert uri_in to well-formed URI with // after tcp: */ + if (!(STRPREFIX(uri_in, "tcp://"))) { + if (virAsprintf(&uri_str, "tcp://%s", p) < 0) { + virReportOOMError(); + goto cleanup; + } + } + + uri = virURIParse(uri_str ? uri_str : uri_in); + VIR_FREE(uri_str); + + if (uri == NULL) { + virReportError(VIR_ERR_INVALID_ARG, _("unable to parse URI: %s"), + uri_in); + goto cleanup; + } + + if (uri->server == NULL) { + virReportError(VIR_ERR_INVALID_ARG, _("missing host in migration" + " URI: %s"), uri_in); + goto cleanup; + } else { + hostname = uri->server; + } + + if (uri->port == 0) { /* Generate a port */ this_port = QEMUD_MIGRATION_FIRST_PORT + port++; if (port == QEMUD_MIGRATION_NUM_PORTS) @@ -1907,21 +1937,29 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, } } else { - p++; /* definitely has a ':' in it, see above */ - this_port = virParseNumber(&p); - if (this_port == -1 || p-uri_in != strlen(uri_in)) { - virReportError(VIR_ERR_INVALID_ARG, - "%s", _("URI ended with incorrect ':port'")); - goto cleanup; - } + this_port = uri->port; } } + if (getaddrinfo(hostname, NULL, NULL, &info)) { + VIR_WARN("unable to get address info for %s, defaulting to IPv4", + hostname); + } else { + ipv6 = info->ai_family == AF_INET6; + } + if (*uri_out) VIR_DEBUG("Generated uri_out=%s", *uri_out); - /* QEMU will be started with -incoming tcp:0.0.0.0:port */ - snprintf(migrateFrom, sizeof(migrateFrom), "tcp:0.0.0.0:%d", this_port); + /* QEMU will be started with -incoming tcp:0.0.0.0:port + * or -incoming tcp:[::]:port for IPv6 */ + if (ipv6) { + snprintf(migrateFrom, sizeof(migrateFrom), + "tcp:[::]:%d", this_port); + } else { + snprintf(migrateFrom, sizeof(migrateFrom), + "tcp:0.0.0.0:%d", this_port); + } ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, dname, dom_xml, -- 1.7.12.4

2 3

[libvirt] [PATCH v2 RESEND 0/2] cgroup refactor
by Hu Tao 20 Feb '13

20 Feb '13

This series is for early review. This series refactors cgroup code to: - provide lazy creation of cgroup directories, despite of what level they are. - remove cgroup directories if no one is using the corresponding virCgroup. changes from v2: - rebase to latest tree. - squash patches 2-5 in v2 into one. Hu Tao (2): refactor virCgroupDetectMounts and virCgroupDetectPlacement cgroup: refactor virCgroup src/conf/domain_conf.h | 5 + src/libvirt_private.syms | 7 +- src/lxc/lxc_cgroup.c | 40 +- src/lxc/lxc_cgroup.h | 2 +- src/lxc/lxc_controller.c | 31 +- src/lxc/lxc_driver.c | 177 +++---- src/lxc/lxc_process.c | 19 +- src/qemu/qemu_cgroup.c | 162 +++--- src/qemu/qemu_cgroup.h | 3 +- src/qemu/qemu_driver.c | 350 +++++-------- src/qemu/qemu_hotplug.c | 21 +- src/qemu/qemu_migration.c | 20 +- src/qemu/qemu_process.c | 7 +- src/util/vircgroup.c | 1263 +++++++++++++++++++++------------------------ src/util/vircgroup.h | 19 +- 15 files changed, 920 insertions(+), 1206 deletions(-) -- 1.8.0.1.240.ge8a1f5a

1 3

[libvirt] [PATCH] virsh: distinguish errors between missing argument and wrong option
by Guannan Ren 20 Feb '13

20 Feb '13

Specifying ':' to suppress the error messages printed by getopt(). Then, distinguish the two types of errors. Before: virsh: option requires an argument -- 'c' error: unsupported option '-?'. See --help. After: error: option '-c' requires an argument error: unsupported option '-x'. See --help. --- tools/virsh.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index f5a01b3..c0e62eb 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2919,7 +2919,7 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) /* Standard (non-command) options. The leading + ensures that no * argument reordering takes place, so that command options are * not confused with top-level virsh options. */ - while ((arg = getopt_long(argc, argv, "+d:hqtc:vVrl:e:", opt, NULL)) != -1) { + while ((arg = getopt_long(argc, argv, "+:d:hqtc:vVrl:e:", opt, NULL)) != -1) { switch (arg) { case 'd': if (virStrToLong_i(optarg, NULL, 10, &debug) < 0) { @@ -2973,8 +2973,14 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) exit(EXIT_FAILURE); } break; + case ':': + vshError(ctl, _("option '-%c' requires an argument"), optopt); + exit(EXIT_FAILURE); + case '?': + vshError(ctl, _("unsupported option '-%c'. See --help."), optopt); + exit(EXIT_FAILURE); default: - vshError(ctl, _("unsupported option '-%c'. See --help."), arg); + vshError(ctl, _("unknown option")); exit(EXIT_FAILURE); } } -- 1.7.11.2

2 3

[libvirt] [PATCH 0/2] re-sort libvirt_private.syms
by Eric Blake 20 Feb '13

20 Feb '13

After Dan's recent round of file renames, our symbols file was not up-to-date. This fixes it. It looks big, but the second patch is mechanical (took me about 2 minutes); most of my time was spent on the first patch getting ready for the sort. Eric Blake (2): maint: fix header file owners of private symbols maint: sort private syms to reflect recent header renames src/libvirt_private.syms | 2046 +++++++++++++++++++++++----------------------- 1 file changed, 1025 insertions(+), 1021 deletions(-) -- 1.8.1.2

2 4

[libvirt] [PATCH v5 0/3] net: support set source address(es) and ports for NAT
by Natanael Copa 19 Feb '13

19 Feb '13

Fixes the commented nitpicks[1]. I also provide an optional 3rd patch, that uses structs for address and port ranges. There was a struct for DHCP address range that could be used after a rename. I put the structs in virsocketaddr.h so they are accessible from util/viriptables. I don't think it is worth its own headerfile and sockaddr was what I thought was most relevant. Skip the third patch if unsure. Please CC me as i don't subscribe to the list. Changes v5: - Remove unused "longdef" leftover from rebase - UseCamelCaseVariableNames - Use virAsprintf instead of snprintf and avoid strings on stack - Add an optional 3rd patch that uses structs for addr and port ranges. Natanael Copa (3): net: support set public ip range for forward mode nat net: add support for specifying port range for forward mode nat net: use structs for address and port ranges docs/formatnetwork.html.in | 33 +++++++ src/conf/network_conf.c | 206 +++++++++++++++++++++++++++++++++++++++++--- src/conf/network_conf.h | 13 ++- src/network/bridge_driver.c | 16 ++++ src/util/viriptables.c | 83 ++++++++++++++++-- src/util/viriptables.h | 4 + src/util/virsocketaddr.h | 14 +++ 7 files changed, 341 insertions(+), 28 deletions(-) [1] http://www.redhat.com/archives/libvir-list/2013-February/msg00817.html -- 1.8.1.3

2 6

[libvirt] Online resize of virtio-blk device does not emit udev event
by Milos Vyletel 19 Feb '13

19 Feb '13

Hi, I hope this is the right list to ask. If not please tell me what will be more appropriate place to ask this question. I'm doing some work with online resizing of guest's block device on LVM. What I do is call lvresize to expand logical volume itself and virsh blockresize to notify guest about this change. Besides the fact that I needed to patch kernel to get this working on mounted partitions(BZ 906050) everything works just fine. Now that I've had this part successfully tested I wanted to make a script that will be triggered by udev whenever particular device is changed. I assumed that after capacity change virtio_blk driver would emit udev event but that's not the case. I'm able to see one on host but not on guest. [root@host ~]# lvresize -f -L +100M /dev/vgguests/evd2 && \ > virsh blockresize resize vdb 1 Rounding size to boundary between physical extents: 128.00 MiB Extending logical volume evd2 to 15.00 GiB Logical volume evd2 successfully resized Block device 'vdb' is resized [root@host ~]# udevadm monitor ... KERNEL[1361292279.619233] change /devices/virtual/block/dm-9 (block) UDEV [1361292279.736012] change /devices/virtual/block/dm-9 (block) ... I was looking at the virtblk_config_changed_work function in RHEL6.3 kernel's drivers/block/virtio_blk.c which I believe is the function handling blockresize and it does not look like it tries to emit any kobject uevent. Before I jump into patching kernel my question is whether it makes sense to have such uevent? I surely can use a way how to detect capacity change from userspace. Thanks, Milos

2 2

[libvirt] [PATCH] qemu: Use atomic ops for driver->nactive
by Jiri Denemark 19 Feb '13

19 Feb '13

--- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_process.c | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index b5a3281..a6bd527 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -168,7 +168,7 @@ struct _virQEMUDriver { virCgroupPtr cgroup; /* Atomic inc/dec only */ - size_t nactive; + unsigned int nactive; /* Immutable pointers. Caller must provide locking */ virStateInhibitCallback inhibitCallback; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 74406a6..b612d72 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -69,6 +69,7 @@ #include "virtime.h" #include "virnetdevtap.h" #include "virbitmap.h" +#include "viratomic.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -3300,9 +3301,8 @@ qemuProcessReconnect(void *opaque) goto error; } - if (!driver->nactive && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); - driver->nactive++; endjob: if (!qemuDomainObjEndJob(driver, obj)) @@ -3589,9 +3589,8 @@ int qemuProcessStart(virConnectPtr conn, qemuDomainSetFakeReboot(driver, vm, false); virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN); - if (!driver->nactive && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); - driver->nactive++; /* Run an early hook to set-up missing devices */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { @@ -4188,8 +4187,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, */ vm->def->id = -1; - driver->nactive--; - if (!driver->nactive && driver->inhibitCallback) + if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) { @@ -4443,9 +4441,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, vm->def->id = qemuDriverAllocateID(driver); - if (!driver->nactive && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); - driver->nactive++; if (virFileMakePath(cfg->logDir) < 0) { virReportSystemError(errno, -- 1.8.1.2

3 4

[libvirt] [PATCH] rpc: Avoid deadlock when closing client connection
by Jiri Denemark 19 Feb '13

19 Feb '13

We need to drop the server lock before calling virObjectUnlock(client) since in case we had the last reference to the client, its dispose callback would be called and that could possibly try to lock the server and cause a deadlock. This is exactly what happens when there is only one QEMU domain running and it is marked to be autodestroyed when the connection dies. This results in qemuProcessAutoDestroy -> qemuProcessStop -> virNetServerRemoveShutdownInhibition call sequence, where the last function locks the server. --- src/rpc/virnetserver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 95333d0..e536cc3 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1120,7 +1120,7 @@ void virNetServerRun(virNetServerPtr srv) if (virNetServerClientWantClose(srv->clients[i])) virNetServerClientClose(srv->clients[i]); if (virNetServerClientIsClosed(srv->clients[i])) { - virObjectUnref(srv->clients[i]); + virNetServerClientPtr client = srv->clients[i]; if (srv->nclients > 1) { memmove(srv->clients + i, srv->clients + i + 1, @@ -1131,6 +1131,10 @@ void virNetServerRun(virNetServerPtr srv) srv->nclients = 0; } + virObjectUnlock(srv); + virObjectUnref(client); + virObjectLock(srv); + goto reprocess; } } -- 1.8.1.2

2 2

[libvirt] [PATCH] Avoid resetting errors in virTypedParamsFree
by Jiri Denemark 19 Feb '13

19 Feb '13

The function does not report any errors so there should be no need too reset an existing error first. Moreover, virTypedParamsFree is mostly called in cleanup phase where it has the potential to reset any useful reported earlier. --- src/util/virtypedparam.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index ae2855a..c196321 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1026,7 +1026,6 @@ void virTypedParamsFree(virTypedParameterPtr params, int nparams) { - virResetLastError(); virTypedParamsClear(params, nparams); VIR_FREE(params); } -- 1.8.1.2

2 2