The existing libxl_domain_shutdown is a bit odd, it takes an integer "req" which can be used to indicate one of: * [0] = "poweroff", * [1] = "reboot", * [2] = "suspend", * [3] = "crash", * [4] = "halt", "suspend" is not usable via this interface since it requires other scaffolding, libxl_domain_suspend provides this already. "halt" is the same as "poweroff". "crash" is unused and at least Linux does not implement it. If a user steps forward then libxl_domain_crash is trivial to add. Therefore split libxl_domain_shutdown into libxl_domain_shutdown and libxl_domain_reboot corresponding to "poweroff" and "reboot" respectively. Also push responsibility for dealing with lack of PV drivers into the caller and at the same time improve the error messages presented to the user when they try and "xl shutdown/reboot" an HVM guest with no PV drivers and the corresponding documentation. Changes since last time: - Remove massive redundancy in libxl_domain_{shutdown,reboot}
Ian Campbell
2011-Dec-21 10:49 UTC
[PATCH 1 of 2 V3] libxl: split libxl_domain_shutdown into libxl_domain_shutdown & libxl_domain_reboot
# HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1324464431 0 # Node ID 311aa2b2b8d137e73888a3ca07096f4670edcca2 # Parent 4656f8a68cc1b0d1353fb744b601437b3a0c4bfe libxl: split libxl_domain_shutdown into libxl_domain_shutdown & libxl_domain_reboot The other integer request types which shutdown supported are not useful. Specifically: * "suspend" is not usable via this interface since it requires other scaffolding, libxl_domain_suspend provides this already. * "halt" is the same as "poweroff". * "crash" is unused and at least Linux does not implement it. If a user steps forward then libxl_domain_crash is trivial to add. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r 4656f8a68cc1 -r 311aa2b2b8d1 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Wed Dec 21 10:46:26 2011 +0000 +++ b/tools/libxl/libxl.c Wed Dec 21 10:47:11 2011 +0000 @@ -594,38 +594,38 @@ int libxl__domain_pvcontrol_write(libxl_ return libxl__xs_write(gc, t, shutdown_path, "%s", cmd); } -static char *req_table[] = { - [0] = "poweroff", - [1] = "reboot", - [2] = "suspend", - [3] = "crash", - [4] = "halt", -}; - -int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid, int req) +static int libxl__domain_pvcontrol(libxl__gc *gc, uint32_t domid, + const char *cmd) +{ + int ret; + + ret = libxl__domain_pvcontrol_available(gc, domid); + if (ret < 0) + return ret; + + if (!ret) { + LIBXL__LOG(CTX, LIBXL__LOG_ERROR, + "PV control interface not available\n"); + return ERROR_FAIL; + } + + return libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, cmd); +} + +int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid) { GC_INIT(ctx); int ret; - - if (req > ARRAY_SIZE(req_table)) { - GC_FREE; - return ERROR_INVAL; - } - - ret = libxl__domain_pvcontrol_available(gc, domid); - if (ret < 0) - goto out; - - if (!ret) { - LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "PV shutdown control not available:" - " graceful shutdown not possible, use destroy"); - ret = ERROR_FAIL; - goto out; - } - - ret = libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, req_table[req]); - -out: + ret = libxl__domain_pvcontrol(gc, domid, "poweroff"); + GC_FREE; + return ret; +} + +int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid) +{ + GC_INIT(ctx); + int ret; + ret = libxl__domain_pvcontrol(gc, domid, "reboot"); GC_FREE; return ret; } diff -r 4656f8a68cc1 -r 311aa2b2b8d1 tools/libxl/libxl.h --- a/tools/libxl/libxl.h Wed Dec 21 10:46:26 2011 +0000 +++ b/tools/libxl/libxl.h Wed Dec 21 10:47:11 2011 +0000 @@ -268,7 +268,8 @@ void libxl_domain_config_dispose(libxl_d int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info, uint32_t domid, int fd); int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid); -int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid, int req); +int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid); +int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid); int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid); int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_info *info, const char *name_suffix, libxl_uuid new_uuid); diff -r 4656f8a68cc1 -r 311aa2b2b8d1 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Wed Dec 21 10:46:26 2011 +0000 +++ b/tools/libxl/xl_cmdimpl.c Wed Dec 21 10:47:11 2011 +0000 @@ -2265,7 +2265,7 @@ static void shutdown_domain(const char * int rc; find_domain(p); - rc=libxl_domain_shutdown(ctx, domid, 0); + rc=libxl_domain_shutdown(ctx, domid); if (rc) { fprintf(stderr,"shutdown failed (rc=%d)\n",rc);exit(-1); } if (wait) { @@ -2307,7 +2307,7 @@ static void reboot_domain(const char *p) { int rc; find_domain(p); - rc=libxl_domain_shutdown(ctx, domid, 1); + rc=libxl_domain_reboot(ctx, domid); if (rc) { fprintf(stderr,"reboot failed (rc=%d)\n",rc);exit(-1); } } diff -r 4656f8a68cc1 -r 311aa2b2b8d1 tools/python/xen/lowlevel/xl/xl.c --- a/tools/python/xen/lowlevel/xl/xl.c Wed Dec 21 10:46:26 2011 +0000 +++ b/tools/python/xen/lowlevel/xl/xl.c Wed Dec 21 10:47:11 2011 +0000 @@ -424,10 +424,10 @@ static PyObject *pyxl_domid_to_name(XlOb static PyObject *pyxl_domain_shutdown(XlObject *self, PyObject *args) { - int domid, req = 0; - if ( !PyArg_ParseTuple(args, "i|i", &domid, &req) ) + int domid; + if ( !PyArg_ParseTuple(args, "i", &domid) ) return NULL; - if ( libxl_domain_shutdown(self->ctx, domid, req) ) { + if ( libxl_domain_shutdown(self->ctx, domid) ) { PyErr_SetString(xl_error_obj, "cannot shutdown domain"); return NULL; } @@ -435,6 +435,19 @@ static PyObject *pyxl_domain_shutdown(Xl return Py_None; } +static PyObject *pyxl_domain_reboot(XlObject *self, PyObject *args) +{ + int domid; + if ( !PyArg_ParseTuple(args, "i", &domid) ) + return NULL; + if ( libxl_domain_reboot(self->ctx, domid) ) { + PyErr_SetString(xl_error_obj, "cannot reboot domain"); + return NULL; + } + Py_INCREF(Py_None); + return Py_None; +} + static PyObject *pyxl_domain_destroy(XlObject *self, PyObject *args) { int domid; @@ -637,6 +650,8 @@ static PyMethodDef pyxl_methods[] = { "Retrieve name from domain-id"}, {"domain_shutdown", (PyCFunction)pyxl_domain_shutdown, METH_VARARGS, "Shutdown a domain"}, + {"domain_reboot", (PyCFunction)pyxl_domain_reboot, METH_VARARGS, + "Reboot a domain"}, {"domain_destroy", (PyCFunction)pyxl_domain_destroy, METH_VARARGS, "Destroy a domain"}, {"domain_pause", (PyCFunction)pyxl_domain_pause, METH_VARARGS,
Ian Campbell
2011-Dec-21 10:49 UTC
[PATCH 2 of 2 V3] libxl: report failure to reboot/shutdown due to lackof PV interfaces to caller
# HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1324464450 0 # Node ID 3dedf6c82da2bb53d8693fac8b909cdafdfceeca # Parent 311aa2b2b8d137e73888a3ca07096f4670edcca2 libxl: report failure to reboot/shutdown due to lackof PV interfaces to caller This allow the caller to react as they think is appropriate. xl now prints a message much like the library did previously, although hopefully somewhat more informative. Update the xl(1) man page to be similarly more informative. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r 311aa2b2b8d1 -r 3dedf6c82da2 docs/man/xl.pod.1 --- a/docs/man/xl.pod.1 Wed Dec 21 10:47:11 2011 +0000 +++ b/docs/man/xl.pod.1 Wed Dec 21 10:47:30 2011 +0000 @@ -360,7 +360,11 @@ Reboot a domain. This acts just as if t command run from the console. The command returns as soon as it has executed the reboot action, which may be significantly before the domain actually reboots. -It requires PV drivers installed in your guest OS. + +For HVM domains this requires PV drivers to be installed in your guest +OS. If PV drivers are not present but you have configured the guest OS +to behave appropriately you may be able to use the I<button-press> +subcommand to trigger a power button press. The behavior of what happens to a domain when it reboots is set by the B<on_reboot> parameter of the domain configuration file when the @@ -412,9 +416,15 @@ Leave domain running after creating the Gracefully shuts down a domain. This coordinates with the domain OS to perform graceful shutdown, so there is no guarantee that it will succeed, and may take a variable length of time depending on what -services must be shutdown in the domain. The command returns -immediately after signally the domain unless that B<-w> flag is used. -For HVM domains it requires PV drivers to be installed in your guest OS. +services must be shutdown in the domain. + +For HVM domains this requires PV drivers to be installed in your guest +OS. If PV drivers are not present but you have configured the guest OS +to behave appropriately you may be able to use the I<button-press> +subcommand to trigger a power button press. + +The command returns immediately after signally the domain unless that +B<-w> flag is used. The behavior of what happens to a domain when it reboots is set by the B<on_shutdown> parameter of the domain configuration file when the diff -r 311aa2b2b8d1 -r 3dedf6c82da2 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Wed Dec 21 10:47:11 2011 +0000 +++ b/tools/libxl/libxl.c Wed Dec 21 10:47:30 2011 +0000 @@ -603,11 +603,8 @@ static int libxl__domain_pvcontrol(libxl if (ret < 0) return ret; - if (!ret) { - LIBXL__LOG(CTX, LIBXL__LOG_ERROR, - "PV control interface not available\n"); - return ERROR_FAIL; - } + if (!ret) + return ERROR_NOPARAVIRT; return libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, cmd); } diff -r 311aa2b2b8d1 -r 3dedf6c82da2 tools/libxl/libxl.h --- a/tools/libxl/libxl.h Wed Dec 21 10:47:11 2011 +0000 +++ b/tools/libxl/libxl.h Wed Dec 21 10:47:30 2011 +0000 @@ -222,6 +222,7 @@ enum { ERROR_BADFAIL = -7, ERROR_GUEST_TIMEDOUT = -8, ERROR_TIMEDOUT = -9, + ERROR_NOPARAVIRT = -10, }; #define LIBXL_VERSION 0 diff -r 311aa2b2b8d1 -r 3dedf6c82da2 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Wed Dec 21 10:47:11 2011 +0000 +++ b/tools/libxl/xl_cmdimpl.c Wed Dec 21 10:47:30 2011 +0000 @@ -2266,7 +2266,15 @@ static void shutdown_domain(const char * find_domain(p); rc=libxl_domain_shutdown(ctx, domid); - if (rc) { fprintf(stderr,"shutdown failed (rc=%d)\n",rc);exit(-1); } + if (rc) { + if (rc == ERROR_NOPARAVIRT) { + fprintf(stderr, "PV control interface not available:" + " external graceful shutdown not possible.\n"); + fprintf(stderr, "Use \"xl button-press <dom> power\" or" + " \"xl destroy <dom>\".\n"); + } + fprintf(stderr,"shutdown failed (rc=%d)\n",rc);exit(-1); + } if (wait) { libxl_waiter waiter; @@ -2308,7 +2316,14 @@ static void reboot_domain(const char *p) int rc; find_domain(p); rc=libxl_domain_reboot(ctx, domid); - if (rc) { fprintf(stderr,"reboot failed (rc=%d)\n",rc);exit(-1); } + if (rc) { + if (rc == ERROR_NOPARAVIRT) { + fprintf(stderr, "PV control interface not available:" + " external graceful reboot not possible.\n"); + fprintf(stderr, "Use \"xl button-press <dom> power\" or" + " \"xl destroy <dom>\".\n"); + } + fprintf(stderr,"reboot failed (rc=%d)\n",rc);exit(-1); } } static void list_domains_details(const libxl_dominfo *info, int nb_domain)
Ian Jackson
2011-Dec-21 14:55 UTC
Re: [PATCH 2 of 2 V3] libxl: report failure to reboot/shutdown due to lackof PV interfaces to caller
Ian Campbell writes ("[Xen-devel] [PATCH 2 of 2 V3] libxl: report failure to reboot/shutdown due to lackof PV interfaces to caller"):> libxl: report failure to reboot/shutdown due to lackof PV interfaces to callerApplied both of these, thanks. (You should make your commit messages fit in 80 columns; I massaged it for you.) Ian.