- Devel - libvirt List Archives

[libvirt] [PATCH 00/11] Post-Copy Live Migration Support
by Cristian Klein 04 Dec '14

04 Dec '14

Qemu currently implements pre-copy live migration. VM memory pages are first copied from the source hypervisor to the destination, potentially multiple times as pages get dirtied during transfer, then VCPU state is migrated. Unfortunately, if the VM dirties memory faster than the network bandwidth, then pre-copy cannot finish. `virsh` currently includes an option to suspend a VM after a timeout, so that migration may finish, but at the expense of downtime. A future version of qemu will implement post-copy live migration. The VCPU state is first migrated to the destination hypervisor, then memory pages are pulled from the source hypervisor. Post-copy has the potential to do migration with zero-downtime, despite the VM dirtying pages fast, with minimum performance impact. On the other hand, while post-copy is in progress, any network failure would render the VM unusable, as its memory is partitioned between the source and destination hypervisor. Therefore, post-copy should only be used when necessary. Post-copy migration in qemu will work as follows: (1) The `x-postcopy-ram` migration capability needs to be set. (2) Migration is started. (3) When the user decides so, post-copy migration is activated by sending the `migrate-start-postcopy` command. (4) Qemu acknowledges by setting migration status to `postcopy-active`. This patch series implements two ways to access post-copy functionality: low-level and high-level. The low-level API implements a mechanism that basically requires the libvirt user to manually go through the above steps, by calling migration with `VIR_MIGRATE_ENABLE_POSTCOPY`, then during migration from a separate thread, call `virDomainMigrateStartPostCopy`. The choice of when migration should switch from pre-copy to post-copy is left entirely to the user. The high-level API implements a policy that automatically triggers post-copy after one pass of pre-copy, which experiments have shown to minimize downtime. Using it is also simpler: the user only has to call migration with `VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY`. TODO: - Wait for qemu API to become stable, i.e., drop `x-` - Wait for qemu to offer notification for migration state change v4: - Rename low-level API flag to `VIR_MIGRATE_ENABLE_POSTCOPY` - Added high-level API flag `VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY` - Do not introduce a new job type and use migration status instead - Added both low- and high-level interface to virsh - Tested with OpenStack Icehouse Cristian Klein (11): Added public API for post-copy migration qemu: added low-level post-copy migration functions qemu: implemented VIR_MIGRATE_ENABLE_POSTCOPY qemu: implemented post-copy migration logic qemu: implement virDomainMigrateStartPostCopy virsh: added --enable-postcopy and migrate-start-postcopy virsh: added --postcopy-after to migrate command qemu: retrieve dirty sync count qemu: implemented VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY virsh: added --postcopy-after-precopy to migrate Revert "Do not allow changing the UUID of a nwfilter" include/libvirt/libvirt-domain.h | 5 ++ src/conf/nwfilter_conf.c | 11 --- src/driver-hypervisor.h | 5 ++ src/libvirt-domain.c | 92 +++++++++++++++++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_driver.c | 60 ++++++++++++++ src/qemu/qemu_migration.c | 169 +++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_migration.h | 4 +- src/qemu/qemu_monitor.c | 24 +++++- src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 27 ++++++- src/qemu/qemu_monitor_json.h | 1 + src/qemu/qemu_monitor_text.c | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 ++- src/remote_protocol-structs | 5 ++ tests/qemumonitorjsontest.c | 1 + tools/virsh-domain.c | 116 ++++++++++++++++++++++++++- tools/virsh.pod | 21 +++++ 19 files changed, 536 insertions(+), 25 deletions(-) -- 1.9.1

5 17

[libvirt] [PATCH] Report original error when QMP probing fails with new QEMU
by Daniel P. Berrange 04 Dec '14

04 Dec '14

