[libvirt] [PATCH] qemu: Allow graceful domain destroy
This patch introduces support for domain destroying via 'quit' monitor command which gives qemu time to flush caches and therefore prevent disks corruption. However, qemu can be unresponsive and to prevent waiting indefinitely, execute command in a separate thread and wait reasonable time (QEMU_QUIT_WAIT_SECONDS) on a condition. If we hit timeout, qemu is qemuProcessKill'ed which causes monitor close and therefore also thread being terminable. The synchronization between qemu driver and monitor-quit thread is done through mutex and condition. However, this alone is not enough. If a condition is signalized but without anybody listening signal is lost. To prevent this a boolean variable is used that is set iff nobody is listening but condition would be signalized, or iff driver is waiting on given condition. --- include/libvirt/libvirt.h.in | 11 ++- src/qemu/qemu_driver.c | 132 +++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 13 ++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 22 +++++++ src/qemu/qemu_monitor_json.h | 1 + src/qemu/qemu_monitor_text.c | 20 ++++++ src/qemu/qemu_monitor_text.h | 1 + tools/virsh.c | 8 ++- 9 files changed, 198 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa29fb6..fa98b8d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -919,10 +919,13 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ -/* - * typedef enum { - * } virDomainDestroyFlagsValues; - */ + +typedef enum { + VIR_DOMAIN_DESTROY_MONITOR = 1 << 0, /* In hypervisors supporting this, + try to send 'quit' command prior + to killing hypervisor */ +} virDomainDestroyFlagsValues; + virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 421a98e..6585cb4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1564,6 +1564,42 @@ cleanup: return ret; } +struct qemuDomainDestroyHelperData { + struct qemud_driver *driver; + virDomainObjPtr vm; + qemuDomainObjPrivatePtr priv; + virMutex lock; + virCond cond; + bool guard; +}; + +static void +qemuDomainDestroyHelper(void *opaque) +{ + struct qemuDomainDestroyHelperData *data = opaque; + struct qemud_driver *driver = data->driver; + virDomainObjPtr vm = data->vm; + qemuDomainObjPrivatePtr priv = data->priv; + + /* Job was already obtained by caller */ + qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuMonitorQuit(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + + /* To avoid loosing signal, we need to remember + * we tried to send one, but nobody was waiting + * for it. + */ + virMutexLock(&data->lock); + if (data->guard) { + virCondSignal(&data->cond); + } else { + data->guard = true; + } + virMutexUnlock(&data->lock); +} + +#define QEMU_QUIT_WAIT_SECONDS 5 static int qemuDomainDestroyFlags(virDomainPtr dom, @@ -1574,8 +1610,13 @@ qemuDomainDestroyFlags(virDomainPtr dom, int ret = -1; virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; + bool use_thread = false; + bool use_kill = false; + virThread destroy_thread; + struct qemuDomainDestroyHelperData data; + unsigned long long then; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_DESTROY_MONITOR, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1590,12 +1631,14 @@ qemuDomainDestroyFlags(virDomainPtr dom, priv = vm->privateData; priv->fakeReboot = false; - /* Although qemuProcessStop does this already, there may - * be an outstanding job active. We want to make sure we - * can kill the process even if a job is active. Killing - * it now means the job will be released - */ - qemuProcessKill(vm); + if (!flags) { + /* Although qemuProcessStop does this already, there may + * be an outstanding job active. We want to make sure we + * can kill the process even if a job is active. Killing + * it now means the job will be released + */ + qemuProcessKill(vm); + } if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0) goto cleanup; @@ -1606,6 +1649,70 @@ qemuDomainDestroyFlags(virDomainPtr dom, goto endjob; } + if (flags & VIR_DOMAIN_DESTROY_MONITOR) { + /* Try to shutdown domain gracefully. + * Send 'quit' to qemu. However, qemu can be unresponsive. + * Therefore create a separate thread in which we execute + * that monitor comand. Wait max QEMU_QUIT_WAIT_SECONDS. + */ + if (virMutexInit(&data.lock) < 0) { + virReportOOMError(); + goto endjob; + } + + if (virCondInit(&data.cond) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize thread condition")); + virMutexDestroy(&data.lock); + goto endjob; + } + + data.driver = driver; + data.vm = vm; + data.priv = priv; + data.guard = false; + + if (virThreadCreate(&destroy_thread, true, + qemuDomainDestroyHelper, &data) < 0) { + virReportSystemError(errno, "%s", + _("unable to create destroy thread")); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + goto endjob; + } + use_thread = true; + + if (virTimeMs(&then) < 0) + goto endjob; + + then += QEMU_QUIT_WAIT_SECONDS * 1000; + + /* How synchronization with destroy thread works: + * Basically wait on data.cond. But because conditions + * does not 'remember' that they have been signalized + * data.guard is used. Practically, data.guard says + * to destroy thread if we are waiting on condition + * and to us whether we should even try. + */ + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + if (virCondWaitUntil(&data.cond, &data.lock, then) < 0) { + if (errno == ETIMEDOUT) { + use_kill = true; + data.guard = false; + } else { + virMutexUnlock(&data.lock); + goto endjob; + } + } + } + virMutexUnlock(&data.lock); + + if (use_kill) + qemuProcessKill(vm); + } + qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_DESTROYED); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -1626,6 +1733,17 @@ endjob: vm = NULL; cleanup: + if (use_thread) { + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + ignore_value(virCondWait(&data.cond, &data.lock)); + } + virMutexUnlock(&data.lock); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + virThreadJoin(&destroy_thread); + } if (vm) virDomainObjUnlock(vm); if (event) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..41b9c5c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2475,3 +2475,16 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, ret = qemuMonitorTextBlockJob(mon, device, bandwidth, info, mode); return ret; } + +int qemuMonitorQuit(qemuMonitorPtr mon) +{ + int ret; + + VIR_DEBUG("mon=%p", mon); + + if (mon->json) + ret = qemuMonitorJSONQuit(mon); + else + ret = qemuMonitorTextQuit(mon); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..3fe6bb9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -475,6 +475,8 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorQuit(qemuMonitorPtr mon); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..1f078cd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2956,3 +2956,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONQuit(qemuMonitorPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("quit", NULL); + + if (!cmd) + return -1; + + ret =qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..2a7df76 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -231,4 +231,5 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorJSONQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 335e39e..19c2690 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3046,3 +3046,23 @@ cleanup: VIR_FREE(reply); return ret; } + + +int qemuMonitorTextQuit(qemuMonitorPtr mon) +{ + const char *cmd = "quit"; + char *reply = NULL; + int ret = -1; + + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot run monitor command")); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..9ade938 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -224,4 +224,5 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorTextQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_TEXT_H */ diff --git a/tools/virsh.c b/tools/virsh.c index f1eb4ca..0c18d8b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2565,6 +2565,7 @@ static const vshCmdInfo info_destroy[] = { static const vshCmdOptDef opts_destroy[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"monitor", VSH_OT_BOOL, 0, N_("try graceful destroy via monitor first")}, {NULL, 0, 0, NULL} }; @@ -2574,6 +2575,7 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2581,7 +2583,11 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainDestroy(dom) == 0) { + if (vshCommandOptBool(cmd, "monitor")) + flags |= VIR_DOMAIN_DESTROY_MONITOR; + + if ((!flags && virDomainDestroy(dom) == 0) || + virDomainDestroyFlags(dom, flags) == 0) { vshPrint(ctl, _("Domain %s destroyed\n"), name); } else { vshError(ctl, _("Failed to destroy domain %s"), name); -- 1.7.3.4
On Sat, Aug 20, 2011 at 01:06:10PM +0200, Michal Privoznik wrote:
This patch introduces support for domain destroying via 'quit' monitor command which gives qemu time to flush caches and therefore prevent disks corruption. However, qemu can be unresponsive and to prevent waiting indefinitely, execute command in a separate thread and wait reasonable time (QEMU_QUIT_WAIT_SECONDS) on a condition. If we hit timeout, qemu is qemuProcessKill'ed which causes monitor close and therefore also thread being terminable.
The synchronization between qemu driver and monitor-quit thread is done through mutex and condition. However, this alone is not enough. If a condition is signalized but without anybody listening signal is lost. To prevent this a boolean variable is used that is set iff nobody is listening but condition would be signalized, or iff driver is waiting on given condition. --- include/libvirt/libvirt.h.in | 11 ++- src/qemu/qemu_driver.c | 132 +++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 13 ++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 22 +++++++ src/qemu/qemu_monitor_json.h | 1 + src/qemu/qemu_monitor_text.c | 20 ++++++ src/qemu/qemu_monitor_text.h | 1 + tools/virsh.c | 8 ++- 9 files changed, 198 insertions(+), 12 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa29fb6..fa98b8d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -919,10 +919,13 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */
-/* - * typedef enum { - * } virDomainDestroyFlagsValues; - */ + +typedef enum { + VIR_DOMAIN_DESTROY_MONITOR = 1 << 0, /* In hypervisors supporting this, + try to send 'quit' command prior + to killing hypervisor */ +} virDomainDestroyFlagsValues;
Rather than describing the flag based on the QEmu command I would rather describe it's intended effects, i.e. VIR_DOMAIN_DESTROY_FLUSH_CACHE = 1 << 0, /* In hypervisors supporting this, try to get cache flushed prior to destroying the domain */
virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 421a98e..6585cb4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1564,6 +1564,42 @@ cleanup: return ret; }
+struct qemuDomainDestroyHelperData { + struct qemud_driver *driver; + virDomainObjPtr vm; + qemuDomainObjPrivatePtr priv; + virMutex lock; + virCond cond; + bool guard; +}; + +static void +qemuDomainDestroyHelper(void *opaque) +{ + struct qemuDomainDestroyHelperData *data = opaque; + struct qemud_driver *driver = data->driver; + virDomainObjPtr vm = data->vm; + qemuDomainObjPrivatePtr priv = data->priv; + + /* Job was already obtained by caller */ + qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuMonitorQuit(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + + /* To avoid loosing signal, we need to remember + * we tried to send one, but nobody was waiting + * for it. + */ + virMutexLock(&data->lock); + if (data->guard) { + virCondSignal(&data->cond); + } else { + data->guard = true; + } + virMutexUnlock(&data->lock); +} + +#define QEMU_QUIT_WAIT_SECONDS 5
static int qemuDomainDestroyFlags(virDomainPtr dom, @@ -1574,8 +1610,13 @@ qemuDomainDestroyFlags(virDomainPtr dom, int ret = -1; virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; + bool use_thread = false; + bool use_kill = false; + virThread destroy_thread; + struct qemuDomainDestroyHelperData data; + unsigned long long then;
- virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_DESTROY_MONITOR, -1);
qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1590,12 +1631,14 @@ qemuDomainDestroyFlags(virDomainPtr dom, priv = vm->privateData; priv->fakeReboot = false;
- /* Although qemuProcessStop does this already, there may - * be an outstanding job active. We want to make sure we - * can kill the process even if a job is active. Killing - * it now means the job will be released - */ - qemuProcessKill(vm); + if (!flags) {
please test the given flag here instead of assuming the bit we know about now.
+ /* Although qemuProcessStop does this already, there may + * be an outstanding job active. We want to make sure we + * can kill the process even if a job is active. Killing + * it now means the job will be released + */ + qemuProcessKill(vm); + }
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0) goto cleanup; @@ -1606,6 +1649,70 @@ qemuDomainDestroyFlags(virDomainPtr dom, goto endjob; }
+ if (flags & VIR_DOMAIN_DESTROY_MONITOR) { + /* Try to shutdown domain gracefully. + * Send 'quit' to qemu. However, qemu can be unresponsive. + * Therefore create a separate thread in which we execute + * that monitor comand. Wait max QEMU_QUIT_WAIT_SECONDS. + */ + if (virMutexInit(&data.lock) < 0) { + virReportOOMError(); + goto endjob; + } + + if (virCondInit(&data.cond) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize thread condition")); + virMutexDestroy(&data.lock); + goto endjob; + } + + data.driver = driver; + data.vm = vm; + data.priv = priv; + data.guard = false; + + if (virThreadCreate(&destroy_thread, true, + qemuDomainDestroyHelper, &data) < 0) { + virReportSystemError(errno, "%s", + _("unable to create destroy thread")); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + goto endjob; + } + use_thread = true; + + if (virTimeMs(&then) < 0) + goto endjob; + + then += QEMU_QUIT_WAIT_SECONDS * 1000; + + /* How synchronization with destroy thread works: + * Basically wait on data.cond. But because conditions + * does not 'remember' that they have been signalized + * data.guard is used. Practically, data.guard says + * to destroy thread if we are waiting on condition + * and to us whether we should even try. + */ + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + if (virCondWaitUntil(&data.cond, &data.lock, then) < 0) { + if (errno == ETIMEDOUT) { + use_kill = true; + data.guard = false; + } else { + virMutexUnlock(&data.lock); + goto endjob; + } + } + } + virMutexUnlock(&data.lock); + + if (use_kill) + qemuProcessKill(vm); + } + qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_DESTROYED); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -1626,6 +1733,17 @@ endjob: vm = NULL;
cleanup: + if (use_thread) { + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + ignore_value(virCondWait(&data.cond, &data.lock)); + } + virMutexUnlock(&data.lock); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + virThreadJoin(&destroy_thread); + } if (vm) virDomainObjUnlock(vm); if (event) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..41b9c5c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2475,3 +2475,16 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, ret = qemuMonitorTextBlockJob(mon, device, bandwidth, info, mode); return ret; } + +int qemuMonitorQuit(qemuMonitorPtr mon) +{ + int ret; + + VIR_DEBUG("mon=%p", mon); + + if (mon->json) + ret = qemuMonitorJSONQuit(mon); + else + ret = qemuMonitorTextQuit(mon); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..3fe6bb9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -475,6 +475,8 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode);
+int qemuMonitorQuit(qemuMonitorPtr mon); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..1f078cd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2956,3 +2956,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONQuit(qemuMonitorPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("quit", NULL); + + if (!cmd) + return -1; + + ret =qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..2a7df76 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -231,4 +231,5 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode);
+int qemuMonitorJSONQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 335e39e..19c2690 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3046,3 +3046,23 @@ cleanup: VIR_FREE(reply); return ret; } + + +int qemuMonitorTextQuit(qemuMonitorPtr mon) +{ + const char *cmd = "quit"; + char *reply = NULL; + int ret = -1; + + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot run monitor command")); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..9ade938 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -224,4 +224,5 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode);
+int qemuMonitorTextQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_TEXT_H */ diff --git a/tools/virsh.c b/tools/virsh.c index f1eb4ca..0c18d8b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2565,6 +2565,7 @@ static const vshCmdInfo info_destroy[] = {
static const vshCmdOptDef opts_destroy[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"monitor", VSH_OT_BOOL, 0, N_("try graceful destroy via monitor first")}, {NULL, 0, 0, NULL} };
@@ -2574,6 +2575,7 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + int flags = 0;
if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2581,7 +2583,11 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false;
- if (virDomainDestroy(dom) == 0) { + if (vshCommandOptBool(cmd, "monitor")) + flags |= VIR_DOMAIN_DESTROY_MONITOR; + + if ((!flags && virDomainDestroy(dom) == 0) || + virDomainDestroyFlags(dom, flags) == 0) { vshPrint(ctl, _("Domain %s destroyed\n"), name); } else { vshError(ctl, _("Failed to destroy domain %s"), name); -- 1.7.3.4
This sounds right, but the devil lies in the detail, I would add far more debug in the thread and around synchronization as this is informations we would need if something goes really wrong. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Sat, Aug 20, 2011 at 01:06:10PM +0200, Michal Privoznik wrote:
This patch introduces support for domain destroying via 'quit' monitor command which gives qemu time to flush caches and therefore prevent disks corruption. However, qemu can be unresponsive and to prevent waiting indefinitely, execute command in a separate thread and wait reasonable time (QEMU_QUIT_WAIT_SECONDS) on a condition. If we hit timeout, qemu is qemuProcessKill'ed which causes monitor close and therefore also thread being terminable.
The synchronization between qemu driver and monitor-quit thread is done through mutex and condition. However, this alone is not enough. If a condition is signalized but without anybody listening signal is lost. To prevent this a boolean variable is used that is set iff nobody is listening but condition would be signalized, or iff driver is waiting on given condition.
If we want to talk to the monitor, then we can't do that in the virDomainDestroy API call. Your previous patches add a separate high priority queue to libvirtd, for dispatch of RPC calls which do *not* use the monitor which we need to always be processed immediately. virDomainDestroy is one of those high priority calls. We can't do routing of the RPC call based on flag values for the API, therefore, we must *never* use the monitor from virDomainDestroy. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 22.08.2011 12:27, Daniel P. Berrange wrote:
On Sat, Aug 20, 2011 at 01:06:10PM +0200, Michal Privoznik wrote:
This patch introduces support for domain destroying via 'quit' monitor command which gives qemu time to flush caches and therefore prevent disks corruption. However, qemu can be unresponsive and to prevent waiting indefinitely, execute command in a separate thread and wait reasonable time (QEMU_QUIT_WAIT_SECONDS) on a condition. If we hit timeout, qemu is qemuProcessKill'ed which causes monitor close and therefore also thread being terminable.
The synchronization between qemu driver and monitor-quit thread is done through mutex and condition. However, this alone is not enough. If a condition is signalized but without anybody listening signal is lost. To prevent this a boolean variable is used that is set iff nobody is listening but condition would be signalized, or iff driver is waiting on given condition.
If we want to talk to the monitor, then we can't do that in the virDomainDestroy API call.
Your previous patches add a separate high priority queue to libvirtd, for dispatch of RPC calls which do *not* use the monitor which we need to always be processed immediately. virDomainDestroy is one of those high priority calls. We can't do routing of the RPC call based on flag values for the API, therefore, we must *never* use the monitor from virDomainDestroy.
Regards, Daniel
I don't think that's problem. High priority calls must be guaranteed to end in reasonably short time. And although we talk to monitor here, we are guaranteed to end. Therefore no need to change my previous patch. 'Accessing monitor' is a bit unfortunate formulation, because 99% calls which do access monitor can block indefinitely. And this is the real problem. Stuck API. It doesn't really matter if it is because of monitor, NFS, etc. I just wanted to provide guide for developers if they are in doubt whether to mark API as high or low priority. Michal
On 08/22/2011 04:49 AM, Michal Privoznik wrote:
I don't think that's problem. High priority calls must be guaranteed to end in reasonably short time. And although we talk to monitor here, we are guaranteed to end. Therefore no need to change my previous patch.
No, we are not guaranteed to end in a reasonably short time. Monitor calls are, by nature, indefinite time commands, because they require response from an external process. Rather, we should add virDomainShutdownFlags() (which would be low priority, to match virDomainShutdown() being low priority), and make this a flag an option to virDomainShutdownFlags(). That is, you get a choice between shutdown via ACPI signal, or shutdown via monitor command, but both choices involve interaction with the monitor, and are therefore inherently liable to blocking (although shutdown via the monitor is much more likely to succeed in a short amount of time if the monitor is available, because it does not depend on the guest's reaction to ACPI).
'Accessing monitor' is a bit unfortunate formulation, because 99% calls which do access monitor can block indefinitely. And this is the real problem. Stuck API. It doesn't really matter if it is because of monitor, NFS, etc. I just wanted to provide guide for developers if they are in doubt whether to mark API as high or low priority.
The point is that virDomainDestroy() can't make a monitor command, or it will get stuck. I don't think it makes sense to give virDomainDestroyFlags() a flag that it can only honor in cases where the monitor is not stuck, but which gets skipped otherwise. I think that if we support a flag, then the use of the monitor has to be unconditional with that flag, which in turn implies that we need virDomainShutdownFlags(). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/22/2011 07:22 AM, Eric Blake wrote:
On 08/22/2011 04:49 AM, Michal Privoznik wrote:
I don't think that's problem. High priority calls must be guaranteed to end in reasonably short time. And although we talk to monitor here, we are guaranteed to end. Therefore no need to change my previous patch.
No, we are not guaranteed to end in a reasonably short time. Monitor calls are, by nature, indefinite time commands, because they require response from an external process.
Rather, we should add virDomainShutdownFlags() (which would be low priority, to match virDomainShutdown() being low priority), and make this a flag an option to virDomainShutdownFlags(). That is, you get a choice between shutdown via ACPI signal, or shutdown via monitor command, but both choices involve interaction with the monitor, and are therefore inherently liable to blocking (although shutdown via the monitor is much more likely to succeed in a short amount of time if the monitor is available, because it does not depend on the guest's reaction to ACPI).
'Accessing monitor' is a bit unfortunate formulation, because 99% calls which do access monitor can block indefinitely. And this is the real problem. Stuck API. It doesn't really matter if it is because of monitor, NFS, etc. I just wanted to provide guide for developers if they are in doubt whether to mark API as high or low priority.
The point is that virDomainDestroy() can't make a monitor command, or it will get stuck. I don't think it makes sense to give virDomainDestroyFlags() a flag that it can only honor in cases where the monitor is not stuck, but which gets skipped otherwise. I think that if we support a flag, then the use of the monitor has to be unconditional with that flag, which in turn implies that we need virDomainShutdownFlags().
I wrote the above before even glancing at your patch. But now, looking at the patch, it looks like you tried to take this into consideration - you are trying to add condition variables to guarantee that the monitor command will be attempted if possible, but that the command will give up on an unresponsive monitor and resort to the more drastic process kill, so that the command will always succeed even in the face of a stuck monitor. Maybe your approach has merit after all. But in that case, I think we need two ways to expose the monitor command: virDomainDestroyFlags(, VIR_DOMAIN_DESTROY_FLUSH_CACHE) - best effort try to flush cache, but may still lose data because the destroy is guaranteed to happen after x seconds even if monitor could not be obtained virDomainShutdownFlags(, VIR_DOMAIN_SHUTDOWN_FLUSH_CACHE) - guarantee that the flush cache happens, or that the command fails due to a timeout in obtaining the monitor -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 22.08.2011 15:30, Eric Blake wrote:
On 08/22/2011 07:22 AM, Eric Blake wrote:
On 08/22/2011 04:49 AM, Michal Privoznik wrote:
I don't think that's problem. High priority calls must be guaranteed to end in reasonably short time. And although we talk to monitor here, we are guaranteed to end. Therefore no need to change my previous patch.
No, we are not guaranteed to end in a reasonably short time. Monitor calls are, by nature, indefinite time commands, because they require response from an external process.
Rather, we should add virDomainShutdownFlags() (which would be low priority, to match virDomainShutdown() being low priority), and make this a flag an option to virDomainShutdownFlags(). That is, you get a choice between shutdown via ACPI signal, or shutdown via monitor command, but both choices involve interaction with the monitor, and are therefore inherently liable to blocking (although shutdown via the monitor is much more likely to succeed in a short amount of time if the monitor is available, because it does not depend on the guest's reaction to ACPI).
'Accessing monitor' is a bit unfortunate formulation, because 99% calls which do access monitor can block indefinitely. And this is the real problem. Stuck API. It doesn't really matter if it is because of monitor, NFS, etc. I just wanted to provide guide for developers if they are in doubt whether to mark API as high or low priority.
The point is that virDomainDestroy() can't make a monitor command, or it will get stuck. I don't think it makes sense to give virDomainDestroyFlags() a flag that it can only honor in cases where the monitor is not stuck, but which gets skipped otherwise. I think that if we support a flag, then the use of the monitor has to be unconditional with that flag, which in turn implies that we need virDomainShutdownFlags().
I wrote the above before even glancing at your patch. But now, looking at the patch, it looks like you tried to take this into consideration - you are trying to add condition variables to guarantee that the monitor command will be attempted if possible, but that the command will give up on an unresponsive monitor and resort to the more drastic process kill, so that the command will always succeed even in the face of a stuck monitor.
Maybe your approach has merit after all. But in that case, I think we need two ways to expose the monitor command:
virDomainDestroyFlags(, VIR_DOMAIN_DESTROY_FLUSH_CACHE) - best effort try to flush cache, but may still lose data because the destroy is guaranteed to happen after x seconds even if monitor could not be obtained
virDomainShutdownFlags(, VIR_DOMAIN_SHUTDOWN_FLUSH_CACHE) - guarantee that the flush cache happens, or that the command fails due to a timeout in obtaining the monitor
Agree. I've sent v2 for destroy (DV suggested some corrections). And for shutdown - I will sent follow up patch.
On Mon, Aug 22, 2011 at 12:49:48PM +0200, Michal Privoznik wrote:
On 22.08.2011 12:27, Daniel P. Berrange wrote:
On Sat, Aug 20, 2011 at 01:06:10PM +0200, Michal Privoznik wrote:
This patch introduces support for domain destroying via 'quit' monitor command which gives qemu time to flush caches and therefore prevent disks corruption. However, qemu can be unresponsive and to prevent waiting indefinitely, execute command in a separate thread and wait reasonable time (QEMU_QUIT_WAIT_SECONDS) on a condition. If we hit timeout, qemu is qemuProcessKill'ed which causes monitor close and therefore also thread being terminable.
The synchronization between qemu driver and monitor-quit thread is done through mutex and condition. However, this alone is not enough. If a condition is signalized but without anybody listening signal is lost. To prevent this a boolean variable is used that is set iff nobody is listening but condition would be signalized, or iff driver is waiting on given condition.
If we want to talk to the monitor, then we can't do that in the virDomainDestroy API call.
Your previous patches add a separate high priority queue to libvirtd, for dispatch of RPC calls which do *not* use the monitor which we need to always be processed immediately. virDomainDestroy is one of those high priority calls. We can't do routing of the RPC call based on flag values for the API, therefore, we must *never* use the monitor from virDomainDestroy.
Regards, Daniel
I don't think that's problem. High priority calls must be guaranteed to end in reasonably short time. And although we talk to monitor here, we are guaranteed to end. Therefore no need to change my previous patch.
In order to guarentee that it ends though, it is defining another arbitrary timeout for the monitor commands, this time at 5 seconds. What if the management app wants to wait more than 5 seconds before falling back to kill(TERM) for QEMU ? If we had a separate API for sending 'quit' on the monitor, then the mgmt app can decide how long to wait for the graceful shutdown of QEMU before resorting to the hard virDomainDestroy command. If the app knows that there is high I/O load, then it might want to wait for 'quit' to complete longer than normal to allow enough time for I/O flush. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/22/2011 09:21 AM, Daniel P. Berrange wrote:
If we had a separate API for sending 'quit' on the monitor, then the mgmt app can decide how long to wait for the graceful shutdown of QEMU before resorting to the hard virDomainDestroy command. If the app knows that there is high I/O load, then it might want to wait for 'quit' to complete longer than normal to allow enough time for I/O flush.
Indeed - that is exactly what I was envisioning with a virDomainShutdownFlags() call with a flag to request to use the quit monitor command instead of the default ACPI injection. The virDomainShutdownFlags() would have no timeout (it blocks until successful, or returns failure with no 'quit' command attempted), and the caller can inject their own unconditional virDomainDestroy() at whatever timeout they think is appropriate. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Aug 22, 2011 at 09:29:56AM -0600, Eric Blake wrote:
On 08/22/2011 09:21 AM, Daniel P. Berrange wrote:
If we had a separate API for sending 'quit' on the monitor, then the mgmt app can decide how long to wait for the graceful shutdown of QEMU before resorting to the hard virDomainDestroy command. If the app knows that there is high I/O load, then it might want to wait for 'quit' to complete longer than normal to allow enough time for I/O flush.
Indeed - that is exactly what I was envisioning with a virDomainShutdownFlags() call with a flag to request to use the quit monitor command instead of the default ACPI injection. The virDomainShutdownFlags() would have no timeout (it blocks until successful, or returns failure with no 'quit' command attempted), and the caller can inject their own unconditional virDomainDestroy() at whatever timeout they think is appropriate.
The virDomainShutdown API is really about guest initiated graceful shutdown. Sending the 'quit' command to QEMU is still *ungraceful* as far as the guest OS is concerned, so I think it is best not to leverage the Shutdown API for 'quit'. I think this probably calls for a virDomainQuit API. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Aug 22, 2011 at 05:33:12PM +0100, Daniel P. Berrange wrote:
On Mon, Aug 22, 2011 at 09:29:56AM -0600, Eric Blake wrote:
On 08/22/2011 09:21 AM, Daniel P. Berrange wrote:
If we had a separate API for sending 'quit' on the monitor, then the mgmt app can decide how long to wait for the graceful shutdown of QEMU before resorting to the hard virDomainDestroy command. If the app knows that there is high I/O load, then it might want to wait for 'quit' to complete longer than normal to allow enough time for I/O flush.
Indeed - that is exactly what I was envisioning with a virDomainShutdownFlags() call with a flag to request to use the quit monitor command instead of the default ACPI injection. The virDomainShutdownFlags() would have no timeout (it blocks until successful, or returns failure with no 'quit' command attempted), and the caller can inject their own unconditional virDomainDestroy() at whatever timeout they think is appropriate.
The virDomainShutdown API is really about guest initiated graceful shutdown. Sending the 'quit' command to QEMU is still *ungraceful* as far as the guest OS is concerned, so I think it is best not to leverage the Shutdown API for 'quit'.
I think this probably calls for a virDomainQuit API.
Actually this entire thread is on the wrong path. Both the monitor 'quit' command and 'SIGTERM' in QEMU do exactly the same thing. A immediate stop of the guest, but with a graceful shutdown of the QEMU process[1]. In theory there is a difference that sending a signal is asynchronous and 'quit' is a synchronous command, but in practice this is not relevant, since while executio nof the 'quit' command is synchronous, this command only makes the *request* to exit. QEMU won't actually exit until the event loop processes the request later. There is thus no point in us even bothering with sending 'quit' to the QEMU monitor. The virDomainDestroy method calls qemuProcessKill which sends SIGTERM, waits a short while, then sends SIGKILL. It then calls qemuProcessStop to reap the process. This also happens to call qemuProcessKill again with SIGTERM, then SIGKILL. We need to make this more controllable by apps, by making it possible to send just the SIGTERM and not the SIGKILL. Then we can add a new flag to virDomainDestroy to request this SIGTERM only behaviour. If the guest does not actually die, the mgmt app can then just reinvoke virDomainDestroy without the flag, to get the full SIGTERM+SIGKILL behaviour we have today. So there's no need for the QEMU monitor to be involved anywhere in this. Regards, Daniel [1] The SIGTERM handler and 'quit' command handler both end up just calling qemu_system_shutdown_request(). -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 22.08.2011 20:31, Daniel P. Berrange wrote:
On Mon, Aug 22, 2011 at 05:33:12PM +0100, Daniel P. Berrange wrote:
On Mon, Aug 22, 2011 at 09:29:56AM -0600, Eric Blake wrote:
On 08/22/2011 09:21 AM, Daniel P. Berrange wrote:
If we had a separate API for sending 'quit' on the monitor, then the mgmt app can decide how long to wait for the graceful shutdown of QEMU before resorting to the hard virDomainDestroy command. If the app knows that there is high I/O load, then it might want to wait for 'quit' to complete longer than normal to allow enough time for I/O flush.
Indeed - that is exactly what I was envisioning with a virDomainShutdownFlags() call with a flag to request to use the quit monitor command instead of the default ACPI injection. The virDomainShutdownFlags() would have no timeout (it blocks until successful, or returns failure with no 'quit' command attempted), and the caller can inject their own unconditional virDomainDestroy() at whatever timeout they think is appropriate.
The virDomainShutdown API is really about guest initiated graceful shutdown. Sending the 'quit' command to QEMU is still *ungraceful* as far as the guest OS is concerned, so I think it is best not to leverage the Shutdown API for 'quit'.
I think this probably calls for a virDomainQuit API.
Actually this entire thread is on the wrong path.
Both the monitor 'quit' command and 'SIGTERM' in QEMU do exactly the same thing. A immediate stop of the guest, but with a graceful shutdown of the QEMU process[1].
In theory there is a difference that sending a signal is asynchronous and 'quit' is a synchronous command, but in practice this is not relevant, since while executio nof the 'quit' command is synchronous, this command only makes the *request* to exit. QEMU won't actually exit until the event loop processes the request later.
There is thus no point in us even bothering with sending 'quit' to the QEMU monitor.
The virDomainDestroy method calls qemuProcessKill which sends SIGTERM, waits a short while, then sends SIGKILL. It then calls qemuProcessStop to reap the process. This also happens to call qemuProcessKill again with SIGTERM, then SIGKILL.
We need to make this more controllable by apps, by making it possible to send just the SIGTERM and not the SIGKILL. Then we can add a new flag to virDomainDestroy to request this SIGTERM only behaviour. If the guest does not actually die, the mgmt app can then just reinvoke virDomainDestroy without the flag, to get the full SIGTERM+SIGKILL behaviour we have today.
Sending signal to qemu process is just a part of domain destroying. What about cleanup code (emitting event, audit log, removing transient domain, ...)? Can I rely on monitor EOF handling code? What should be the return value for this case when only SIGTERM is sent?
So there's no need for the QEMU monitor to be involved anywhere in this.
Regards, Daniel
[1] The SIGTERM handler and 'quit' command handler both end up just calling qemu_system_shutdown_request().
On Tue, Aug 23, 2011 at 02:36:02PM +0200, Michal Privoznik wrote:
On 22.08.2011 20:31, Daniel P. Berrange wrote:
On Mon, Aug 22, 2011 at 05:33:12PM +0100, Daniel P. Berrange wrote:
On Mon, Aug 22, 2011 at 09:29:56AM -0600, Eric Blake wrote:
On 08/22/2011 09:21 AM, Daniel P. Berrange wrote:
If we had a separate API for sending 'quit' on the monitor, then the mgmt app can decide how long to wait for the graceful shutdown of QEMU before resorting to the hard virDomainDestroy command. If the app knows that there is high I/O load, then it might want to wait for 'quit' to complete longer than normal to allow enough time for I/O flush.
Indeed - that is exactly what I was envisioning with a virDomainShutdownFlags() call with a flag to request to use the quit monitor command instead of the default ACPI injection. The virDomainShutdownFlags() would have no timeout (it blocks until successful, or returns failure with no 'quit' command attempted), and the caller can inject their own unconditional virDomainDestroy() at whatever timeout they think is appropriate.
The virDomainShutdown API is really about guest initiated graceful shutdown. Sending the 'quit' command to QEMU is still *ungraceful* as far as the guest OS is concerned, so I think it is best not to leverage the Shutdown API for 'quit'.
I think this probably calls for a virDomainQuit API.
Actually this entire thread is on the wrong path.
Both the monitor 'quit' command and 'SIGTERM' in QEMU do exactly the same thing. A immediate stop of the guest, but with a graceful shutdown of the QEMU process[1].
In theory there is a difference that sending a signal is asynchronous and 'quit' is a synchronous command, but in practice this is not relevant, since while executio nof the 'quit' command is synchronous, this command only makes the *request* to exit. QEMU won't actually exit until the event loop processes the request later.
There is thus no point in us even bothering with sending 'quit' to the QEMU monitor.
The virDomainDestroy method calls qemuProcessKill which sends SIGTERM, waits a short while, then sends SIGKILL. It then calls qemuProcessStop to reap the process. This also happens to call qemuProcessKill again with SIGTERM, then SIGKILL.
We need to make this more controllable by apps, by making it possible to send just the SIGTERM and not the SIGKILL. Then we can add a new flag to virDomainDestroy to request this SIGTERM only behaviour. If the guest does not actually die, the mgmt app can then just reinvoke virDomainDestroy without the flag, to get the full SIGTERM+SIGKILL behaviour we have today.
Sending signal to qemu process is just a part of domain destroying. What about cleanup code (emitting event, audit log, removing transient domain, ...)? Can I rely on monitor EOF handling code? What should be the return value for this case when only SIGTERM is sent?
QEMU will send an event on the monitor when it shuts down cleanly via 'SIGQUIT' - we already handle that. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Tue, Aug 23, 2011 at 14:31:29 +0100, Daniel P. Berrange wrote:
On Tue, Aug 23, 2011 at 02:36:02PM +0200, Michal Privoznik wrote:
On 22.08.2011 20:31, Daniel P. Berrange wrote:
We need to make this more controllable by apps, by making it possible to send just the SIGTERM and not the SIGKILL. Then we can add a new flag to virDomainDestroy to request this SIGTERM only behaviour. If the guest does not actually die, the mgmt app can then just reinvoke virDomainDestroy without the flag, to get the full SIGTERM+SIGKILL behaviour we have today.
Sending signal to qemu process is just a part of domain destroying. What about cleanup code (emitting event, audit log, removing transient domain, ...)? Can I rely on monitor EOF handling code? What should be the return value for this case when only SIGTERM is sent?
QEMU will send an event on the monitor when it shuts down cleanly via 'SIGQUIT' - we already handle that.
Yes, but that will confuse libvirt and apps because we won't be able to distinguish between normal shutdown and destroy with flushed caches. But that should probably be solved in qemu by sending different events in this two cases. Jirka
On Wed, Aug 24, 2011 at 09:53:49AM +0200, Jiri Denemark wrote:
On Tue, Aug 23, 2011 at 14:31:29 +0100, Daniel P. Berrange wrote:
On Tue, Aug 23, 2011 at 02:36:02PM +0200, Michal Privoznik wrote:
On 22.08.2011 20:31, Daniel P. Berrange wrote:
We need to make this more controllable by apps, by making it possible to send just the SIGTERM and not the SIGKILL. Then we can add a new flag to virDomainDestroy to request this SIGTERM only behaviour. If the guest does not actually die, the mgmt app can then just reinvoke virDomainDestroy without the flag, to get the full SIGTERM+SIGKILL behaviour we have today.
Sending signal to qemu process is just a part of domain destroying. What about cleanup code (emitting event, audit log, removing transient domain, ...)? Can I rely on monitor EOF handling code? What should be the return value for this case when only SIGTERM is sent?
QEMU will send an event on the monitor when it shuts down cleanly via 'SIGQUIT' - we already handle that.
Yes, but that will confuse libvirt and apps because we won't be able to distinguish between normal shutdown and destroy with flushed caches. But that should probably be solved in qemu by sending different events in this two cases.
Well if that is the case, then we already have that problem, because libvirt is already sending SIGQUIT to destroy QEMU. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Jiri Denemark -
Michal Privoznik