xl: handle domain shutdown/reboot/crash in a user configurable way
PATCH 1/1 xl: handle domain shutdown/reboot/crash in a user configurable way
Add support for on_poweroff, on_reboot and on_crash configuration items. In addition to these options from xend also add on_watchdog.
PATCH 1/1 libxl: Add LIBXL_EVENT namespace to enum libxl_event_type
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1280140562 -3600
# Node ID 7a0c37f2a9b51ac012a32bcf6ab222580e7cac94 # Parent 74c8b1f2c2da28fe00492add40a32a257e44dca7
libxl: Add LIBXL_EVENT namespace to enum libxl_event_type
tools/libxl/libxl.c +4 -4
@@ -643,7 +643,7 @@ int libxl_wait_for_domain_death(struct lint libxl_wait_for_domain_death (struct libxl_ctx *ctx,uint32_t domid,libxl_waiter *waiter) { waiter->path =strdup ("@releaseDomain" ); -if (asprintf (&(waiter->token),"%d" ,DOMAIN_DEATH ) <0 ) +if (asprintf (&(waiter->token),"%d" ,LIBXL_EVENT_DOMAIN_DEATH ) <0 )return -1 ;if (!xs_watch (ctx->xsh, waiter->path, waiter->token))return -1 ;
@@ -663,7 +663,7 @@ int libxl_wait_for_disk_ejects(struct li libxl_xs_get_dompath(ctx, domid), device_disk_dev_number(disks[i].virtpath)) < 0)return -1 ; -if (asprintf (&(waiter[i].token),"%d" ,DISK_EJECT ) <0 ) +if (asprintf (&(waiter[i].token),"%d" ,LIBXL_EVENT_DISK_EJECT ) <0 )return -1 ;xs_watch (ctx->xsh, waiter->path, waiter->token); }
@@ -711,7 +711,7 @@ int libxl_event_get_domain_death_info(st {int rc =0 , ret; -if (event && event->type ==DOMAIN_DEATH ) { +if (event && event->type ==LIBXL_EVENT_DOMAIN_DEATH ) { ret =xc_domain_getinfolist (ctx->xch, domid,1 , info);if (ret ==1 && info->domain == domid) { if (info->flags & XEN_DOMINF_running ||
@@ -730,7 +730,7 @@ out:int libxl_event_get_disk_eject_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,libxl_device_disk *disk) { -if (event && event->type ==DISK_EJECT ) { +if (event && event->type ==LIBXL_EVENT_DISK_EJECT ) {char *path;char *backend;char *value =libxl_xs_read (ctx,XBT_NULL , event->path);
tools/libxl/libxl.h +2 -2
@@ -357,8 +357,8 @@ char *libxl_uuid2string(struct libxl_ctx/* events handling */ typedef enum { -DOMAIN_DEATH , -DISK_EJECT , +LIBXL_EVENT_DOMAIN_DEATH , +LIBXL_EVENT_DISK_EJECT , }libxl_event_type ; typedef struct {
tools/libxl/xl_cmdimpl.c +2 -2
@@ -1348,7 +1348,7 @@ start:continue ;libxl_get_event (&ctx, &event);switch (event.type) { -case DOMAIN_DEATH : +case LIBXL_EVENT_DOMAIN_DEATH :if (libxl_event_get_domain_death_info (&ctx, domid, &event, &info)) {LOG ("Domain %d is dead" , domid);if (info.flags & XEN_DOMINF_dying || (info.flags & XEN_DOMINF_shutdown && (((info.flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask) != SHUTDOWN_suspend))) {
@@ -1374,7 +1374,7 @@ start:exit (0 ); }break ; -case DISK_EJECT : +case LIBXL_EVENT_DISK_EJECT :if (libxl_event_get_disk_eject_info (&ctx, domid, &event, &disk))libxl_cdrom_insert (&ctx, domid, &disk);break ;
PATCH 1/1 libxl: return libxl_dominfo from libxl_event_get_domain_death_info
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1280140562 -3600
# Node ID f6300d42a667cf6a1a02fc065ecd9eaea0e10ecc
# Parent 7a0c37f2a9b51ac012a32bcf6ab222580e7cac94
libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Removes a libxc data type from the libxl interface.
tools/libxl/libxl.c +27 -14
@@ -405,8 +405,6 @@ static void xcinfo2xlinfo(const xc_domaistatic void xcinfo2xlinfo (const xc_domaininfo_t *xcinfo,struct libxl_dominfo *xlinfo) { -unsigned int shutdown_reason; -memcpy (&(xlinfo->uuid), xcinfo->handle,sizeof (xen_domain_handle_t)); xlinfo->domid = xcinfo->domain;
@@ -416,12 +414,28 @@ static void xcinfo2xlinfo(const xc_domai xlinfo->blocked = !!(xcinfo->flags&XEN_DOMINF_blocked); xlinfo->running = !!(xcinfo->flags&XEN_DOMINF_running); xlinfo->crashed =0 ; + xlinfo->shutdown_reason =-1 ; - shutdown_reason = (xcinfo->flags>>XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask; - -if ( xlinfo->shutdown && (shutdown_reason == SHUTDOWN_crash) ) { - xlinfo->shutdown =0 ; - xlinfo->crashed =1 ; +if (xlinfo->shutdown) { +switch ((xcinfo->flags>>XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask) { +case SHUTDOWN_poweroff: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_POWEROFF ; +break ; +case SHUTDOWN_reboot: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_REBOOT ; +break ; +case SHUTDOWN_suspend: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_SUSPEND ; +break ; +case SHUTDOWN_crash: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_CRASH ; + xlinfo->shutdown =0 ; + xlinfo->crashed =1 ; +break ; +case SHUTDOWN_watchdog: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_WATCHDOG ; +break ; + } } xlinfo->max_memkb =PAGE_TO_MEMKB (xcinfo->tot_pages);
@@ -707,20 +721,19 @@ int libxl_free_waiter(libxl_waiter *waitreturn 0 ; } -int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,xc_domaininfo_t *info) +int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,struct libxl_dominfo *info) {int rc =0 , ret; +if (event && event->type ==LIBXL_EVENT_DOMAIN_DEATH ) { + ret =libxl_domain_info (ctx, info, domid); -if (event && event->type ==LIBXL_EVENT_DOMAIN_DEATH ) { - ret =xc_domain_getinfolist (ctx->xch, domid,1 , info); -if (ret ==1 && info->domain == domid) { -if (info->flags & XEN_DOMINF_running || - (!(info->flags & XEN_DOMINF_shutdown) && !(info->flags & XEN_DOMINF_dying))) +if (ret ==0 && info->domid == domid) { +if (info->running || (!info->shutdown && !info->dying && !info->crashed)) goto out; rc =1 ; goto out; } -memset (info,0 ,sizeof (xc_dominfo_t)); +memset (info,0 ,sizeof (*info)); rc =1 ; goto out; }
tools/libxl/libxl.h +11 -1
@@ -22,6 +22,14 @@#include <xs.h> #include <sys/wait.h> /* for pid_t */ +enum libxl_shutdown_reason { +LIBXL_SHUTDOWN_POWEROFF ,/* Domain exited normally. Clean up and kill. */ +LIBXL_SHUTDOWN_REBOOT ,/* Clean up, kill, and then restart. */ +LIBXL_SHUTDOWN_SUSPEND ,/* Clean up, save suspend info, kill. */ +LIBXL_SHUTDOWN_CRASH ,/* Tell controller we've crashed. */ +LIBXL_SHUTDOWN_WATCHDOG ,/* Restart because watchdog time expired. */ +}; +struct libxl_dominfo {uint8_t uuid[16 ];uint32_t domid;
@@ -31,6 +39,8 @@ struct libxl_dominfo { uint8_t shutdown:1; uint8_t crashed:1; uint8_t dying:1; +enum libxl_shutdown_reason shutdown_reason; +uint64_t max_memkb;uint64_t cpu_time;uint32_t vcpu_max_id;
@@ -385,7 +395,7 @@ int libxl_free_event(libxl_event *event)int libxl_free_event (libxl_event *event);int libxl_free_waiter (libxl_waiter *waiter); -int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,xc_domaininfo_t *info); +int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,struct libxl_dominfo *info);int libxl_event_get_disk_eject_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,libxl_device_disk *disk); int libxl_domain_rename(struct libxl_ctx *ctx, uint32_t domid,
tools/libxl/xl_cmdimpl.c +3 -4
@@ -1335,7 +1335,7 @@ start:while (1 ) {int ret;fd_set rfds; -xc_domaininfo_t info; +struct libxl_dominfo info;libxl_event event;libxl_device_disk disk;memset (&info,0x00 ,sizeof (xc_domaininfo_t));
@@ -1351,11 +1351,10 @@ start:case LIBXL_EVENT_DOMAIN_DEATH :if (libxl_event_get_domain_death_info (&ctx, domid, &event, &info)) {LOG ("Domain %d is dead" , domid); -if (info.flags & XEN_DOMINF_dying || (info.flags & XEN_DOMINF_shutdown && (((info.flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask) != SHUTDOWN_suspend))) { +if (info.crashed || info.dying || (info.shutdown && info.shutdown_reason != SHUTDOWN_suspend)) {LOG ("Domain %d needs to be clean: destroying the domain" , domid);libxl_domain_destroy (&ctx, domid,0 ); -if (info.flags & XEN_DOMINF_shutdown && - (((info.flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask) == SHUTDOWN_reboot)) { +if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) {libxl_free_waiter (w1);libxl_free_waiter (w2);free (w1);
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Quoted 40 lines · Ian Campbell
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1280140562 -3600
# Node IDf6300d42a667cf6a1a02fc065ecd9eaea0e10ecc
# Parent7a0c37f2a9b51ac012a32bcf6ab222580e7cac94
libxl: returnlibxl_dominfofromlibxl_event_get_domain_death_infoRemoves a libxc data type from the libxl interface.
Signed-off-by Ian Campbell <[email protected]>diff -r
7a0c37f2a9b5-rf6300d42a667tools/libxl/libxl.ctools/libxl/libxl.c +6
-7@@ -405,8 +405,6 @@ static void xcinfo2xlinfo(const xc_domaistatic void xcinfo2xlinfo (const xc_domaininfo_t *xcinfo,struct libxl_dominfo *xlinfo) { -unsigned int shutdown_reason; -memcpy (&(xlinfo->uuid), xcinfo->handle,sizeof (xen_domain_handle_t)); xlinfo->domid = xcinfo->domain;@@ -416,12 +414,28 @@ static void xcinfo2xlinfo(const xc_domai xlinfo->blocked = !!(xcinfo->flags&XEN_DOMINF_blocked); xlinfo->running = !!(xcinfo->flags&XEN_DOMINF_running); xlinfo->crashed =0 ; + xlinfo->shutdown_reason =-1 ; - shutdown_reason = (xcinfo->flags>>XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask; - -if ( xlinfo->shutdown && (shutdown_reason == SHUTDOWN_crash) ) { - xlinfo->shutdown =0 ; - xlinfo->crashed =1 ; +if (xlinfo->shutdown) { +switch ((xcinfo->flags>>XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask) { +case SHUTDOWN_poweroff: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_POWEROFF ; +break ;
tools/libxl/libxl.c static void xcinfo2xlinfo(const xc_domai · line 433
Quoted 10 lines
tools/libxl/libxl.c +10
-0+case SHUTDOWN_reboot: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_REBOOT ; +break ; +case SHUTDOWN_suspend: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_SUSPEND ; +break ; +case SHUTDOWN_crash: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_CRASH ; + xlinfo->shutdown =0 ; + xlinfo->crashed =1 ;
do we still need xlinfo->crashed, now that we have xlinfo->shutdown_reason?
Quoted 84 lines
tools/libxl/libxl.c +11
-7+break ; +case SHUTDOWN_watchdog: + xlinfo->shutdown_reason =LIBXL_SHUTDOWN_WATCHDOG ; +break ; + } } xlinfo->max_memkb =PAGE_TO_MEMKB (xcinfo->tot_pages);@@ -707,20 +721,19 @@ int libxl_free_waiter(libxl_waiter *waitreturn 0 ; } -int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,xc_domaininfo_t *info) +int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,struct libxl_dominfo *info) {int rc =0 , ret; +if (event && event->type ==LIBXL_EVENT_DOMAIN_DEATH ) { + ret =libxl_domain_info (ctx, info, domid); -if (event && event->type ==LIBXL_EVENT_DOMAIN_DEATH ) { - ret =xc_domain_getinfolist (ctx->xch, domid,1 , info); -if (ret ==1 && info->domain == domid) { -if (info->flags & XEN_DOMINF_running || - (!(info->flags & XEN_DOMINF_shutdown) && !(info->flags & XEN_DOMINF_dying))) +if (ret ==0 && info->domid == domid) { +if (info->running || (!info->shutdown && !info->dying && !info->crashed)) goto out; rc =1 ; goto out; } -memset (info,0 ,sizeof (xc_dominfo_t)); +memset (info,0 ,sizeof (*info)); rc =1 ; goto out; }diff -r
7a0c37f2a9b5-rf6300d42a667tools/libxl/libxl.htools/libxl/libxl.h +11
-1@@ -22,6 +22,14 @@#include <xs.h> #include <sys/wait.h> /* for pid_t */ +enum libxl_shutdown_reason { +LIBXL_SHUTDOWN_POWEROFF ,/* Domain exited normally. Clean up and kill. */ +LIBXL_SHUTDOWN_REBOOT ,/* Clean up, kill, and then restart. */ +LIBXL_SHUTDOWN_SUSPEND ,/* Clean up, save suspend info, kill. */ +LIBXL_SHUTDOWN_CRASH ,/* Tell controller we've crashed. */ +LIBXL_SHUTDOWN_WATCHDOG ,/* Restart because watchdog time expired. */ +}; +struct libxl_dominfo {uint8_t uuid[16 ];uint32_t domid;@@ -31,6 +39,8 @@ struct libxl_dominfo { uint8_t shutdown:1; uint8_t crashed:1; uint8_t dying:1; +enum libxl_shutdown_reason shutdown_reason; +uint64_t max_memkb;uint64_t cpu_time;uint32_t vcpu_max_id;@@ -385,7 +395,7 @@ int libxl_free_event(libxl_event *event)int libxl_free_event (libxl_event *event);int libxl_free_waiter (libxl_waiter *waiter); -int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,xc_domaininfo_t *info); +int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,struct libxl_dominfo *info);int libxl_event_get_disk_eject_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,libxl_device_disk *disk); int libxl_domain_rename(struct libxl_ctx *ctx, uint32_t domid,diff -r
7a0c37f2a9b5-rf6300d42a667tools/libxl/xl_cmdimpl.ctools/libxl/xl_cmdimpl.c +1
-1@@ -1335,7 +1335,7 @@ start:while (1 ) {int ret;fd_set rfds; -xc_domaininfo_t info; +struct libxl_dominfo info;libxl_event event;libxl_device_disk disk;memset (&info,0x00 ,sizeof (xc_domaininfo_t));
tools/libxl/xl_cmdimpl.c start: · line 1354
Quoted 5 lines
tools/libxl/xl_cmdimpl.c +1
-1@@ -1351,11 +1351,10 @@ start:case LIBXL_EVENT_DOMAIN_DEATH :if (libxl_event_get_domain_death_info (&ctx, domid, &event, &info)) {LOG ("Domain %d is dead" , domid); -if (info.flags & XEN_DOMINF_dying || (info.flags & XEN_DOMINF_shutdown && (((info.flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask) != SHUTDOWN_suspend))) { +if (info.crashed || info.dying || (info.shutdown && info.shutdown_reason != SHUTDOWN_suspend)) {
shouldn't this be LIBXL_SHUTDOWN_SUSPEND
tools/libxl/xl_cmdimpl.c start: · line 1357
Quoted 5 lines
tools/libxl/xl_cmdimpl.c +1
-2LOG ("Domain %d needs to be clean: destroying the domain" , domid);libxl_domain_destroy (&ctx, domid,0 ); -if (info.flags & XEN_DOMINF_shutdown && - (((info.flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask) == SHUTDOWN_reboot)) { +if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) {
shouldn't this be LIBXL_SHUTDOWN_REBOOT?
It seems that both SHUDOWN_ are removed in following patches anyway, in that case it is Ok I guess.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Quoted 3 lines · Stefano Stabellini
On Mon, 26 Jul 2010, Ian Campbell wrote:
do we still needxlinfo->crashed, now that we havexlinfo->shutdown_reason?
I guess we could get rid of it, the only other place it is used (AFAIK) is in the "xl list" output I sent the other day.
Quoted 1 line
shouldn't this be
LIBXL_SHUTDOWN_SUSPEND
[...]
Quoted 1 line
shouldn't this be
LIBXL_SHUTDOWN_REBOOT?
Yes in both cases.
Quoted 2 lines
It seems that both
SHUDOWN_are removed in following patches anyway, in that case it is Ok I guess.
Phew!
I can regenerate the series if necessary though.
Ian.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Quoted 7 lines · Ian Campbell
Quoted 3 lines · Stefano Stabellini
On Mon, 26 Jul 2010, Ian Campbell wrote:
do we still needxlinfo->crashed, now that we havexlinfo->shutdown_reason?I guess we could get rid of it, the only other place it is used (AFAIK) is in the "xl list" output I sent the other day.
All right let's do that.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
In reply to Ian CampbellIan Campbell writes ("[Xen-devel] [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info"):
Quoted 2 lines
libxl: return
libxl_dominfofromlibxl_event_get_domain_death_infoRemoves a libxc data type from the libxl interface.
I don't think this is the right approach, mainly because shutdown reasons aren't a libxc datatype, but a Xen one. My view is that libxl should hide libxc, but that it is allowed to expose Xen.
So libxl callers are allowed to #include xen/include/public/sched.h.
Ian.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Quoted 9 lines · Ian Jackson
Ian Campbell writes ("[Xen-devel] [PATCH 2 of 9] libxl: return
libxl_dominfofromlibxl_event_get_domain_death_info"):Quoted 2 lines
libxl: return
libxl_dominfofromlibxl_event_get_domain_death_infoRemoves a libxc data type from the libxl interface.I don't think this is the right approach, mainly because shutdown reasons aren't a libxc datatype, but a Xen one. My view is that libxl should hide libxc, but that it is allowed to expose Xen.
So libxl callers are allowed to #include
xen/include/public/sched.h.
I'm happy with exposing Xen datatypes if that is the approach we want to take but not with returning an xc_domaininfo_t.
I think the right compromise is therefore to declare that libxl_dominfo.shutdown_reason contains the values defined in xen/.../sched.h rather than a new libxl enum and retain the switch to returning libxl_dominfo.
(Maybe that's what you meant anyhow)
Ian.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Ian Campbell writes ("Re: [Xen-devel] [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info"):
Quoted 2 lines
I'm happy with exposing Xen datatypes if that is the approach we want to take but not with returning an
xc_domaininfo_t.
Yes.
Quoted 4 lines
I think the right compromise is therefore to declare that libxl_dominfo.shutdown_reason contains the values defined in
xen/.../sched.hrather than a new libxl enum and retain the switch to returninglibxl_dominfo.
Yes, exactly.
Ian.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
In reply to Ian JacksonQuoted 10 lines · Ian Jackson
Ian Campbell writes ("[Xen-devel] [PATCH 2 of 9] libxl: return
libxl_dominfofromlibxl_event_get_domain_death_info"):Quoted 2 lines
libxl: return
libxl_dominfofromlibxl_event_get_domain_death_infoRemoves a libxc data type from the libxl interface.I don't think this is the right approach, mainly because shutdown reasons aren't a libxc datatype, but a Xen one. My view is that libxl should hide libxc, but that it is allowed to expose Xen.
So libxl callers are allowed to #include
xen/include/public/sched.h.
if libxl should hide libxc then allowing clients to include a Xen header is an even worse layering violation.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info"):
Quoted 2 lines
if libxl should hide libxc then allowing clients to include a Xen header is an even worse layering violation.
I don't think so. The purpose of the doctrine that libxl callers should not need to call libxc is so that libxl is complete. And anything which libxc exports is contingent.
I certainly think that libxl callers should not make Xen hypercalls, need to know SCHEDOP numbers, etc. etc. But I think it is OK for libxl to expose it its callers anything that forms part of the abstract interface to Xen guests. So that includes:
* shutdown reasons
* the difference between various kinds of HV and PV block devices (hda, xvda, sda, ...), including the facility to specify the actual xenstore interface combined PV block device number
* the fact that a guest may have multiple consoles (PV and emulated serial) - even if the current toolstack implementation does not support all possible combinations
The concrete interface (event channels, rings, frame numbers, etc.) should remain opaque but there is no point abstracting away and translating the numerical values of a Xen public enum whose semantics we _do_ want to expose.
Ian.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Quoted 20 lines · Ian Jackson
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 2 of 9] libxl: return
libxl_dominfofromlibxl_event_get_domain_death_info"):Quoted 2 lines
if libxl should hide libxc then allowing clients to include a Xen header is an even worse layering violation.
I don't think so. The purpose of the doctrine that libxl callers should not need to call libxc is so that libxl is complete. And anything which libxc exports is contingent.
I certainly think that libxl callers should not make Xen hypercalls, need to know SCHEDOP numbers, etc. etc. But I think it is OK for libxl to expose it its callers anything that forms part of the abstract interface to Xen guests. So that includes:
* shutdown reasons
* the difference between various kinds of HV and PV block devices (hda, xvda, sda, ...), including the facility to specify the actual xenstore interface combined PV block device number
* the fact that a guest may have multiple consoles (PV and emulated serial) - even if the current toolstack implementation does not support all possible combinations
Quoted 5 lines
The concrete interface (event channels, rings, frame numbers, etc.) should remain opaque but there is no point abstracting away and translating the numerical values of a Xen public enum whose semantics we _do_ want to expose.
enum alone would OK, but most header files contains other stuff as well, so including features.h would be fine but including platform.h would not. In order to have a clear policy we should just say that including Xen header files should be avoided.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info"):
Quoted 4 lines
enum alone would OK, but most header files contains other stuff as well, so including
features.hwould be fine but includingplatform.hwould not. In order to have a clear policy we should just say that including Xen header files should be avoided.
I don't think there is any difficulty with libxl callers #including Xen public header files, no matter that those Xen header files have lots of other stuff in that is not relevant to them.
It's not as if #including the Xen public headers exposes functions for making hypercalls, or indeed any functions or objects at all.
Ian.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
Quoted 6 lines · Ian Jackson
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 2 of 9] libxl: return
libxl_dominfofromlibxl_event_get_domain_death_info"):Quoted 4 lines
enum alone would OK, but most header files contains other stuff as well, so including
features.hwould be fine but includingplatform.hwould not. In order to have a clear policy we should just say that including Xen header files should be avoided.
Quoted 6 lines
I don't think there is any difficulty with libxl callers #including Xen public header files, no matter that those Xen header files have lots of other stuff in that is not relevant to them.
It's not as if #including the Xen public headers exposes functions for making hypercalls, or indeed any functions or objects at all.
In my repost of the series I will use the SHUTDOWN_* values from sched.h. We can always update it if consensus goes the other way.
Ian.
Re: [PATCH 2 of 9] libxl: return libxl_dominfo from libxl_event_get_domain_death_info
In reply to Ian JacksonQuoted 6 lines · Ian Jackson
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 2 of 9] libxl: return
libxl_dominfofromlibxl_event_get_domain_death_info"):Quoted 4 lines
enum alone would OK, but most header files contains other stuff as well, so including
features.hwould be fine but includingplatform.hwould not. In order to have a clear policy we should just say that including Xen header files should be avoided.
Quoted 7 lines
I don't think there is any difficulty with libxl callers #including Xen public header files, no matter that those Xen header files have lots of other stuff in that is not relevant to them.
It's not as if #including the Xen public headers exposes functions for making hypercalls, or indeed any functions or objects at all.
No, but they include the shared_info page struct, the start_info page struct and hypercall numbers and arguments, that is as close as you can get.
I still think that callers shouldn't be required to include xen header files.
PATCH 1/1 libxl: signal caller if domain already destroyed on domain death event
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1280140562 -3600
# Node ID 1e0b63948031587b958ea307410d19c7b2be9614
# Parent f6300d42a667cf6a1a02fc065ecd9eaea0e10ecc
libxl: signal caller if domain already destroyed on domain death event
Currently libxl_event_get_domain_death_info returns 0 if the event was not a domain death event and 1 if it was but does not infom the user if someone else has already cleaned up the domain, which means the caller must replicate some of the logic from within libxl.
Instead have the libxl_event_get_XXX_info functions required that the event is of the right type (the caller must have recently switched on event->type anyway).
This allows the return codes to be used in an event specific way and we take advantage of this by returning an error from libxl_event_get_domain_death_info if the domain is not dying.
tools/libxl/libxl.c +37 -37
@@ -721,54 +721,54 @@ int libxl_free_waiter(libxl_waiter *waitreturn 0 ; } +/* +* Returns: +* - 0 if the domain is dead and there is no cleanup to be done. e.g because someone else has already done it. +* - 1 if the domain is dead and there is cleanup to be done. +* +* Can return error if the domain exists and is still running +*/ int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,struct libxl_dominfo *info) { -int rc =0 , ret; -if (event && event->type ==LIBXL_EVENT_DOMAIN_DEATH ) { - ret =libxl_domain_info (ctx, info, domid); +if (libxl_domain_info (ctx, info, domid) <0 ) +return 0 ; -if (ret ==0 && info->domid == domid) { -if (info->running || (!info->shutdown && !info->dying && !info->crashed)) - goto out; - rc =1 ; - goto out; - } -memset (info,0 ,sizeof (*info)); - rc =1 ; - goto out; - } -out: -return rc; +if (info->running || (!info->shutdown && !info->dying && !info->crashed)) +return ERROR_INVAL ; + +return 1 ; } +/* +* Returns true and fills *disk if the caller should eject the disk +*/ int libxl_event_get_disk_eject_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,libxl_device_disk *disk) { -if (event && event->type ==LIBXL_EVENT_DISK_EJECT ) { -char *path; -char *backend; -char *value =libxl_xs_read (ctx,XBT_NULL , event->path); +char *path; +char *backend; +char *value; -if (!value ||strcmp (value,"eject" )) -return 0 ; + value =libxl_xs_read (ctx,XBT_NULL , event->path); - path =strdup (event->path); - path[strlen (path) -6 ] ='\0' ; - backend =libxl_xs_read (ctx,XBT_NULL ,libxl_sprintf (ctx,"%s/backend" , path)); +if (!value ||strcmp (value,"eject" )) +return 0 ; - disk->backend_domid =0 ; - disk->domid = domid; - disk->physpath =NULL ; - disk->phystype =0 ; -/* this value is returned to the user: do not free right away */ - disk->virtpath =libxl_xs_read (ctx,XBT_NULL ,libxl_sprintf (ctx,"%s/dev" , backend)); - disk->unpluggable =1 ; - disk->readwrite =0 ; - disk->is_cdrom =1 ; + path =strdup (event->path); + path[strlen (path) -6 ] ='\0' ; + backend =libxl_xs_read (ctx,XBT_NULL ,libxl_sprintf (ctx,"%s/backend" , path)); -free (path); -return 1 ; - } -return 0 ; + disk->backend_domid =0 ; + disk->domid = domid; + disk->physpath =NULL ; + disk->phystype =0 ; +/* this value is returned to the user: do not free right away */ + disk->virtpath =libxl_xs_read (ctx,XBT_NULL ,libxl_sprintf (ctx,"%s/dev" , backend)); + disk->unpluggable =1 ; + disk->readwrite =0 ; + disk->is_cdrom =1 ; + +free (path); +return 1 ; }static int libxl_destroy_device_model (struct libxl_ctx *ctx,uint32_t domid)
tools/libxl/xl_cmdimpl.c +9 -5
@@ -1338,7 +1338,6 @@ start:struct libxl_dominfo info;libxl_event event;libxl_device_disk disk; -memset (&info,0x00 ,sizeof (xc_domaininfo_t));FD_ZERO (&rfds);FD_SET (fd, &rfds);
@@ -1349,12 +1348,17 @@ start:libxl_get_event (&ctx, &event);switch (event.type) {case LIBXL_EVENT_DOMAIN_DEATH : -if (libxl_event_get_domain_death_info (&ctx, domid, &event, &info)) { -LOG ("Domain %d is dead" , domid); -if (info.crashed || info.dying || (info.shutdown && info.shutdown_reason != SHUTDOWN_suspend)) { + ret =libxl_event_get_domain_death_info (&ctx, domid, &event, &info); + +if (ret <0 )continue ; + +LOG ("Domain %d is dead" , domid); + +if (ret) { +if (info.shutdown_reason != SHUTDOWN_suspend) {LOG ("Domain %d needs to be clean: destroying the domain" , domid);libxl_domain_destroy (&ctx, domid,0 ); -if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) { +if (info.shutdown_reason == SHUTDOWN_reboot) {libxl_free_waiter (w1);libxl_free_waiter (w2);free (w1);
Re: [PATCH 3 of 9] libxl: signal caller if domain already destroyed on domain death event
Quoted 137 lines · Ian Campbell
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1280140562 -3600
# Node ID1e0b63948031587b958ea307410d19c7b2be9614
# Parentf6300d42a667cf6a1a02fc065ecd9eaea0e10ecc
libxl: signal caller if domain already destroyed on domain death eventCurrently
libxl_event_get_domain_death_inforeturns 0 if the event was not a domain death event and 1 if it was but does not infom the user if someone else has already cleaned up the domain, which means the caller must replicate some of the logic from within libxl.Instead have the
libxl_event_get_XXX_infofunctions required that the event is of the right type (the caller must have recently switched onevent->typeanyway).This allows the return codes to be used in an event specific way and we take advantage of this by returning an error from
libxl_event_get_domain_death_infoif the domain is not dying.Signed-off-by Ian Campbell <[email protected]>diff -r
f6300d42a667-r1e0b63948031tools/libxl/libxl.ctools/libxl/libxl.c +37
-37@@ -721,54 +721,54 @@ int libxl_free_waiter(libxl_waiter *waitreturn 0 ; } +/* +* Returns: +* - 0 if the domain is dead and there is no cleanup to be done. e.g because someone else has already done it. +* - 1 if the domain is dead and there is cleanup to be done. +* +* Can return error if the domain exists and is still running +*/ int libxl_event_get_domain_death_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,struct libxl_dominfo *info) { -int rc =0 , ret; -if (event && event->type ==LIBXL_EVENT_DOMAIN_DEATH ) { - ret =libxl_domain_info (ctx, info, domid); +if (libxl_domain_info (ctx, info, domid) <0 ) +return 0 ; -if (ret ==0 && info->domid == domid) { -if (info->running || (!info->shutdown && !info->dying && !info->crashed)) - goto out; - rc =1 ; - goto out; - } -memset (info,0 ,sizeof (*info)); - rc =1 ; - goto out; - } -out: -return rc; +if (info->running || (!info->shutdown && !info->dying && !info->crashed)) +return ERROR_INVAL ; + +return 1 ; } +/* +* Returns true and fills *disk if the caller should eject the disk +*/ int libxl_event_get_disk_eject_info (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,libxl_device_disk *disk) { -if (event && event->type ==LIBXL_EVENT_DISK_EJECT ) { -char *path; -char *backend; -char *value =libxl_xs_read (ctx,XBT_NULL , event->path); +char *path; +char *backend; +char *value; -if (!value ||strcmp (value,"eject" )) -return 0 ; + value =libxl_xs_read (ctx,XBT_NULL , event->path); - path =strdup (event->path); - path[strlen (path) -6 ] ='\0' ; - backend =libxl_xs_read (ctx,XBT_NULL ,libxl_sprintf (ctx,"%s/backend" , path)); +if (!value ||strcmp (value,"eject" )) +return 0 ; - disk->backend_domid =0 ; - disk->domid = domid; - disk->physpath =NULL ; - disk->phystype =0 ; -/* this value is returned to the user: do not free right away */ - disk->virtpath =libxl_xs_read (ctx,XBT_NULL ,libxl_sprintf (ctx,"%s/dev" , backend)); - disk->unpluggable =1 ; - disk->readwrite =0 ; - disk->is_cdrom =1 ; + path =strdup (event->path); + path[strlen (path) -6 ] ='\0' ; + backend =libxl_xs_read (ctx,XBT_NULL ,libxl_sprintf (ctx,"%s/backend" , path)); -free (path); -return 1 ; - } -return 0 ; + disk->backend_domid =0 ; + disk->domid = domid; + disk->physpath =NULL ; + disk->phystype =0 ; +/* this value is returned to the user: do not free right away */ + disk->virtpath =libxl_xs_read (ctx,XBT_NULL ,libxl_sprintf (ctx,"%s/dev" , backend)); + disk->unpluggable =1 ; + disk->readwrite =0 ; + disk->is_cdrom =1 ; + +free (path); +return 1 ; }static int libxl_destroy_device_model (struct libxl_ctx *ctx,uint32_t domid)diff -r
f6300d42a667-r1e0b63948031tools/libxl/xl_cmdimpl.ctools/libxl/xl_cmdimpl.c +2
-4@@ -1338,7 +1338,6 @@ start:struct libxl_dominfo info;libxl_event event;libxl_device_disk disk; -memset (&info,0x00 ,sizeof (xc_domaininfo_t));FD_ZERO (&rfds);FD_SET (fd, &rfds);@@ -1349,12 +1348,17 @@ start:libxl_get_event (&ctx, &event);switch (event.type) {case LIBXL_EVENT_DOMAIN_DEATH : -if (libxl_event_get_domain_death_info (&ctx, domid, &event, &info)) { -LOG ("Domain %d is dead" , domid); -if (info.crashed || info.dying || (info.shutdown && info.shutdown_reason != SHUTDOWN_suspend)) { + ret =libxl_event_get_domain_death_info (&ctx, domid, &event, &info); +
tools/libxl/xl_cmdimpl.c start: · line 1361
Quoted 10 lines
tools/libxl/xl_cmdimpl.c +7
-1+if (ret <0 )continue ; + +LOG ("Domain %d is dead" , domid); + +if (ret) { +if (info.shutdown_reason != SHUTDOWN_suspend) {LOG ("Domain %d needs to be clean: destroying the domain" , domid);libxl_domain_destroy (&ctx, domid,0 ); -if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) { +if (info.shutdown_reason == SHUTDOWN_reboot) {
Isn't still possible to get here and have info.shutdown == 0 (and even info.dying == 0, after reading the fourth patch)?
If so, the previous test is probably clearer.
The rest of the series looks fine.
Re: [PATCH 3 of 9] libxl: signal caller if domain already destroyed on domain death event
Quoted 10 lines · Stefano Stabellini
Quoted 3 lines
tools/libxl/xl_cmdimpl.c +3
-0+ ret =libxl_event_get_domain_death_info (&ctx, domid, &event, &info); + +if (ret <0 )continue ;[....]
tools/libxl/xl_cmdimpl.c start: · line 1361
Quoted 5 lines
tools/libxl/xl_cmdimpl.c +2
-1+if (info.shutdown_reason != SHUTDOWN_suspend) {LOG ("Domain %d needs to be clean: destroying the domain" , domid);libxl_domain_destroy (&ctx, domid,0 ); -if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) { +if (info.shutdown_reason == SHUTDOWN_reboot) {
Quoted 3 lines
Isn't still possible to get here and have info.shutdown == 0 (and even info.dying == 0, after reading the fourth patch)?
If so, the previous test is probably clearer.
Umm...
I think libxl_event_get_domain_death_info should have returned ERROR_INVAL in that case and we'd have take the earlier if (ret < 0) continue.
Quoted 1 line
The rest of the series looks fine.
Thanks.
Re: [PATCH 3 of 9] libxl: signal caller if domain already destroyed on domain death event
Quoted 15 lines · Ian Campbell
Quoted 10 lines · Stefano Stabellini
Quoted 3 lines
tools/libxl/xl_cmdimpl.c +3
-0+ ret =libxl_event_get_domain_death_info (&ctx, domid, &event, &info); + +if (ret <0 )continue ;[....]
tools/libxl/xl_cmdimpl.c start: · line 1361
Quoted 5 lines
tools/libxl/xl_cmdimpl.c +2
-1+if (info.shutdown_reason != SHUTDOWN_suspend) {LOG ("Domain %d needs to be clean: destroying the domain" , domid);libxl_domain_destroy (&ctx, domid,0 ); -if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) { +if (info.shutdown_reason == SHUTDOWN_reboot) {Quoted 3 lines
Isn't still possible to get here and have info.shutdown == 0 (and even info.dying == 0, after reading the fourth patch)?
If so, the previous test is probably clearer.
Quoted 5 lines
Umm...
I think
libxl_event_get_domain_death_infoshould have returnedERROR_INVALin that case and we'd have take the earlier if (ret < 0) continue.
I think we should at least write in a comment in libxl.h that shutdown_reason is valid when (shutdown || dying) and that when libxl_event_get_domain_death_info returns 1 shutdown_reason is always set.
Also I still like the old test more, just for clarity.
Re: [PATCH 3 of 9] libxl: signal caller if domain already destroyed on domain death event
Quoted 18 lines · Stefano Stabellini
Quoted 17 lines · Ian Campbell
Quoted 10 lines · Stefano Stabellini
Quoted 3 lines
tools/libxl/xl_cmdimpl.c +3
-0+ ret =libxl_event_get_domain_death_info (&ctx, domid, &event, &info); + +if (ret <0 )continue ;[....]
tools/libxl/xl_cmdimpl.c start: · line 1361
Quoted 5 lines
tools/libxl/xl_cmdimpl.c +2
-1+if (info.shutdown_reason != SHUTDOWN_suspend) {LOG ("Domain %d needs to be clean: destroying the domain" , domid);libxl_domain_destroy (&ctx, domid,0 ); -if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) { +if (info.shutdown_reason == SHUTDOWN_reboot) {Quoted 3 lines
Isn't still possible to get here and have info.shutdown == 0 (and even info.dying == 0, after reading the fourth patch)?
If so, the previous test is probably clearer.Umm...
Quoted 8 lines
Quoted 3 lines
I think
libxl_event_get_domain_death_infoshould have returnedERROR_INVALin that case and we'd have take the earlier if (ret < 0) continue.I think we should at least write in a comment in
libxl.hthatshutdown_reasonis valid when (shutdown || dying) and that whenlibxl_event_get_domain_death_inforeturns 1shutdown_reasonis always set.
Will do.
Quoted 1 line
Also I still like the old test more, just for clarity.
It would need to be "(info.shutdown||info.crashed||info.dying) || info.shutdown_reason ....." which duplicates the logic in libxl, which I wanted to get rid of (I think that was fully realised the next patch or so)
Ian.
Re: [PATCH 3 of 9] libxl: signal caller if domain already destroyed on domain death event
Quoted 28 lines · Ian Campbell
Quoted 18 lines · Stefano Stabellini
Quoted 17 lines · Ian Campbell
Quoted 10 lines · Stefano Stabellini
Quoted 3 lines
tools/libxl/xl_cmdimpl.c +3
-0+ ret =libxl_event_get_domain_death_info (&ctx, domid, &event, &info); + +if (ret <0 )continue ;[....]
tools/libxl/xl_cmdimpl.c start: · line 1361
Quoted 5 lines
tools/libxl/xl_cmdimpl.c +2
-1+if (info.shutdown_reason != SHUTDOWN_suspend) {LOG ("Domain %d needs to be clean: destroying the domain" , domid);libxl_domain_destroy (&ctx, domid,0 ); -if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) { +if (info.shutdown_reason == SHUTDOWN_reboot) {Quoted 3 lines
Isn't still possible to get here and have info.shutdown == 0 (and even info.dying == 0, after reading the fourth patch)?
If so, the previous test is probably clearer.Umm...
Quoted 8 lines
Quoted 3 lines
I think
libxl_event_get_domain_death_infoshould have returnedERROR_INVALin that case and we'd have take the earlier if (ret < 0) continue.I think we should at least write in a comment in
libxl.hthatshutdown_reasonis valid when (shutdown || dying) and that whenlibxl_event_get_domain_death_inforeturns 1shutdown_reasonis always set.
Quoted 9 lines
Will do.
Quoted 1 line
Also I still like the old test more, just for clarity.
It would need to be "(info.shutdown||info.crashed||info.dying) || info.shutdown_reason ....." which duplicates the logic in libxl, which I wanted to get rid of (I think that was fully realised the next patch or so)
I only meant the second test:
if ((info.shutdown || info.dying) && info.shutdown_reason == SHUTDOWN_reboot)
or we can set shutdown_reason to LIBXL_SHUTDOWN_INVAL by default and keep this test as it is.
I wouldn't want users to believe that shutdown_reason
is LIBXL_SHUTDOWN_POWEROFF just because is 0.
PATCH 1/1 libxl: should consider shutdown_reason for dying as well as shutdown domains
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1280140563 -3600
# Node ID 982bf44850029802b7395a03659bf440d191be2f
# Parent 1e0b63948031587b958ea307410d19c7b2be9614
libxl: should consider shutdown_reason for dying as well as shutdown domains.
A dying domain is one which has notified the hypervisor (with SCHEDOP_shutdown) and provided a reason code but which hasn't actually shutdown yet.
Therefore a domain which is dying will always have a valid shutdown_reason and therefore we should obey it.
Failure to do so means there is a race between domain cleanup in the hypervisor and the toolstack notification which can mean we may destroy a domain which actually wanted to be rebooted.
tools/libxl/libxl.c +2 -2
@@ -416,7 +416,7 @@ static void xcinfo2xlinfo(const xc_domai xlinfo->crashed =0 ; xlinfo->shutdown_reason =-1 ; -if (xlinfo->shutdown) { +if (xlinfo->dying || xlinfo->shutdown) {switch ((xcinfo->flags>>XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask) {case SHUTDOWN_poweroff: xlinfo->shutdown_reason =LIBXL_SHUTDOWN_POWEROFF ;
@@ -429,7 +429,7 @@ static void xcinfo2xlinfo(const xc_domaibreak ;case SHUTDOWN_crash: xlinfo->shutdown_reason =LIBXL_SHUTDOWN_CRASH ; - xlinfo->shutdown =0 ; + xlinfo->shutdown = xlinfo->dying =0 ; xlinfo->crashed =1 ;break ;case SHUTDOWN_watchdog:
PATCH 1/1 libxl: add libxl_domain_preserve
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1280140563 -3600
# Node ID 85b50d34d7cc8a70c083c0acdc4c2e7ead675029 # Parent 982bf44850029802b7395a03659bf440d191be2f
libxl: add libxl_domain_preserve
This method is intended to preserve an existing domain (for debugging purposes) in such a way that the domain can also be restarted.
There is likely to be more required to achieve this aim than the function currently does.
tools/libxl/libxl.c +55 -0
@@ -399,6 +399,61 @@ int libxl_domain_resume(struct libxl_ctx domid);return ERROR_FAIL ; } +return 0 ; +} + +/* +* Preserves a domain but rewrites xenstore etc to make it unique so +* that the domain can be restarted. +* +* Does not modify info so that it may be reused. +*/ +int libxl_domain_preserve (struct libxl_ctx *ctx,uint32_t domid, +libxl_domain_create_info *info,const char *name_suffix,uint8_t new_uuid[16 ]) +{ +struct xs_permissions roperm[2 ]; +xs_transaction_t t; +char *preserved_name; +char *uuid_string; +char *vm_path; +char *dom_path; + +int rc; + + preserved_name =libxl_sprintf (ctx,"%s%s" , info->name, name_suffix); +if (!preserved_name)return ERROR_NOMEM ; + + uuid_string =libxl_uuid2string (ctx, new_uuid); +if (!uuid_string)return ERROR_NOMEM ; + + dom_path =libxl_xs_get_dompath (ctx, domid); +if (!dom_path)return ERROR_FAIL ; + + vm_path =libxl_sprintf (ctx,"/vm/%s" , uuid_string); +if (!vm_path)return ERROR_FAIL ; + + roperm[0 ].id =0 ; + roperm[0 ].perms =XS_PERM_NONE ; + roperm[1 ].id = domid; + roperm[1 ].perms =XS_PERM_READ ; + + retry_transaction: + t =xs_transaction_start (ctx->xsh); + +xs_rm (ctx->xsh, t, vm_path); +xs_mkdir (ctx->xsh, t, vm_path); +xs_set_permissions (ctx->xsh, t, vm_path, roperm,ARRAY_SIZE (roperm)); + +xs_write (ctx->xsh, t,libxl_sprintf (ctx,"%s/vm" , dom_path), vm_path,strlen (vm_path)); + rc =libxl_domain_rename (ctx, domid, info->name, preserved_name, t); +if (rc)return rc; + +xs_write (ctx->xsh, t,libxl_sprintf (ctx,"%s/uuid" , vm_path), uuid_string,strlen (uuid_string)); + +if (!xs_transaction_end (ctx->xsh, t,0 )) +if (errno ==EAGAIN ) + goto retry_transaction; +return 0 ; }
tools/libxl/libxl.h +1 -0
@@ -340,6 +340,7 @@ int libxl_domain_resume(struct libxl_ctxint libxl_domain_resume (struct libxl_ctx *ctx,uint32_t domid);int libxl_domain_shutdown (struct libxl_ctx *ctx,uint32_t domid,int req);int libxl_domain_destroy (struct libxl_ctx *ctx,uint32_t domid,int force); +int libxl_domain_preserve (struct libxl_ctx *ctx,uint32_t domid,libxl_domain_create_info *info,const char *name_suffix,uint8_t new_uuid[16 ]);int libxl_file_reference_map (struct libxl_ctx *ctx,libxl_file_reference *f);int libxl_file_reference_unmap (struct libxl_ctx *ctx,libxl_file_reference *f);
Re: [PATCH 5 of 9] libxl: add libxl_domain_preserve
Ian Campbell writes ("[Xen-devel] [PATCH 5 of 9] libxl: add libxl_domain_preserve"):
Quoted 2 lines
This method is intended to preserve an existing domain (for debugging purposes) in such a way that the domain can also be restarted.
This sounds interesting, but I'm not quite sure what the semantics are intended to be. I guess it works on an existing running domain or a paused one. Does it stop the domain's execution ?
The current code seems mainly to change the name and the uuid in xenstore - to a specified new uuid - which seems a strange thing to do.
Ian.
Re: [PATCH 5 of 9] libxl: add libxl_domain_preserve
Quoted 7 lines · Ian Jackson
Ian Campbell writes ("[Xen-devel] [PATCH 5 of 9] libxl: add
libxl_domain_preserve"):Quoted 2 lines
This method is intended to preserve an existing domain (for debugging purposes) in such a way that the domain can also be restarted.
This sounds interesting, but I'm not quite sure what the semantics are intended to be. I guess it works on an existing running domain or a paused one. Does it stop the domain's execution ?
It was intended to be used to implement the rename-restart action for the on_reboot (and on_crash I guess) actions which I introduce in a later patch.
I only added this option because xend has it -- I'd be just as happy to only implement simple restart until someone who wants the behaviour comes along. In the meantime preserve or coredump-restart are pretty good substitutes for rename-restart.
Anyhow the actual actual semantics of libxl_domain_preserve is that it acts on a shutdown (but not destroyed) domain. The name and/or comments could certainly better reflect this.
Quoted 3 lines
The current code seems mainly to change the name and the uuid in xenstore - to a specified new uuid - which seems a strange thing to do.
It's (a subset of) what xend does ;-) (For reference its XendDomainInfo._preserveForRestart from tools/python/xen/xend/XendDomainInfo.py)
I suspect the reason is something to do with not nuking /vm/<uuid> for a running domain when destroying a previously preserved instance of that domain.
Ian.
PATCH 1/1 xl: do not try and auto re-connect console on reboot
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1280140563 -3600
# Node ID 333254f8e11bc7bfa6901fe5e5585abb25e31398 # Parent 85b50d34d7cc8a70c083c0acdc4c2e7ead675029
xl: do not try and auto re-connect console on reboot
It is not possible to run the console client if we are rebooting a guest via the backgrounded xl process so we may as well turn off console autoconnect after the first boot.
tools/libxl/xl_cmdimpl.c +6 -0
@@ -1188,6 +1188,12 @@ start:*/ dom_info->console_autoconnect =0 ; +/* +* Do not attempt to reconnect if we come round again due to a +* guest reboot -- the stdin/out will be disconnected by then. +*/ + dom_info->console_autoconnect =0 ; + ret =libxl_run_bootloader (&ctx, &b_info, num_disks >0 ? &disks[0 ] :NULL , domid);if (ret) {fprintf (stderr,"failed to run bootloader: %d\n" , ret);
PATCH 1/1 xl: Add function to generate random uuid and use it
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1280140563 -3600
# Node ID 8a13008e84ae463436fb304c50a2892324d0baaf # Parent 333254f8e11bc7bfa6901fe5e5585abb25e31398
xl: Add function to generate random uuid and use it.
tools/libxl/xl_cmdimpl.c +10 -8
@@ -211,15 +211,19 @@ static void init_build_info(libxl_domain } } +static void random_uuid (uint8_t *uuid) +{ +int i; +for (i =0 ; i <16 ; i++) + uuid[i] =rand (); +} +static void init_dm_info (libxl_device_model_info *dm_info,libxl_domain_create_info *c_info,libxl_domain_build_info *b_info) { -int i;memset (dm_info,'\0' ,sizeof (*dm_info)); -for (i =0 ; i <16 ; i++) { - dm_info->uuid[i] =rand (); - } +random_uuid (&dm_info->uuid[0 ]); dm_info->dom_name = c_info->name; dm_info->device_model ="qemu-dm" ;
@@ -493,7 +497,7 @@ static void parse_config_data(const charXLU_ConfigList *vbds, *nics, *pcis, *cvfbs, *net2s;int pci_power_mgmt =0 ;int pci_msitranslate =1 ; -int i, e; +int e; config=xlu_cfg_init (stderr, configfile_filename_report);if (!config) {
@@ -521,9 +525,7 @@ static void parse_config_data(const char c_info->name =strdup (buf); else c_info->name = "test"; -for (i =0 ; i <16 ; i++) { - c_info->uuid[i] =rand (); - } +random_uuid (&c_info->uuid[0 ]);if (!xlu_cfg_get_long (config,"oos" , &l)) c_info->oos = l;
PATCH 1/1 xl: Factor out domain death handling into a separate function
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1280140563 -3600
# Node ID ee265e700eede4111e429a3c55d2c78f31805028
# Parent 8a13008e84ae463436fb304c50a2892324d0baaf
xl: Factor out domain death handling into a separate function.
tools/libxl/xl_cmdimpl.c +25 -18
@@ -988,6 +988,18 @@ int autoconnect_console(int hvm)_exit (1 ); } +/* Returns 1 if domain should be restarted */ +static int handle_domain_death (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,struct libxl_dominfo *info) +{ +if (info->shutdown_reason != SHUTDOWN_suspend) { +LOG ("Domain %d needs to be clean: destroying the domain" , domid); +libxl_domain_destroy (ctx, domid,0 ); +if (info->shutdown_reason == SHUTDOWN_reboot) +return 1 ; + } +return 0 ; +} +struct domain_create {int debug;int daemonize;
@@ -1363,25 +1375,20 @@ start:LOG ("Domain %d is dead" , domid);if (ret) { -if (info.shutdown_reason != SHUTDOWN_suspend) { -LOG ("Domain %d needs to be clean: destroying the domain" , domid); -libxl_domain_destroy (&ctx, domid,0 ); -if (info.shutdown_reason == SHUTDOWN_reboot) { -libxl_free_waiter (w1); -libxl_free_waiter (w2); -free (w1); -free (w2); -LOG ("Done. Rebooting now" ); -/* -* XXX FIXME: If this sleep is not there then -* domain re-creation fails sometimes. -*/ -sleep (2 ); - goto start; - } -LOG ("Done. Exiting now" ); +if (handle_domain_death (&ctx, domid, &event, &info)) { +libxl_free_waiter (w1); +libxl_free_waiter (w2); +free (w1); +free (w2); +/* +* XXX FIXME: If this sleep is not there then domain +* re-creation fails sometimes. +*/ +LOG ("Done. Rebooting now" ); +sleep (2 ); + goto start; } -LOG ("Domain %d does not need to be clean, exiting now" , domid); +LOG ("Done. Exiting now" );exit (0 ); }break ;
PATCH 1/1 xl: support on_{poweroff, reboot, crash} domain configuration options
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1280140563 -3600
# Node ID 113b04a7e60718c8730368a045a49ddd56e64fac
# Parent ee265e700eede4111e429a3c55d2c78f31805028
xl: support on_{poweroff,reboot,crash} domain configuration options.
Adds on_watchdog compared to xend.
I have further plans for struct domain_config so it isn't as pointless as it first looks.
tools/libxl/xl_cmdimpl.c +201 -15
@@ -99,6 +99,38 @@ struct save_file_header {uint32_t optional_data_len;/* skip, or skip tail, if not understood */ }; + +enum action_on_shutdown { +ACTION_DESTROY , + +ACTION_RESTART , +ACTION_RESTART_RENAME , + +ACTION_PRESERVE , + +ACTION_COREDUMP_DESTROY , +ACTION_COREDUMP_RESTART , +}; + +static char *action_on_shutdown_names[] = { + [ACTION_DESTROY ] ="destroy" , + + [ACTION_RESTART ] ="restart" , + [ACTION_RESTART_RENAME ] ="rename-restart" , + + [ACTION_PRESERVE ] ="preserve" , + + [ACTION_COREDUMP_DESTROY ] ="coredump-destroy" , + [ACTION_COREDUMP_RESTART ] ="coredump-restart" , +}; + +struct domain_config { +enum action_on_shutdown on_poweroff; +enum action_on_shutdown on_reboot; +enum action_on_shutdown on_watchdog; +enum action_on_shutdown on_crash; +}; +/* Optional data, in order: * 4 bytes uint32_t config file size * n bytes config file in Unix text file format
@@ -472,11 +504,28 @@ static void printf_info(int domid,printf (")\n" ); } +static int parse_action_on_shutdown (const char *buf,enum action_on_shutdown *a) +{ +int i; +const char *n; + +for (i =0 ; i <sizeof (action_on_shutdown_names) /sizeof (action_on_shutdown_names[0 ]); i++) { + n = action_on_shutdown_names[i]; + +if (strcmp (buf, n) ==0 ) { + *a = i; +return 1 ; + } + } +return 0 ; +} + static void parse_config_data(const char *configfile_filename_report, const char *configfile_data, int configfile_len, libxl_domain_create_info *c_info, libxl_domain_build_info *b_info, + struct domain_config *d_config, libxl_device_disk **disks, int *num_disks, libxl_device_nic **vifs,
@@ -551,6 +600,35 @@ static void parse_config_data(const charif (!xlu_cfg_get_long (config,"memory" , &l)) { b_info->max_memkb = l *1024 ; b_info->target_memkb = b_info->max_memkb; + } + +if (xlu_cfg_get_string (config,"on_poweroff" , &buf)) + buf ="destroy" ; +if (!parse_action_on_shutdown (buf, &d_config->on_poweroff)) { +fprintf (stderr,"Unknown on_poweroff action \"%s\" specified\n" , buf); +exit (1 ); + } + +if (xlu_cfg_get_string (config,"on_reboot" , &buf)) + buf ="restart" ; +if (!parse_action_on_shutdown (buf, &d_config->on_reboot)) { +fprintf (stderr,"Unknown on_reboot action \"%s\" specified\n" , buf); +exit (1 ); + } + +if (xlu_cfg_get_string (config,"on_watchdog" , &buf)) + buf ="destroy" ; +if (!parse_action_on_shutdown (buf, &d_config->on_watchdog)) { +fprintf (stderr,"Unknown on_watchdog action \"%s\" specified\n" , buf); +exit (1 ); + } + + +if (xlu_cfg_get_string (config,"on_crash" , &buf)) + buf ="destroy" ; +if (!parse_action_on_shutdown (buf, &d_config->on_crash)) { +fprintf (stderr,"Unknown on_crash action \"%s\" specified\n" , buf); +exit (1 ); }/* libxl_get_required_shadow_memory() must be called after final values
@@ -988,16 +1066,112 @@ int autoconnect_console(int hvm)_exit (1 ); } -/* Returns 1 if domain should be restarted */ -static int handle_domain_death (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event,struct libxl_dominfo *info) -{ -if (info->shutdown_reason != SHUTDOWN_suspend) { -LOG ("Domain %d needs to be clean: destroying the domain" , domid); +/* Returns 1 if domain should be restarted, 2 if domain should be renamed then restarted */ +static int handle_domain_death (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event, +libxl_domain_create_info *c_info, +struct domain_config *d_config,struct libxl_dominfo *info) +{ +int restart =0 ; +enum action_on_shutdown action; + +switch (info->shutdown_reason) { +case LIBXL_SHUTDOWN_POWEROFF : + action = d_config->on_poweroff; +break ; +case LIBXL_SHUTDOWN_REBOOT : + action = d_config->on_reboot; +break ; +case LIBXL_SHUTDOWN_SUSPEND : +return 0 ; +case LIBXL_SHUTDOWN_CRASH : + action = d_config->on_crash; +break ; +case LIBXL_SHUTDOWN_WATCHDOG : + action = d_config->on_watchdog; +break ; + } + +LOG ("Action for shutdown reason code %d is %s" , info->shutdown_reason, action_on_shutdown_names[action]); + +if (action ==ACTION_COREDUMP_DESTROY || action ==ACTION_COREDUMP_RESTART ) { +char *corefile; +int rc; + +if (asprintf (&corefile,"/var/xen/dump/%s" , c_info->name) <0 ) { +LOG ("failed to construct core dump path" ); + }else { +LOG ("dumping core to %s" , corefile); + rc=libxl_domain_core_dump (ctx, domid, corefile); +if (rc)LOG ("core dump failed (rc=%d)." , rc); + } +/* No point crying over spilled milk, continue on failure. */ + +if (action ==ACTION_COREDUMP_DESTROY ) + action =ACTION_DESTROY ; +else + action =ACTION_RESTART ; + } + +switch (action) { +case ACTION_PRESERVE : +break ; + +case ACTION_RESTART_RENAME : + restart =2 ; +break ; + +case ACTION_RESTART : + restart =1 ; +/* fall-through */ +case ACTION_DESTROY : +LOG ("Domain %d needs to be cleaned up: destroying the domain" , domid);libxl_domain_destroy (ctx, domid,0 ); -if (info->shutdown_reason == SHUTDOWN_reboot) -return 1 ; - } -return 0 ; +break ; + +case ACTION_COREDUMP_DESTROY : +case ACTION_COREDUMP_RESTART : +/* Already handled these above. */ +abort (); + } + +return restart; +} + +static int preserve_domain (struct libxl_ctx *ctx,uint32_t domid,libxl_event *event, +libxl_domain_create_info *c_info, +struct domain_config *d_config,struct libxl_dominfo *info) +{ +time_t now; +struct tm tm; +char stime[24 ]; + +uint8_t new_uuid[16 ]; + +int rc; + + now =time (NULL ); +if (now == ((time_t) -1 )) { +LOG ("Failed to get current time for domain rename" ); +return 0 ; + } + +tzset (); +if (gmtime_r (&now, &tm) ==NULL ) { +LOG ("Failed to convert time to UTC" ); +return 0 ; + } + +if (!strftime (&stime[0 ],sizeof (stime),"-%Y%m%dT%H%MZ" , &tm)) { +LOG ("Failed to format time as a string" ); +return 0 ; + } + +random_uuid (&new_uuid[0 ]); + +LOG ("Preserving domain %d %s with suffix%s" , domid, c_info->name, stime); + rc =libxl_domain_preserve (ctx, domid, c_info, stime, new_uuid); + +return rc ==0 ?1 :0 ; }struct domain_create {
@@ -1016,6 +1190,8 @@ struct domain_create {static int create_domain (struct domain_create *dom_info) { +struct domain_config d_config; +libxl_domain_create_info c_info;libxl_domain_build_info b_info;libxl_domain_build_state state;
@@ -1150,7 +1326,7 @@ static int create_domain(struct domain_cif (!dom_info->quiet)printf ("Parsing config file %s\n" , config_file); -parse_config_data (config_file, config_data, config_len, &c_info, &b_info, &disks, &num_disks, &vifs, &num_vifs, &vif2s, &num_vif2s, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info); +parse_config_data (config_file, config_data, config_len, &c_info, &b_info, &d_config, &disks, &num_disks, &vifs, &num_vifs, &vif2s, &num_vif2s, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info);if (dom_info->dryrun)return 0 ;
@@ -1369,13 +1545,20 @@ start:switch (event.type) {case LIBXL_EVENT_DOMAIN_DEATH : ret =libxl_event_get_domain_death_info (&ctx, domid, &event, &info); -if (ret <0 )continue ;LOG ("Domain %d is dead" , domid);if (ret) { -if (handle_domain_death (&ctx, domid, &event, &info)) { +switch (handle_domain_death (&ctx, domid, &event, &c_info, &d_config, &info)) { +case 2 : +if (!preserve_domain (&ctx, domid, &event, &c_info, &d_config, &info)) +/* If we fail then exit leaving the old domain in place. */ +exit (-1 ); + +/* Otherwise fall through and restart. */ +case 1 : +libxl_free_waiter (w1);libxl_free_waiter (w2);free (w1);
@@ -1387,9 +1570,10 @@ start:LOG ("Done. Rebooting now" );sleep (2 ); goto start; +case 0 : +LOG ("Done. Exiting now" ); +exit (0 ); } -LOG ("Done. Exiting now" ); -exit (0 ); }break ;case LIBXL_EVENT_DISK_EJECT :
@@ -1898,6 +2082,8 @@ void list_domains_details(void)void list_domains_details (void ) {struct libxl_dominfo *info; +struct domain_config d_config; +char *config_file;uint8_t *data;int nb_domain, i, len, rc;
@@ -1923,7 +2109,7 @@ void list_domains_details(void)if (rc)continue ;CHK_ERRNO (asprintf (&config_file,"<domid %d data>" , info[i].domid)); -parse_config_data (config_file, (char *)data, len, &c_info, &b_info, &disks, &num_disks, &vifs, &num_vifs, &vif2s, &num_vif2s, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info); +parse_config_data (config_file, (char *)data, len, &c_info, &b_info, &d_config, &disks, &num_disks, &vifs, &num_vifs, &vif2s, &num_vif2s, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info);printf_info (info[i].domid, &c_info, &b_info, disks, num_disks, vifs, num_vifs, pcidevs, num_pcidevs, vfbs, num_vfbs, vkbs, num_vkbs, &dm_info);free (data);free (config_file);