If probing capabilities via QMP fails, we now have a check that prevents us falling back to -help parsing. Unfortunately the error message "Failed to probe capabilities for /usr/bin/qemu-kvm: unsupported configuration: QEMU 2.1.2 is too new for help parsing" is proving rather unhelpful to the user. We need to be telling them why QMP failed (the root cause), rather than they can't use -help (the side effect). To do this we should capture stderr during QMP probing, and if -help parsing then sees a new QEMU version, we know that QMP should have worked, and so we can show the messages from stderr. The message thus becomes "Failed to probe capabilities for /usr/bin/qemu-kvm: internal error: QEMU / QMP failed: Could not access KVM kernel module: No such file or directory failed to initialize KVM: No such file or directory" --- src/qemu/qemu_capabilities.c | 37 +++++++++++++++++++++++++++---------- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_process.c | 4 ++-- tests/qemuhelptest.c | 2 +- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d2c046d..f777484 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1331,7 +1331,8 @@ int virQEMUCapsParseHelpStr(const char *qemu, unsigned int *version, bool *is_kvm, unsigned int *kvm_version, - bool check_yajl) + bool check_yajl, + const char *qemuerr) { unsigned major, minor, micro; const char *p = help; @@ -1392,9 +1393,15 @@ int virQEMUCapsParseHelpStr(const char *qemu, * using QMP probing. */ if (*version >= 1002000) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("QEMU %u.%u.%u is too new for help parsing"), - major, minor, micro); + if (qemuerr && *qemuerr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QEMU / QMP failed: %s"), + qemuerr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("QEMU %u.%u.%u is too new for help parsing"), + major, minor, micro); + } goto cleanup; } @@ -2967,7 +2974,7 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) #define QEMU_SYSTEM_PREFIX "qemu-system-" static int -virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid) +virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid, const char *qemuerr) { virCommandPtr cmd = NULL; bool is_kvm; @@ -2998,7 +3005,8 @@ virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid) &qemuCaps->version, &is_kvm, &qemuCaps->kvmVersion, - false) < 0) + false, + qemuerr) < 0) goto cleanup; /* x86_64 and i686 support PCI-multibus on all machine types @@ -3249,7 +3257,8 @@ static int virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, const char *libDir, uid_t runUid, - gid_t runGid) + gid_t runGid, + char **qemuerr) { int ret = -1; virCommandPtr cmd = NULL; @@ -3311,13 +3320,16 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, virCommandSetGID(cmd, runGid); virCommandSetUID(cmd, runUid); + virCommandSetErrorBuffer(cmd, qemuerr); + /* Log, but otherwise ignore, non-zero status. */ if (virCommandRun(cmd, &status) < 0) goto cleanup; if (status != 0) { ret = 0; - VIR_DEBUG("QEMU %s exited with status %d", qemuCaps->binary, status); + VIR_DEBUG("QEMU %s exited with status %d: %s", + qemuCaps->binary, status, *qemuerr); goto cleanup; } @@ -3366,6 +3378,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, VIR_ERROR(_("Failed to kill process %lld: %s"), (long long) pid, virStrerror(errno, ebuf, sizeof(ebuf))); + + VIR_FREE(*qemuerr); } if (pidfile) { unlink(pidfile); @@ -3406,6 +3420,7 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, virQEMUCapsPtr qemuCaps; struct stat sb; int rv; + char *qemuerr = NULL; if (!(qemuCaps = virQEMUCapsNew())) goto error; @@ -3436,13 +3451,13 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, goto error; if (rv == 0) { - if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0) { + if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qemuerr) < 0) { virQEMUCapsLogProbeFailure(binary); goto error; } if (!qemuCaps->usedQMP && - virQEMUCapsInitHelp(qemuCaps, runUid, runGid) < 0) { + virQEMUCapsInitHelp(qemuCaps, runUid, runGid, qemuerr) < 0) { virQEMUCapsLogProbeFailure(binary); goto error; } @@ -3451,9 +3466,11 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, goto error; } + VIR_FREE(qemuerr); return qemuCaps; error: + VIR_FREE(qemuerr); virObjectUnref(qemuCaps); qemuCaps = NULL; return NULL; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c5542d1..f53b8f4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -308,7 +308,8 @@ int virQEMUCapsParseHelpStr(const char *qemu, unsigned int *version, bool *is_kvm, unsigned int *kvm_version, - bool check_yajl); + bool check_yajl, + const char *qemuerr); /* Only for use by test suite */ int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, const char *str); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a14b6f7..68777e8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3554,12 +3554,12 @@ qemuProcessReconnect(void *opaque) struct qemuProcessReconnectData *data = opaque; virQEMUDriverPtr driver = data->driver; virDomainObjPtr obj = data->obj; - qemuDomainObjPrivatePtr priv; + qemuDomainObjPrivatePtr priv = NULL; virConnectPtr conn = data->conn; struct qemuDomainJobObj oldjob; int state; int reason; - virQEMUDriverConfigPtr cfg; + virQEMUDriverConfigPtr cfg = NULL; size_t i; int ret; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 0c68367..6d9525c 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -59,7 +59,7 @@ static int testHelpStrParsing(const void *data) goto cleanup; if (virQEMUCapsParseHelpStr("QEMU", help, flags, - &version, &is_kvm, &kvm_version, false) == -1) { + &version, &is_kvm, &kvm_version, false, NULL) == -1) { if (info->error && virGetLastError()->code == info->error) ret = 0; goto cleanup; -- 2.1.0

2 1

[libvirt] [PATCH] docs: storage: fix rbd pool indentation
by Cole Robinson 04 Dec '14

04 Dec '14

--- Pushed as trivial docs/storage.html.in | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/storage.html.in b/docs/storage.html.in index 9933548..f920dae 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -551,12 +551,12 @@ <name>myrbdpool</name> <source> <name>rbdpool</name> - <host name='1.2.3.4' port='6789'/> - <host name='my.ceph.monitor' port='6789'/> - <host name='third.ceph.monitor' port='6789'/> - <auth username='admin' type='ceph'> - <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> - </auth> + <host name='1.2.3.4' port='6789'/> + <host name='my.ceph.monitor' port='6789'/> + <host name='third.ceph.monitor' port='6789'/> + <auth username='admin' type='ceph'> + <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> + </auth> </source> </pool></pre> -- 2.1.0

