5:02 p.m.
This series is the result of a report from Kim Larry [1] about missing
shutdown events in the libxl driver. Kim's report revealed that the
libxl driver was ignoring the <on_*> domain lifecycle event configuration!
Patch1 adds support for the four standard actions (destroy, restart,
preserve, and rename-restart) of <on_*>. Patch2 queues the shutdown
event earlier in the shutdown handler, to avoid it being delivered
after the a start event in the case of a restart. Patches 3 and 4 add
support for the extra <on_crash> actions coredump-destroy and
coredump-restart.
[1] https://www.redhat.com/archives/libvir-list/2014-February/msg01121.html
Jim Fehlig (4):
libxl: honor domain lifecycle event configuration
libxl: queue domain event earlier in shutdown handler
libxl: add dump dir to libxlDriverConfig object
libxl: handle on_crash coredump actions
src/libxl/libxl_conf.c | 3 +
src/libxl/libxl_conf.h | 2 +
src/libxl/libxl_driver.c | 148 ++++++++++++++++++++++++++++++++++++++---------
3 files changed, 125 insertions(+), 28 deletions(-)
--
1.8.1.4
5:02 p.m.
New subject: [libvirt] [PATCH 1/4] libxl: honor domain lifecycle event configuration
The libxl driver was ignoring the <on_*> domain event configuration,
causing e.g. a domain to be rebooted even when on_reboot is set to
destroy.
This patch honors the <on_*> configuration in the shutdown event
handler.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 83 +++++++++++++++++++++++++++++++++---------------
1 file changed, 57 insertions(+), 26 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c7c7b4f..721577d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -372,38 +372,69 @@ libxlDomainShutdownThread(void *opaque)
virObjectLock(vm);
- switch (xl_reason) {
- case LIBXL_SHUTDOWN_REASON_POWEROFF:
- case LIBXL_SHUTDOWN_REASON_CRASH:
- if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
- dom_event = virDomainEventLifecycleNewFromObj(vm,
+ if (xl_reason == LIBXL_SHUTDOWN_REASON_POWEROFF) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ switch (vm->def->onPoweroff) {
+ case VIR_DOMAIN_LIFECYCLE_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ goto cleanup;
+ }
+ } else if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
- reason = VIR_DOMAIN_SHUTOFF_CRASHED;
- } else {
- dom_event = virDomainEventLifecycleNewFromObj(vm,
+ switch (vm->def->onCrash) {
+ case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_CRASHED;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE:
+ goto cleanup;
+ }
+ } else if (xl_reason == LIBXL_SHUTDOWN_REASON_REBOOT) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
- reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
- }
- libxl_domain_destroy(ctx, vm->def->id, NULL);
- if (libxlVmCleanupJob(driver, vm, reason)) {
- if (!vm->persistent) {
- virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
- }
- }
- break;
- case LIBXL_SHUTDOWN_REASON_REBOOT:
- libxl_domain_destroy(ctx, vm->def->id, NULL);
- libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
- libxlVmStart(driver, vm, 0, -1);
- break;
- default:
- VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
- break;
+ switch (vm->def->onReboot) {
+ case VIR_DOMAIN_LIFECYCLE_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ goto cleanup;
+ }
+ } else {
+ VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
+ goto cleanup;
}
+destroy:
+ libxl_domain_destroy(ctx, vm->def->id, NULL);
+ if (libxlVmCleanupJob(driver, vm, reason)) {
+ if (!vm->persistent) {
+ virDomainObjListRemove(driver->domains, vm);
+ vm = NULL;
+ }
+ }
+ goto cleanup;
+
+restart:
+ libxl_domain_destroy(ctx, vm->def->id, NULL);
+ libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+ libxlVmStart(driver, vm, 0, -1);
+
+cleanup:
if (vm)
virObjectUnlock(vm);
if (dom_event)
--
1.8.1.4
6:19 p.m.
New subject: [libvirt] [PATCH 1/4] libxl: honor domain lifecycle event configuration
Jim Fehlig wrote:
The libxl driver was ignoring the <on_*> domain event
configuration,
causing e.g. a domain to be rebooted even when on_reboot is set to
destroy.
This patch honors the <on_*> configuration in the shutdown event
handler.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 83 +++++++++++++++++++++++++++++++++---------------
1 file changed, 57 insertions(+), 26 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c7c7b4f..721577d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -372,38 +372,69 @@ libxlDomainShutdownThread(void *opaque)
virObjectLock(vm);
- switch (xl_reason) {
- case LIBXL_SHUTDOWN_REASON_POWEROFF:
- case LIBXL_SHUTDOWN_REASON_CRASH:
- if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
- dom_event = virDomainEventLifecycleNewFromObj(vm,
+ if (xl_reason == LIBXL_SHUTDOWN_REASON_POWEROFF) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ switch (vm->def->onPoweroff) {
+ case VIR_DOMAIN_LIFECYCLE_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ goto cleanup;
+ }
+ } else if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
- reason = VIR_DOMAIN_SHUTOFF_CRASHED;
- } else {
- dom_event = virDomainEventLifecycleNewFromObj(vm,
+ switch (vm->def->onCrash) {
+ case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_CRASHED;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE:
+ goto cleanup;
+ }
+ } else if (xl_reason == LIBXL_SHUTDOWN_REASON_REBOOT) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
- reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
- }
- libxl_domain_destroy(ctx, vm->def->id, NULL);
- if (libxlVmCleanupJob(driver, vm, reason)) {
- if (!vm->persistent) {
- virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
- }
- }
- break;
- case LIBXL_SHUTDOWN_REASON_REBOOT:
- libxl_domain_destroy(ctx, vm->def->id, NULL);
- libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
- libxlVmStart(driver, vm, 0, -1);
- break;
- default:
- VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
- break;
+ switch (vm->def->onReboot) {
+ case VIR_DOMAIN_LIFECYCLE_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ goto cleanup;
+ }
+ } else {
+ VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
+ goto cleanup;
}
+destroy:
+ libxl_domain_destroy(ctx, vm->def->id, NULL);
+ if (libxlVmCleanupJob(driver, vm, reason)) {
+ if (!vm->persistent) {
+ virDomainObjListRemove(driver->domains, vm);
+ vm = NULL;
+ }
+ }
+ goto cleanup;
+
+restart:
+ libxl_domain_destroy(ctx, vm->def->id, NULL);
+ libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
Opps, botched rebasing of my own branches. This should be
libxlVmCleanupJob. I've fixed it in my local branch.
Regards,
Jim
+ libxlVmStart(driver, vm, 0, -1);
+
+cleanup:
if (vm)
virObjectUnlock(vm);
if (dom_event)
7:39 a.m.
New subject: [libvirt] [PATCH 1/4] libxl: honor domain lifecycle event configuration
On 21.02.2014 00:02, Jim Fehlig wrote:
The libxl driver was ignoring the <on_*> domain event
configuration,
causing e.g. a domain to be rebooted even when on_reboot is set to
destroy.
This patch honors the <on_*> configuration in the shutdown event
handler.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 83 +++++++++++++++++++++++++++++++++---------------
1 file changed, 57 insertions(+), 26 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c7c7b4f..721577d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -372,38 +372,69 @@ libxlDomainShutdownThread(void *opaque)
virObjectLock(vm);
- switch (xl_reason) {
- case LIBXL_SHUTDOWN_REASON_POWEROFF:
- case LIBXL_SHUTDOWN_REASON_CRASH:
- if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
- dom_event = virDomainEventLifecycleNewFromObj(vm,
+ if (xl_reason == LIBXL_SHUTDOWN_REASON_POWEROFF) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ switch (vm->def->onPoweroff) {
+ case VIR_DOMAIN_LIFECYCLE_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ goto cleanup;
+ }
+ } else if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
- reason = VIR_DOMAIN_SHUTOFF_CRASHED;
- } else {
- dom_event = virDomainEventLifecycleNewFromObj(vm,
+ switch (vm->def->onCrash) {
+ case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_CRASHED;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE:
+ goto cleanup;
+ }
+ } else if (xl_reason == LIBXL_SHUTDOWN_REASON_REBOOT) {
+ dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
- reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
- }
- libxl_domain_destroy(ctx, vm->def->id, NULL);
- if (libxlVmCleanupJob(driver, vm, reason)) {
- if (!vm->persistent) {
- virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
- }
- }
- break;
- case LIBXL_SHUTDOWN_REASON_REBOOT:
- libxl_domain_destroy(ctx, vm->def->id, NULL);
- libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
- libxlVmStart(driver, vm, 0, -1);
- break;
- default:
- VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
- break;
+ switch (vm->def->onReboot) {
+ case VIR_DOMAIN_LIFECYCLE_DESTROY:
+ reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
+ goto restart;
+ case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ goto cleanup;
+ }
+ } else {
+ VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
+ goto cleanup;
}
+destroy:
+ libxl_domain_destroy(ctx, vm->def->id, NULL);
+ if (libxlVmCleanupJob(driver, vm, reason)) {
Funny, my gcc-4.8.2 warns me about @reason may be used uninitialized here.
+ if (!vm->persistent) {
+ virDomainObjListRemove(driver->domains, vm);
+ vm = NULL;
+ }
+ }
+ goto cleanup;
+
+restart:
+ libxl_domain_destroy(ctx, vm->def->id, NULL);
+ libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
Right. You've already notice that needs to be libxlVmCleanupJob().
+ libxlVmStart(driver, vm, 0, -1);
+
+cleanup:
if (vm)
virObjectUnlock(vm);
if (dom_event)
ACK with this squashed in:
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 721577d..0b9bf7d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -368,7 +368,7 @@ libxlDomainShutdownThread(void *opaque)
libxl_ctx *ctx = priv->ctx;
virObjectEventPtr dom_event = NULL;
libxl_shutdown_reason xl_reason = ev->u.domain_shutdown.shutdown_reason;
- virDomainShutoffReason reason;
+ virDomainShutoffReason reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
virObjectLock(vm);
@@ -376,7 +376,7 @@ libxlDomainShutdownThread(void *opaque)
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
- switch (vm->def->onPoweroff) {
+ switch ((enum virDomainLifecycleAction) vm->def->onPoweroff) {
case VIR_DOMAIN_LIFECYCLE_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
goto destroy;
@@ -384,13 +384,14 @@ libxlDomainShutdownThread(void *opaque)
case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
goto restart;
case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ case VIR_DOMAIN_LIFECYCLE_LAST:
goto cleanup;
}
} else if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
- switch (vm->def->onCrash) {
+ switch ((enum virDomainLifecycleCrashAction) vm->def->onCrash) {
case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_CRASHED;
goto destroy;
@@ -398,13 +399,16 @@ libxlDomainShutdownThread(void *opaque)
case VIR_DOMAIN_LIFECYCLE_CRASH_RESTART_RENAME:
goto restart;
case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE:
+ case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY:
+ case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_CRASH_LAST:
goto cleanup;
}
} else if (xl_reason == LIBXL_SHUTDOWN_REASON_REBOOT) {
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
- switch (vm->def->onReboot) {
+ switch ((enum virDomainLifecycleAction) vm->def->onReboot) {
case VIR_DOMAIN_LIFECYCLE_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
goto destroy;
@@ -412,6 +416,7 @@ libxlDomainShutdownThread(void *opaque)
case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
goto restart;
case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ case VIR_DOMAIN_LIFECYCLE_LAST:
goto cleanup;
}
} else {
@@ -431,7 +436,7 @@ destroy:
restart:
libxl_domain_destroy(ctx, vm->def->id, NULL);
- libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+ libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
libxlVmStart(driver, vm, 0, -1);
cleanup:
Michal
8:56 a.m.
New subject: [libvirt] [PATCH 1/4] libxl: honor domain lifecycle event configuration
Michal Privoznik wrote:
On 21.02.2014 00:02, Jim Fehlig wrote:
> The libxl driver was ignoring the <on_*> domain event configuration,
> causing e.g. a domain to be rebooted even when on_reboot is set to
> destroy.
>
> This patch honors the <on_*> configuration in the shutdown event
> handler.
>
[...]
ACK with this squashed in:
Can this series be pushed for 1.2.2? If so, I'll squash in your changes
and push later today. Thanks for the review!
Regards,
Jim
9:17 a.m.
New subject: [libvirt] [PATCH 1/4] libxl: honor domain lifecycle event configuration
On 24.02.2014 15:56, Jim Fehlig wrote:
Michal Privoznik wrote:
> On 21.02.2014 00:02, Jim Fehlig wrote:
>
>> The libxl driver was ignoring the <on_*> domain event configuration,
>> causing e.g. a domain to be rebooted even when on_reboot is set to
>> destroy.
>>
>> This patch honors the <on_*> configuration in the shutdown event
>> handler.
>>
[...]
>
> ACK with this squashed in:
>
Can this series be pushed for 1.2.2? If so, I'll squash in your changes
and push later today. Thanks for the review!
Regards,
Jim
I think these patches are bugfix. Which allows us to push them even in
freeze. Also note that I've not ACKed 2/4 as I think a different pattern
is better. However, that does not stop you from pushing the rest.
Michal
9:21 a.m.
New subject: [libvirt] [PATCH 1/4] libxl: honor domain lifecycle event configuration
On Mon, Feb 24, 2014 at 04:17:52PM +0100, Michal Privoznik wrote:
On 24.02.2014 15:56, Jim Fehlig wrote:
>Michal Privoznik wrote:
>>On 21.02.2014 00:02, Jim Fehlig wrote:
>>
>>>The libxl driver was ignoring the <on_*> domain event configuration,
>>>causing e.g. a domain to be rebooted even when on_reboot is set to
>>>destroy.
>>>
>>>This patch honors the <on_*> configuration in the shutdown event
>>>handler.
>>>
>[...]
>>
>>ACK with this squashed in:
>>
>
>Can this series be pushed for 1.2.2? If so, I'll squash in your changes
>and push later today. Thanks for the review!
>
>Regards,
>Jim
>
I think these patches are bugfix. Which allows us to push them even
in freeze. Also note that I've not ACKed 2/4 as I think a different
pattern is better. However, that does not stop you from pushing the
rest.
Agreed, it was posted before freeze too, so fine to commit IMO
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 :|
12:14 p.m.
New subject: [libvirt] [PATCH 1/4] libxl: honor domain lifecycle event configuration
Michal Privoznik wrote:
On 24.02.2014 15:56, Jim Fehlig wrote:
>
> Can this series be pushed for 1.2.2? If so, I'll squash in your changes
> and push later today. Thanks for the review!
>
> Regards,
> Jim
>
I think these patches are bugfix. Which allows us to push them even in
freeze. Also note that I've not ACKed 2/4 as I think a different
pattern is better. However, that does not stop you from pushing the rest.
I pushed 1, 3, and 4 and resent 2 with your suggested change
https://www.redhat.com/archives/libvir-list/2014-February/msg01500.html
Regards,
Jim
5:02 p.m.
New subject: [libvirt] [PATCH 2/4] libxl: queue domain event earlier in shutdown handler
The shutdown handler may restart a domain when handling a reboot
event or when <on_*> is set to 'restart'. Restarting consists of
calling libxlVmCleanup followed by libxlVmStart. libxlVmStart will
emit a VIR_DOMAIN_EVENT_STARTED event, but the SHUTDOWN event is
not emitted until exiting the shutdown handler, after the STARTED
event. Queue the event immediately after creation to avoid emitting
it after the start event.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 721577d..e600de7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -376,6 +376,8 @@ libxlDomainShutdownThread(void *opaque)
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ if (dom_event)
+ libxlDomainEventQueue(driver, dom_event);
switch (vm->def->onPoweroff) {
case VIR_DOMAIN_LIFECYCLE_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
@@ -390,6 +392,8 @@ libxlDomainShutdownThread(void *opaque)
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
+ if (dom_event)
+ libxlDomainEventQueue(driver, dom_event);
switch (vm->def->onCrash) {
case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_CRASHED;
@@ -404,6 +408,8 @@ libxlDomainShutdownThread(void *opaque)
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ if (dom_event)
+ libxlDomainEventQueue(driver, dom_event);
switch (vm->def->onReboot) {
case VIR_DOMAIN_LIFECYCLE_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
@@ -437,8 +443,6 @@ restart:
cleanup:
if (vm)
virObjectUnlock(vm);
- if (dom_event)
- libxlDomainEventQueue(driver, dom_event);
libxl_event_free(ctx, ev);
VIR_FREE(shutdown_info);
}
--
1.8.1.4
7:39 a.m.
New subject: [libvirt] [PATCH 2/4] libxl: queue domain event earlier in shutdown handler
On 21.02.2014 00:02, Jim Fehlig wrote:
The shutdown handler may restart a domain when handling a reboot
event or when <on_*> is set to 'restart'. Restarting consists of
calling libxlVmCleanup followed by libxlVmStart. libxlVmStart will
emit a VIR_DOMAIN_EVENT_STARTED event, but the SHUTDOWN event is
not emitted until exiting the shutdown handler, after the STARTED
event. Queue the event immediately after creation to avoid emitting
it after the start event.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 721577d..e600de7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -376,6 +376,8 @@ libxlDomainShutdownThread(void *opaque)
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ if (dom_event)
+ libxlDomainEventQueue(driver, dom_event);
switch (vm->def->onPoweroff) {
case VIR_DOMAIN_LIFECYCLE_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
@@ -390,6 +392,8 @@ libxlDomainShutdownThread(void *opaque)
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
+ if (dom_event)
+ libxlDomainEventQueue(driver, dom_event);
switch (vm->def->onCrash) {
case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_CRASHED;
@@ -404,6 +408,8 @@ libxlDomainShutdownThread(void *opaque)
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ if (dom_event)
+ libxlDomainEventQueue(driver, dom_event);
switch (vm->def->onReboot) {
case VIR_DOMAIN_LIFECYCLE_DESTROY:
reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
@@ -437,8 +443,6 @@ restart:
cleanup:
if (vm)
virObjectUnlock(vm);
- if (dom_event)
- libxlDomainEventQueue(driver, dom_event);
libxl_event_free(ctx, ev);
VIR_FREE(shutdown_info);
}
Wouldn't it be better to enqueue events at the beginning of 'destroy'
and 'restart' labels? I'm thinking about something among these lines:
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0b9bf7d..c009407 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -425,6 +425,10 @@ libxlDomainShutdownThread(void *opaque)
}
destroy:
+ if (dom_event) {
+ libxlDomainEventQueue(driver, dom_event);
+ dom_event = NULL;
+ }
libxl_domain_destroy(ctx, vm->def->id, NULL);
if (libxlVmCleanupJob(driver, vm, reason)) {
if (!vm->persistent) {
@@ -435,6 +439,10 @@ destroy:
goto cleanup;
restart:
+ if (dom_event) {
+ libxlDomainEventQueue(driver, dom_event);
+ dom_event = NULL;
+ }
libxl_domain_destroy(ctx, vm->def->id, NULL);
libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
libxlVmStart(driver, vm, 0, -1);
Michal
10:04 a.m.
New subject: [libvirt] [PATCH 2/4] libxl: queue domain event earlier in shutdown handler
Michal Privoznik wrote:
On 21.02.2014 00:02, Jim Fehlig wrote:
> The shutdown handler may restart a domain when handling a reboot
> event or when <on_*> is set to 'restart'. Restarting consists of
> calling libxlVmCleanup followed by libxlVmStart. libxlVmStart will
> emit a VIR_DOMAIN_EVENT_STARTED event, but the SHUTDOWN event is
> not emitted until exiting the shutdown handler, after the STARTED
> event. Queue the event immediately after creation to avoid emitting
> it after the start event.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_driver.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 721577d..e600de7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -376,6 +376,8 @@ libxlDomainShutdownThread(void *opaque)
> dom_event = virDomainEventLifecycleNewFromObj(vm,
> VIR_DOMAIN_EVENT_STOPPED,
>
> VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
> + if (dom_event)
> + libxlDomainEventQueue(driver, dom_event);
> switch (vm->def->onPoweroff) {
> case VIR_DOMAIN_LIFECYCLE_DESTROY:
> reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
> @@ -390,6 +392,8 @@ libxlDomainShutdownThread(void *opaque)
> dom_event = virDomainEventLifecycleNewFromObj(vm,
> VIR_DOMAIN_EVENT_STOPPED,
>
> VIR_DOMAIN_EVENT_STOPPED_CRASHED);
> + if (dom_event)
> + libxlDomainEventQueue(driver, dom_event);
> switch (vm->def->onCrash) {
> case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
> reason = VIR_DOMAIN_SHUTOFF_CRASHED;
> @@ -404,6 +408,8 @@ libxlDomainShutdownThread(void *opaque)
> dom_event = virDomainEventLifecycleNewFromObj(vm,
> VIR_DOMAIN_EVENT_STOPPED,
>
> VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
> + if (dom_event)
> + libxlDomainEventQueue(driver, dom_event);
> switch (vm->def->onReboot) {
> case VIR_DOMAIN_LIFECYCLE_DESTROY:
> reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
> @@ -437,8 +443,6 @@ restart:
> cleanup:
> if (vm)
> virObjectUnlock(vm);
> - if (dom_event)
> - libxlDomainEventQueue(driver, dom_event);
> libxl_event_free(ctx, ev);
> VIR_FREE(shutdown_info);
> }
>
Wouldn't it be better to enqueue events at the beginning of 'destroy'
and 'restart' labels? I'm thinking about something among these lines:
Yes, agreed. As discussed on IRC, better to enqueue the event at the
labels since it is one less call to libxlDomainEventQueue, and improves
the code to handle any future <on_foo> action.
Regards,
Jim
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0b9bf7d..c009407 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -425,6 +425,10 @@ libxlDomainShutdownThread(void *opaque)
}
destroy:
+ if (dom_event) {
+ libxlDomainEventQueue(driver, dom_event);
+ dom_event = NULL;
+ }
libxl_domain_destroy(ctx, vm->def->id, NULL);
if (libxlVmCleanupJob(driver, vm, reason)) {
if (!vm->persistent) {
@@ -435,6 +439,10 @@ destroy:
goto cleanup;
restart:
+ if (dom_event) {
+ libxlDomainEventQueue(driver, dom_event);
+ dom_event = NULL;
+ }
libxl_domain_destroy(ctx, vm->def->id, NULL);
libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
libxlVmStart(driver, vm, 0, -1);
Michal
5:02 p.m.
New subject: [libvirt] [PATCH 3/4] libxl: add dump dir to libxlDriverConfig object
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
Without a libxl config file, I suppose the only thing needed here is
the #define in libxl_conf.h. But I have a dusty series adding lockd
support to the libxl driver, which I'll be cleaning off soon. That
series introduces libxl.conf, and can be expanded to include an auto
dump dir entry.
src/libxl/libxl_conf.c | 3 +++
src/libxl/libxl_conf.h | 2 ++
src/libxl/libxl_driver.c | 7 +++++++
3 files changed, 12 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4cefadf..13eddd1 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -97,6 +97,7 @@ libxlDriverConfigDispose(void *obj)
VIR_FREE(cfg->stateDir);
VIR_FREE(cfg->libDir);
VIR_FREE(cfg->saveDir);
+ VIR_FREE(cfg->autoDumpDir);
}
static int
@@ -1078,6 +1079,8 @@ libxlDriverConfigNew(void)
goto error;
if (VIR_STRDUP(cfg->saveDir, LIBXL_SAVE_DIR) < 0)
goto error;
+ if (VIR_STRDUP(cfg->autoDumpDir, LIBXL_DUMP_DIR) < 0)
+ goto error;
if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) <
0)
goto error;
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index ca7bc7d..f089a92 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -48,6 +48,7 @@
# define LIBXL_LOG_DIR LOCALSTATEDIR "/log/libvirt/libxl"
# define LIBXL_LIB_DIR LOCALSTATEDIR "/lib/libvirt/libxl"
# define LIBXL_SAVE_DIR LIBXL_LIB_DIR "/save"
+# define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
# define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
@@ -82,6 +83,7 @@ struct _libxlDriverConfig {
char *stateDir;
char *libDir;
char *saveDir;
+ char *autoDumpDir;
};
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e600de7..e18fead 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -976,6 +976,13 @@ libxlStateInitialize(bool privileged,
virStrerror(errno, ebuf, sizeof(ebuf)));
goto error;
}
+ if (virFileMakePath(cfg->autoDumpDir) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to create dump dir '%s': %s"),
+ cfg->autoDumpDir,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+ goto error;
+ }
/* read the host sysinfo */
libxl_driver->hostsysinfo = virSysinfoRead();
--
1.8.1.4
7:39 a.m.
New subject: [libvirt] [PATCH 3/4] libxl: add dump dir to libxlDriverConfig object
On 21.02.2014 00:02, Jim Fehlig wrote:
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
Without a libxl config file, I suppose the only thing needed here is
the #define in libxl_conf.h. But I have a dusty series adding lockd
support to the libxl driver, which I'll be cleaning off soon. That
series introduces libxl.conf, and can be expanded to include an auto
dump dir entry.
src/libxl/libxl_conf.c | 3 +++
src/libxl/libxl_conf.h | 2 ++
src/libxl/libxl_driver.c | 7 +++++++
3 files changed, 12 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4cefadf..13eddd1 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -97,6 +97,7 @@ libxlDriverConfigDispose(void *obj)
VIR_FREE(cfg->stateDir);
VIR_FREE(cfg->libDir);
VIR_FREE(cfg->saveDir);
+ VIR_FREE(cfg->autoDumpDir);
}
static int
@@ -1078,6 +1079,8 @@ libxlDriverConfigNew(void)
goto error;
if (VIR_STRDUP(cfg->saveDir, LIBXL_SAVE_DIR) < 0)
goto error;
+ if (VIR_STRDUP(cfg->autoDumpDir, LIBXL_DUMP_DIR) < 0)
+ goto error;
if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir)
< 0)
goto error;
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index ca7bc7d..f089a92 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -48,6 +48,7 @@
# define LIBXL_LOG_DIR LOCALSTATEDIR "/log/libvirt/libxl"
# define LIBXL_LIB_DIR LOCALSTATEDIR "/lib/libvirt/libxl"
# define LIBXL_SAVE_DIR LIBXL_LIB_DIR "/save"
+# define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
# define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
@@ -82,6 +83,7 @@ struct _libxlDriverConfig {
char *stateDir;
char *libDir;
char *saveDir;
+ char *autoDumpDir;
};
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e600de7..e18fead 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -976,6 +976,13 @@ libxlStateInitialize(bool privileged,
virStrerror(errno, ebuf, sizeof(ebuf)));
goto error;
}
+ if (virFileMakePath(cfg->autoDumpDir) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to create dump dir '%s': %s"),
+ cfg->autoDumpDir,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+ goto error;
+ }
/* read the host sysinfo */
libxl_driver->hostsysinfo = virSysinfoRead();
ACK
Michal
5:02 p.m.
New subject: [libvirt] [PATCH 4/4] libxl: handle on_crash coredump actions
Add support for coredump-{destroy,restart} actions of <on_crash> event.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e18fead..7640801 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -251,6 +251,50 @@ error:
}
/*
+ * Core dump domain to default dump path.
+ *
+ * virDomainObjPtr should be locked on invocation
+ */
+static int
+libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver,
+ virDomainObjPtr vm)
+{
+ libxlDomainObjPrivatePtr priv = vm->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ time_t curtime = time(NULL);
+ char timestr[100];
+ struct tm time_info;
+ char *dumpfile = NULL;
+ int ret = -1;
+
+ localtime_r(&curtime, &time_info);
+ strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
+
+ if (virAsprintf(&dumpfile, "%s/%s-%s",
+ cfg->autoDumpDir,
+ vm->def->name,
+ timestr) < 0)
+ goto cleanup;
+
+ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ /* Unlock virDomainObj while dumping core */
+ virObjectUnlock(vm);
+ libxl_domain_core_dump(priv->ctx, vm->def->id, dumpfile, NULL);
+ virObjectLock(vm);
+
+ ignore_value(libxlDomainObjEndJob(driver, vm));
+ ret = 0;
+
+cleanup:
+ VIR_FREE(dumpfile);
+ virObjectUnref(cfg);
+
+ return ret;
+}
+
+/*
* Cleanup function for domain that has reached shutoff state.
*
* virDomainObjPtr should be locked on invocation
@@ -403,6 +447,12 @@ libxlDomainShutdownThread(void *opaque)
goto restart;
case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE:
goto cleanup;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY:
+ libxlDomainAutoCoreDump(driver, vm);
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_RESTART:
+ libxlDomainAutoCoreDump(driver, vm);
+ goto restart;
}
} else if (xl_reason == LIBXL_SHUTDOWN_REASON_REBOOT) {
dom_event = virDomainEventLifecycleNewFromObj(vm,
--
1.8.1.4
7:39 a.m.
New subject: [libvirt] [PATCH 4/4] libxl: handle on_crash coredump actions
On 21.02.2014 00:02, Jim Fehlig wrote:
Add support for coredump-{destroy,restart} actions of
<on_crash> event.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e18fead..7640801 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -251,6 +251,50 @@ error:
}
/*
+ * Core dump domain to default dump path.
+ *
+ * virDomainObjPtr should be locked on invocation
+ */
+static int
+libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver,
+ virDomainObjPtr vm)
+{
+ libxlDomainObjPrivatePtr priv = vm->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ time_t curtime = time(NULL);
+ char timestr[100];
+ struct tm time_info;
+ char *dumpfile = NULL;
+ int ret = -1;
+
+ localtime_r(&curtime, &time_info);
+ strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
+
+ if (virAsprintf(&dumpfile, "%s/%s-%s",
+ cfg->autoDumpDir,
+ vm->def->name,
+ timestr) < 0)
+ goto cleanup;
+
+ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ /* Unlock virDomainObj while dumping core */
+ virObjectUnlock(vm);
+ libxl_domain_core_dump(priv->ctx, vm->def->id, dumpfile, NULL);
+ virObjectLock(vm);
+
+ ignore_value(libxlDomainObjEndJob(driver, vm));
+ ret = 0;
+
+cleanup:
+ VIR_FREE(dumpfile);
+ virObjectUnref(cfg);
+
+ return ret;
+}
+
+/*
* Cleanup function for domain that has reached shutoff state.
*
* virDomainObjPtr should be locked on invocation
@@ -403,6 +447,12 @@ libxlDomainShutdownThread(void *opaque)
goto restart;
case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE:
goto cleanup;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY:
+ libxlDomainAutoCoreDump(driver, vm);
+ goto destroy;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_RESTART:
+ libxlDomainAutoCoreDump(driver, vm);
+ goto restart;
}
} else if (xl_reason == LIBXL_SHUTDOWN_REASON_REBOOT) {
dom_event = virDomainEventLifecycleNewFromObj(vm,
ACK
Michal
3924
days inactive
3928
days old
14 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Jim Fehlig -
Michal Privoznik