1 0

[libvirt] [PATCH] qemu: process: Avoid uninitialized use two vars when reconnecting to vm
by Peter Krempa 04 Dec '14

04 Dec '14

3ecebf07110ca8d3413072557f29137943e848e3 breaks the build as it adds a way to jump to cleanup before the 'cfg' object is retrieved and 'priv' is initialized. --- Notes: Pushed under the build-breaker rule. src/qemu/qemu_process.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 310a08d..a14b6f7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3571,16 +3571,16 @@ qemuProcessReconnect(void *opaque) * deleted if qemuConnectMonitor() failed */ virObjectRef(obj); + cfg = virQEMUDriverGetConfig(driver); + priv = obj->privateData; + if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) < 0) goto killvm; virNWFilterReadLockFilterUpdates(); - cfg = virQEMUDriverGetConfig(driver); VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name); - priv = obj->privateData; - /* XXX check PID liveliness & EXE path */ if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, -1) < 0) goto error; -- 2.1.0

1 0

[libvirt] [PATCHv2] qemu: process: Refactor reconnecting to qemu processes
by Peter Krempa 04 Dec '14

04 Dec '14

Move entering the job into the thread to simplify the program flow. Also as the code holds a separate reference to the domain object some conditions can be simplified. --- Notes: Version 2: - fix leak of 'data' when not reconnecting src/qemu/qemu_process.c | 161 ++++++++++++++++++++++-------------------------- 1 file changed, 72 insertions(+), 89 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 08d6b7c..310a08d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3524,8 +3524,7 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver, struct qemuProcessReconnectData { virConnectPtr conn; virQEMUDriverPtr driver; - void *payload; - struct qemuDomainJobObj oldjob; + virDomainObjPtr obj; }; /* * Open an existing VM's monitor, re-detect VCPU threads @@ -3534,13 +3533,27 @@ struct qemuProcessReconnectData { * We own the virConnectPtr we are passed here - whoever started * this thread function has increased the reference counter to it * so that we now have to close it. + * + * This function also inherits a locked domain object. + * + * This function needs to: + * 1. Enter job + * 1. just before monitor reconnect do lightweight MonitorEnter + * (increase VM refcount and unlock VM) + * 2. reconnect to monitor + * 3. do lightweight MonitorExit (lock VM) + * 4. continue reconnect process + * 5. EndJob + * + * We can't do normal MonitorEnter & MonitorExit because these two lock the + * monitor lock, which does not exists in this early phase. */ static void qemuProcessReconnect(void *opaque) { struct qemuProcessReconnectData *data = opaque; virQEMUDriverPtr driver = data->driver; - virDomainObjPtr obj = data->payload; + virDomainObjPtr obj = data->obj; qemuDomainObjPrivatePtr priv; virConnectPtr conn = data->conn; struct qemuDomainJobObj oldjob; @@ -3550,26 +3563,24 @@ qemuProcessReconnect(void *opaque) size_t i; int ret; - memcpy(&oldjob, &data->oldjob, sizeof(oldjob)); - VIR_FREE(data); - virNWFilterReadLockFilterUpdates(); + qemuDomainObjRestoreJob(obj, &oldjob); - virObjectLock(obj); + /* Hold an extra reference because we can't allow 'vm' to be + * deleted if qemuConnectMonitor() failed */ + virObjectRef(obj); + + if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) < 0) + goto killvm; + + virNWFilterReadLockFilterUpdates(); cfg = virQEMUDriverGetConfig(driver); VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name); priv = obj->privateData; - /* Job was started by the caller for us */ - qemuDomainObjTransferJob(obj); - - /* Hold an extra reference because we can't allow 'vm' to be - * deleted if qemuConnectMonitor() failed */ - virObjectRef(obj); - /* XXX check PID liveliness & EXE path */ if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, -1) < 0) goto error; @@ -3701,10 +3712,10 @@ qemuProcessReconnect(void *opaque) driver->inhibitCallback(true, driver->inhibitOpaque); endjob: - if (!qemuDomainObjEndJob(driver, obj)) - obj = NULL; + /* we hold an extra reference, so this will never fail */ + ignore_value(qemuDomainObjEndJob(driver, obj)); - if (obj && virObjectUnref(obj)) + if (virObjectUnref(obj)) virObjectUnlock(obj); virObjectUnref(conn); @@ -3714,35 +3725,35 @@ qemuProcessReconnect(void *opaque) return; error: - if (!qemuDomainObjEndJob(driver, obj)) - obj = NULL; - - if (obj) { - if (!virDomainObjIsActive(obj)) { - if (virObjectUnref(obj)) - virObjectUnlock(obj); - } else if (virObjectUnref(obj)) { - /* We can't get the monitor back, so must kill the VM - * to remove danger of it ending up running twice if - * user tries to start it again later - */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { - /* If we couldn't get the monitor and qemu supports - * no-shutdown, we can safely say that the domain - * crashed ... */ - state = VIR_DOMAIN_SHUTOFF_CRASHED; - } else { - /* ... but if it doesn't we can't say what the state - * really is and FAILED means "failed to start" */ - state = VIR_DOMAIN_SHUTOFF_UNKNOWN; - } - qemuProcessStop(driver, obj, state, 0); - if (!obj->persistent) - qemuDomainRemoveInactive(driver, obj); - else - virObjectUnlock(obj); + /* we hold an extra reference, so this will never fail */ + ignore_value(qemuDomainObjEndJob(driver, obj)); + + killvm: + if (virDomainObjIsActive(obj)) { + /* We can't get the monitor back, so must kill the VM + * to remove danger of it ending up running twice if + * user tries to start it again later + */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { + /* If we couldn't get the monitor and qemu supports + * no-shutdown, we can safely say that the domain + * crashed ... */ + state = VIR_DOMAIN_SHUTOFF_CRASHED; + } else { + /* ... but if it doesn't we can't say what the state + * really is and FAILED means "failed to start" */ + state = VIR_DOMAIN_SHUTOFF_UNKNOWN; } + qemuProcessStop(driver, obj, state, 0); } + + if (virObjectUnref(obj)) { + if (!obj->persistent) + qemuDomainRemoveInactive(driver, obj); + else + virObjectUnlock(obj); + } + virObjectUnref(conn); virObjectUnref(cfg); virNWFilterUnlockFilterUpdates(); @@ -3756,6 +3767,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj, struct qemuProcessReconnectData *src = opaque; struct qemuProcessReconnectData *data; + /* If the VM was inactive, we don't need to reconnect */ if (!obj->pid) return 0; @@ -3763,64 +3775,35 @@ qemuProcessReconnectHelper(virDomainObjPtr obj, return -1; memcpy(data, src, sizeof(*data)); - data->payload = obj; - - /* - * We create a separate thread to run qemuProcessReconnect in it. - * However, qemuProcessReconnect needs to: - * 1. just before monitor reconnect do lightweight MonitorEnter - * (increase VM refcount, unlock VM & driver) - * 2. reconnect to monitor - * 3. do lightweight MonitorExit (lock VM) - * 4. continue reconnect process - * 5. EndJob - * - * NB, we can't do normal MonitorEnter & MonitorExit because - * these two lock the monitor lock, which does not exists in - * this early phase. - */ + data->obj = obj; + /* this lock will be eventually transferred to the thread that handles the + * reconnect */ virObjectLock(obj); - qemuDomainObjRestoreJob(obj, &data->oldjob); - - if (qemuDomainObjBeginJob(src->driver, obj, QEMU_JOB_MODIFY) < 0) - goto error; - - /* Since we close the connection later on, we have to make sure - * that the threads we start see a valid connection throughout their - * lifetime. We simply increase the reference counter here. + /* Since we close the connection later on, we have to make sure that the + * threads we start see a valid connection throughout their lifetime. We + * simply increase the reference counter here. */ virObjectRef(data->conn); if (virThreadCreate(&thread, false, qemuProcessReconnect, data) < 0) { - - virObjectUnref(data->conn); - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create thread. QEMU initialization " "might be incomplete")); - if (!qemuDomainObjEndJob(src->driver, obj)) { - obj = NULL; - } else if (virObjectUnref(obj)) { - /* We can't spawn a thread and thus connect to monitor. - * Kill qemu */ - qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, 0); - if (!obj->persistent) - qemuDomainRemoveInactive(src->driver, obj); - else - virObjectUnlock(obj); - } - goto error; - } + /* We can't spawn a thread and thus connect to monitor. Kill qemu. */ + qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, 0); + if (!obj->persistent) + qemuDomainRemoveInactive(src->driver, obj); + else + virObjectUnlock(obj); - virObjectUnlock(obj); + virObjectUnref(data->conn); + VIR_FREE(data); + return -1; + } return 0; - - error: - VIR_FREE(data); - return -1; } /** -- 2.1.0

2 2

[libvirt] [PATCH] spec: Remove qemu-img dependency from storage driver
by Jiri Denemark 04 Dec '14

04 Dec '14

Both qemu-img and its stripped down version qcow-create are searched for dynamically with virFindFileInPath, functions using them report errors when the required binary is not present, and we don't compile in a fixed path detected by configure anymore. So I don't see a reason for keeping the dependencies for our storage driver. Moreover, it's weired when the dependencies (of the storage driver) change depending on wheter a particular hypervisor driver (xen/qemu) is enabled or not. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- libvirt.spec.in | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index bda28e7..dbeb62a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -530,15 +530,6 @@ BuildRequires: PolicyKit-devel >= 0.6 # For mount/umount in FS driver BuildRequires: util-linux %endif -%if %{with_qemu} -# From QEMU RPMs -BuildRequires: /usr/bin/qemu-img -%else - %if %{with_xen} -# From Xen RPMs -BuildRequires: /usr/sbin/qcow-create - %endif -%endif %if %{with_storage_lvm} # For LVM drivers BuildRequires: lvm2 @@ -871,15 +862,6 @@ Requires: device-mapper # For Sheepdog support Requires: sheepdog %endif - %if %{with_qemu} -# From QEMU RPMs -Requires: /usr/bin/qemu-img - %else - %if %{with_xen} -# From Xen RPMs -Requires: /usr/sbin/qcow-create - %endif - %endif %description daemon-driver-storage The storage driver plugin for the libvirtd daemon, providing -- 2.2.0

3 4

[libvirt] [PATCH] drvbhyve: Automatically tear down guest domains on shutdown
by Conrad Meyer 04 Dec '14

04 Dec '14

Reboot requires more sophistication and is left as a future work item -- but at least part of the plumbing is in place. --- Looking for feedback. I'm pretty unfamiliar with libvirt; maybe this isn't quite the right way to do this. Sorry. If you want me to rework it in some way, just let me know. I tried to model this off of DrvQEMU, which I knew to support this behavior. Reboot should probably be processed on a threadpool thread outside of the event loop, so I omitted it for now. P.S., the 'read-non-seekable' check test seems to just hang forever on FreeBSD. I haven't dug into it, but POSIX FIFO behavior is pretty weird. --- src/Makefile.am | 2 + src/bhyve/bhyve_monitor.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_monitor.h | 36 +++++++++ src/bhyve/bhyve_process.c | 14 +++- 4 files changed, 233 insertions(+), 3 deletions(-) create mode 100644 src/bhyve/bhyve_monitor.c create mode 100644 src/bhyve/bhyve_monitor.h diff --git a/src/Makefile.am b/src/Makefile.am index d8fe624..b6c1701 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -833,6 +833,8 @@ BHYVE_DRIVER_SOURCES = \ bhyve/bhyve_domain.h \ bhyve/bhyve_driver.h \ bhyve/bhyve_driver.c \ + bhyve/bhyve_monitor.c \ + bhyve/bhyve_monitor.h \ bhyve/bhyve_process.c \ bhyve/bhyve_process.h \ bhyve/bhyve_utils.h \ diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c new file mode 100644 index 0000000..cd3cf6e --- /dev/null +++ b/src/bhyve/bhyve_monitor.c @@ -0,0 +1,184 @@ +/* + * bhyve_monitor.c: Tear-down or reboot bhyve domains on guest shutdown + * + * Copyright (C) 2014 Conrad Meyer + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Conrad Meyer <cse.cem(a)gmail.com> + */ + +#include <config.h> + +#include <sys/types.h> +#include <sys/event.h> +#include <sys/time.h> +#include <sys/wait.h> + +#include "bhyve_monitor.h" +#include "bhyve_process.h" +#include "viralloc.h" +#include "virerror.h" +#include "virfile.h" +#include "virlog.h" + +#define VIR_FROM_THIS VIR_FROM_BHYVE + +VIR_LOG_INIT("bhyve.bhyve_monitor"); + +struct _bhyveMonitor { + int kq; + int watch; + virDomainObjPtr vm; + bhyveConnPtr driver; +}; + +static void +bhyveMonitorIO(int watch, int kq, int events ATTRIBUTE_UNUSED, void *opaque) +{ + const struct timespec zerowait = {}; + bhyveMonitorPtr mon = opaque; + struct kevent kev; + int rc, status; + + if (watch != mon->watch || kq != mon->kq) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("event from unexpected fd %d!=%d / watch %d!=%d"), + mon->kq, kq, mon->watch, watch); + return; + } + + rc = kevent(kq, NULL, 0, &kev, 1, &zerowait); + if (rc < 0) { + virReportSystemError(errno, "%s", _("Unable to query kqueue")); + return; + } + + if (rc == 0) + return; + + if ((kev.flags & EV_ERROR) != 0) { + virReportSystemError(kev.data, "%s", _("Unable to query kqueue")); + return; + } + + if (kev.filter == EVFILT_PROC && (kev.fflags & NOTE_EXIT) != 0) { + if ((pid_t)kev.ident != mon->vm->pid) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("event from unexpected proc %ju!=%ju"), + (uintmax_t)mon->vm->pid, (uintmax_t)kev.ident); + return; + } + + status = kev.data; + if (WIFSIGNALED(status) && WCOREDUMP(status)) { + VIR_ERROR("Guest %s got signal %d and crashed", mon->vm->def->name, + WTERMSIG(status)); + virBhyveProcessStop(mon->driver, mon->vm, + VIR_DOMAIN_SHUTOFF_CRASHED); + } else if (WIFEXITED(status)) { + if (WEXITSTATUS(status) == 0) { + /* 0 - reboot */ + /* TODO: Implementing reboot is a little more complicated. */ + VIR_INFO("Guest %s rebooted; destroying domain.", + mon->vm->def->name); + virBhyveProcessStop(mon->driver, mon->vm, + VIR_DOMAIN_SHUTOFF_SHUTDOWN); + } else if (WEXITSTATUS(status) < 3) { + /* 1 - shutdown, 2 - halt, 3 - triple fault. others - error */ + VIR_INFO("Guest %s shut itself down; destroying domain.", + mon->vm->def->name); + virBhyveProcessStop(mon->driver, mon->vm, + VIR_DOMAIN_SHUTOFF_SHUTDOWN); + } else { + VIR_INFO("Guest %s had an error and exited with status %d; destroying domain.", + mon->vm->def->name, WEXITSTATUS(status)); + virBhyveProcessStop(mon->driver, mon->vm, + VIR_DOMAIN_SHUTOFF_UNKNOWN); + } + } + } +} + +static void +bhyveMonitorRelease(void *opaque) +{ + bhyveMonitorPtr mon = opaque; + + VIR_FORCE_CLOSE(mon->kq); + virObjectUnref(mon->vm); + VIR_FREE(mon); +} + +bhyveMonitorPtr +bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver) +{ + bhyveMonitorPtr mon; + struct kevent kev; + int rc; + + if (VIR_ALLOC(mon) < 0) + return NULL; + + mon->vm = virObjectRef(vm); + mon->driver = driver; + + mon->kq = kqueue(); + if (mon->kq < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, "%s", + _("Unable to create kqueue")); + goto cleanup; + } + + EV_SET(&kev, vm->pid, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, mon); + rc = kevent(mon->kq, &kev, 1, NULL, 0, NULL); + if (rc < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, "%s", + _("Unable to register process kevent")); + goto cleanup; + } + + mon->watch = virEventAddHandle(mon->kq, + VIR_EVENT_HANDLE_READABLE | + VIR_EVENT_HANDLE_ERROR | + VIR_EVENT_HANDLE_HANGUP, + bhyveMonitorIO, + mon, + bhyveMonitorRelease); + if (mon->watch < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to register monitor events")); + goto cleanup; + } + + return mon; + + cleanup: + bhyveMonitorRelease(mon); + return NULL; +} + +void +bhyveMonitorClose(bhyveMonitorPtr mon) +{ + + if (mon == NULL) + return; + + if (mon->watch > 0) + virEventRemoveHandle(mon->watch); + else + bhyveMonitorRelease(mon); +} diff --git a/src/bhyve/bhyve_monitor.h b/src/bhyve/bhyve_monitor.h new file mode 100644 index 0000000..226d878 --- /dev/null +++ b/src/bhyve/bhyve_monitor.h @@ -0,0 +1,36 @@ +/* + * bhyve_monitor.h: Tear-down or reboot bhyve domains on guest shutdown + * + * Copyright (C) 2014 Conrad Meyer + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Conrad Meyer <cse.cem(a)gmail.com> + */ + +#ifndef BHYVE_MONITOR_H +# define BHYVE_MONITOR_H + +# include "internal.h" +# include "domain_conf.h" +# include "bhyve_utils.h" + +typedef struct _bhyveMonitor bhyveMonitor; +typedef bhyveMonitor *bhyveMonitorPtr; + +bhyveMonitorPtr bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver); +void bhyveMonitorClose(bhyveMonitorPtr mon); + +#endif /* BHYVE_MONITOR_H */ diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index a30e36a..284641a 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -32,8 +32,9 @@ #include <net/if_tap.h> #include "bhyve_device.h" -#include "bhyve_process.h" #include "bhyve_command.h" +#include "bhyve_monitor.h" +#include "bhyve_process.h" #include "datatypes.h" #include "virerror.h" #include "virlog.h" @@ -209,6 +210,7 @@ virBhyveProcessStart(virConnectPtr conn, vm->def->id = vm->pid; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); + vm->privateData = bhyveMonitorOpen(vm, driver); if (virDomainSaveStatus(driver->xmlopt, BHYVE_STATE_DIR, @@ -268,6 +270,9 @@ virBhyveProcessStop(bhyveConnPtr driver, return -1; } + if (vm->privateData != NULL) + bhyveMonitorClose((bhyveMonitorPtr)vm->privateData); + /* First, try to kill 'bhyve' process */ if (virProcessKillPainfully(vm->pid, true) != 0) VIR_WARN("Failed to gracefully stop bhyve VM '%s' (pid: %d)", @@ -371,9 +376,12 @@ virBhyveProcessReconnect(virDomainObjPtr vm, goto cleanup; proc_argv = kvm_getargv(data->kd, kp, 0); - if (proc_argv && proc_argv[0]) - if (STREQ(expected_proctitle, proc_argv[0])) + if (proc_argv && proc_argv[0]) { + if (STREQ(expected_proctitle, proc_argv[0])) { ret = 0; + vm->privateData = bhyveMonitorOpen(vm, data->driver); + } + } cleanup: if (ret < 0) { -- 1.9.3

2 2

[libvirt] [PATCH 0/3] qemu: support update graphic device persistently
by Wang Rui 04 Dec '14

04 Dec '14

We can change vnc password by using virDomainUpdateDeviceFlags API with live flag. But it can't be changed with config flag. 1/3: improve the error number when update graphics failed 2/3: refactor the function qemuDomainFindGraphics for the future patch 3/3: support updating vnc/spice auth arguments persistently Wang Rui (3): qemu: report properer error number when change graphics failed qemu: revise qemuDomainFindGraphics to be reused in the future patch qemu: make persistent update of graphics device supported src/conf/domain_conf.c | 3 ++- src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.c | 23 +++++++++++------------ src/qemu/qemu_hotplug.h | 3 +++ 4 files changed, 53 insertions(+), 14 deletions(-) -- 1.7.12.4

2 5

[libvirt] [PATCHv2 0/9] Let libvirt manage a bridge's FDB
by Laine Stump 04 Dec '14

04 Dec '14

(Everyone - see the request for opinions/ideas towards the bottom) The idea behind these patches is the following: 1) most virtual machines only have a single MAC address behind each interface, and that MAC address is known by libvirt. 2) If we (i.e. libvirt) manually add an entry to the bridge's forwarding database (fdb) for the MAC address associated with a port on the bridge, we can turn off learning and unicast_flooding for that port. 3) kernels starting with 3.15 (and actually working correctly starting in kernel 3.17) will notice that all of a bridge's ports have flood and learning turned off, and in that case will turn off promiscuous mode on all ports. If all but one of the ports have flood/learning turned off, then promiscuous will be turned off on that port (and left on for all the other ports) 4) When (4) can be done, there is a measurable performance advantage. It can also *kind of* help security, as it will prevent a guest from doing anything useful if it changes its MAC address (but won't prevent the guest from *sending* packets with a spoofed MAC address). NB: These only work with a fixed MAC address, and no vlan tags set in the guest. Support for both of those will be coming. HERE IS THE REQUEST FOR OPINIONS/IDEAS: This V2 of the patchset addresses several issues brought up by jferlan on the original series, and changes the name of the attribute from: promiscLinks='yes|no' to fdb='learningWithFlood|managed' I'm somewhat more happy with this new naming than the previous but still looking for better ideas. It is closer to describing what the new code really does, but "learningWithFlood" seems a bit long and awkward, while I have been told that "fdb" is too short and unrecognizeable (I will point out that 1) "fdb" is the same name used by iproute2's "bridge" command, and 2) another bridge option, "stp" is also a three letter acronym that will only be recognized by those familiar with configuring an L2 bridge device or watching NASCAR on Saturday afternoons (or whenever it's on - not a fan myself :-)) Here is a full list of every idea that either I or someone else has come up with since I started thinking about this: promiscLinks='yes|no' After initially going with this for the v1 of the patchset, I later decided against it, because it doesn't describe what libvirt is doing, but only a *possible* side effect on *some* of the ports connected to the bridge (in practice, it only happens to the physdev port). fdb='auto|managed' I like "fdb" as the name of the attribute, because I think it really gets at what libvirt is doing - it is taking over management of the bridge's fdb (Forwarding Database), which ends up providing better performance in several ways. The problem with this proposal is that the two values are kind of ambiguous - it's not clear which one is using the bridge module's built-in management (I had figured this would be "auto"), and which is telling libvirt to manage it ("managed"). (on the other hand, the first option is "ignore the issue, let the underlying system handle it", vs. "libvirt should manage it", so maybe it *is* a reasonable choice). Also, see the comment above about the perceived terseness and obscurity of "fdb". fdb='learningWithFlood|managed' This alternate name was suggested by Michael Tsirkin as a way to unambiguously indicate what was being done in the mode where libvirt isn't involved in the fdb management. There was some criticism on IRC that the name is *too* verbose, especially when contrasted with "fdb". fdb='learning|managed' A suggested shortening of "learningWithFlood". forwardingDatabase='blah' A way to get around criticism of "fdb". I think this is too verbose, but maybe I'm biased :-) [specify each minor item that separately] In order to manage the fdb by itself, libvirt disables "learning" and "unicast_flood" for each tap device, enables "vlan_filtering" for the bridge itself, then adds an fdb entry for each tap to the bridge. There was one suggestion that, rather than trying to come up with a single option that says "do all of these things", we should instead make each of them separately configured. The problem with this is that it makes it too easy to screw up the configuration such that it causes sub-par performance, or simply doesn't work at all. Part of libvirt's job is making it difficult to screw up (or at least easier to succeed); for example, libvirt's virtual networks do a lot of things automatically - create a bridge, add iptables rules for filtering and NAT, run an instance of dnsmasq - over time we've offered the option of tweaking more and more of the details of this setup, but the initial aim was to provide something that worked with as few required (or even permitted) tweaks as possible. I guess what I'm getting at is that I think it would be a mistake to require turning on several different knobs (which individually make little/no sense) in order to get the bridge into this higher performing mode. So - does anyone have an opinion of any of the options offered above, or any ideas for alternates? In the meantime, note that while the default is currently "learningWithFlood" (meaning that that name is never actually directly used/required anywhere, but is just in the RNG and the enum definition), the intent of the people who developed this functionality in the kernel is that eventually it will work so well that libvirt management of the fdb can silently become the default with no visible change in behavior. NOTE: If you want to actually try out these patches, you'll need to apply the following patch which I haven't yet pushed: https://www.redhat.com/archives/libvir-list/2014-November/msg00948.html Also, while the description of V1 stated that patches 08 and 09 were not intended to be pushed yet, due to a problem they caused when restarting libvirtd after an update, that problem has been solved, so I now intend to push patches 08 and 09 along with the rest. Laine Stump (9): util: new functions for setting bridge and bridge port attributes util: functions to manage bridge fdb (forwarding database) conf: new network bridge device attribute fdb network: save bridge name in ActualNetDef when actualType==network too network: store network fdb setting in NetDef actual object network: setup bridge devices for fdb='managed' qemu: setup tap devices for fdb='managed' qemu: always use virDomainNetGetActualBridgeName to get interface's bridge lxc: always use virDomainNetGetActualBridgeName to get interface's bridge docs/formatnetwork.html.in | 42 ++- docs/schemas/network.rng | 9 + src/conf/domain_conf.c | 129 ++++--- src/conf/domain_conf.h | 2 + src/conf/network_conf.c | 50 ++- src/conf/network_conf.h | 11 + src/libvirt_private.syms | 11 + src/lxc/lxc_driver.c | 32 +- src/lxc/lxc_process.c | 32 +- src/network/bridge_driver.c | 74 ++++ src/qemu/qemu_command.c | 53 ++- src/qemu/qemu_hotplug.c | 60 +--- src/util/virnetdevbridge.c | 382 ++++++++++++++++++++- src/util/virnetdevbridge.h | 44 ++- tests/networkxml2xmlin/host-bridge-no-flood.xml | 6 + .../nat-network-explicit-flood.xml | 21 ++ tests/networkxml2xmlout/host-bridge-no-flood.xml | 6 + .../nat-network-explicit-flood.xml | 23 ++ tests/networkxml2xmltest.c | 2 + 19 files changed, 778 insertions(+), 211 deletions(-) create mode 100644 tests/networkxml2xmlin/host-bridge-no-flood.xml create mode 100644 tests/networkxml2xmlin/nat-network-explicit-flood.xml create mode 100644 tests/networkxml2xmlout/host-bridge-no-flood.xml create mode 100644 tests/networkxml2xmlout/nat-network-explicit-flood.xml -- 1.9.3

3 24

[libvirt] [PATCH] Fix handling of whitespae in preprocessor macros for API generator
by Daniel P. Berrange 04 Dec '14

04 Dec '14

The apibuild.py script did not handle whitespace in preprocessor macros, so it failed to detect constants declared with '# define' instead of '#define'. Since we now correctly indent our public header files, we have silently lost all constants from libvirt-api.xml. This also caused us to not detect formatting errors in constant docs --- docs/apibuild.py | 8 ++++++++ include/libvirt/libvirt-host.h | 12 ++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 7549a63..9fa9361 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -459,6 +459,14 @@ class CLexer: if line[0] == '#': self.tokens = map((lambda x: ('preproc', x)), string.split(line)) + + # We might have whitespace between the '#' and preproc + # macro name, so instead of having a single token element + # of '#define' we might end up with '#' and 'define'. This + # merges them back together + if self.tokens[0][1] == "#": + self.tokens[0] = ('preproc', self.tokens[0][1] + self.tokens[1][1]) + self.tokens = self.tokens[:1] + self.tokens[2:] break l = len(line) if line[0] == '"' or line[0] == "'": diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 5bd9563..53b529f 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -536,28 +536,32 @@ struct _virNodeMemoryStats { */ # define VIR_NODE_MEMORY_SHARED_PAGES_SHARING "shm_pages_sharing" -/* VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED: +/* + * VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED: * * Macro for typed parameter that represents how many pages unique * but repeatedly checked for merging. */ # define VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED "shm_pages_unshared" -/* VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE: +/* + * VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE: * * Macro for typed parameter that represents how many pages changing * too fast to be placed in a tree. */ # define VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE "shm_pages_volatile" -/* VIR_NODE_MEMORY_SHARED_FULL_SCAN: +/* + * VIR_NODE_MEMORY_SHARED_FULL_SCANS: * * Macro for typed parameter that represents how many times all * mergeable areas have been scanned. */ # define VIR_NODE_MEMORY_SHARED_FULL_SCANS "shm_full_scans" -/* VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES: +/* + * VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES: * * Macro for typed parameter that represents whether pages from * different NUMA nodes can be merged. The parameter has type int, -- 2.1.0

2 1