|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 03/19] libxl: domain restore: reshuffle, preparing for ao
On Fri, 2012-06-08 at 18:34 +0100, Ian Jackson wrote:
> We are going to arrange that libxl, instead of calling
> xc_domain_restore, calls a stub function which forks and execs a
> helper program, so that restore can be asynchronous rather than
> blocking the whole toolstack.
>
> This stub function will be called libxl__xc_domain_restore.
>
> However, its prospective call site is unsuitable for a function which
> needs to make a callback, and is buried in two nested single-call-site
> functions which are logically part of the domain creation procedure.
>
> So we first abolish those single-call-site functions, integrate their
> contents into domain creation in their proper temporal order, and
> break out libxl__xc_domain_restore ready for its reimplementation.
>
> No functional change - just the following reorganisation:
>
> * Abolish libxl__domain_restore_common, as it had only one caller.
> Move its contents into (what was) domain_restore.
>
> * There is a new stage function domcreate_rebuild_done containing what
> used to be the bulk of domcreate_bootloader_done, since
> domcreate_bootloader_done now simply starts the restore (or does the
> rebuild) and arranges to call the next stage.
>
> * Move the contents of domain_restore into its correct place in the
> domain creation sequence. We put it inside
> domcreate_bootloader_done, which now either calls
"either" but no "or"? I suppose the or case is call
domcreate_rebuild_done directly?
> libxl__xc_domain_restore which will call the new function
> domcreate_rebuild_done.
>
> * Various general-purpose local variables (`i' etc.) and convenience
> alias variables need to be shuffled about accordingly.
>
> * Consequently libxl__toolstack_restore needs to gain external linkage
> as it is now in a different file to its user.
>
> * Move the xc_domain_save callbacks struct from the stack into
> libxl__domain_create_state.
>
> In general the moved code remains almost identical. Two returns in
> what used to be libxl__domain_restore_common have been changed to set
> the return value and "goto out", and the call sites for the abolished
> and new functions have been adjusted.
>
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
I confirmed at a high level that the same blocks of code exist and are
called in the same overall order, but I didn't do a line by line
comparison of the blocks in question, assuming they really are mostly
motion and adjustments for the new context.
>
> Changes in v2:
> * Also move the save callbacks
> ---
> tools/libxl/Makefile | 1 +
> tools/libxl/libxl_create.c | 244 +++++++++++++++++++++++--------------
> tools/libxl/libxl_dom.c | 45 +-------
> tools/libxl/libxl_internal.h | 19 +++-
> tools/libxl/libxl_save_callout.c | 37 ++++++
> 5 files changed, 206 insertions(+), 140 deletions(-)
> create mode 100644 tools/libxl/libxl_save_callout.c
>
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index e7d5cc2..1d8b80a 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -67,6 +67,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o
> libxl_pci.o \
> libxl_dom.o libxl_exec.o libxl_xshelp.o
> libxl_device.o \
> libxl_internal.o libxl_utils.o libxl_uuid.o \
> libxl_json.o libxl_aoutils.o \
> + libxl_save_callout.o \
> libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
> LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 4456ae8..4439367 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -21,7 +21,6 @@
> #include "libxl_arch.h"
>
> #include <xc_dom.h>
> -#include <xenguest.h>
>
> void libxl_domain_config_init(libxl_domain_config *d_config)
> {
> @@ -317,89 +316,6 @@ out:
> return ret;
> }
>
> -static int domain_restore(libxl__gc *gc, libxl_domain_build_info *info,
> - uint32_t domid, int fd,
> - libxl__domain_build_state *state)
> -{
> - libxl_ctx *ctx = libxl__gc_owner(gc);
> - char **vments = NULL, **localents = NULL;
> - struct timeval start_time;
> - int i, ret, esave, flags;
> -
> - ret = libxl__build_pre(gc, domid, info, state);
> - if (ret)
> - goto out;
> -
> - ret = libxl__domain_restore_common(gc, domid, info, state, fd);
> - if (ret)
> - goto out;
> -
> - gettimeofday(&start_time, NULL);
> -
> - switch (info->type) {
> - case LIBXL_DOMAIN_TYPE_HVM:
> - vments = libxl__calloc(gc, 7, sizeof(char *));
> - vments[0] = "rtc/timeoffset";
> - vments[1] = (info->u.hvm.timeoffset) ? info->u.hvm.timeoffset : "";
> - vments[2] = "image/ostype";
> - vments[3] = "hvm";
> - vments[4] = "start_time";
> - vments[5] = libxl__sprintf(gc, "%lu.%02d",
> start_time.tv_sec,(int)start_time.tv_usec/10000);
> - break;
> - case LIBXL_DOMAIN_TYPE_PV:
> - vments = libxl__calloc(gc, 11, sizeof(char *));
> - i = 0;
> - vments[i++] = "image/ostype";
> - vments[i++] = "linux";
> - vments[i++] = "image/kernel";
> - vments[i++] = (char *) state->pv_kernel.path;
> - vments[i++] = "start_time";
> - vments[i++] = libxl__sprintf(gc, "%lu.%02d",
> start_time.tv_sec,(int)start_time.tv_usec/10000);
> - if (state->pv_ramdisk.path) {
> - vments[i++] = "image/ramdisk";
> - vments[i++] = (char *) state->pv_ramdisk.path;
> - }
> - if (state->pv_cmdline) {
> - vments[i++] = "image/cmdline";
> - vments[i++] = (char *) state->pv_cmdline;
> - }
> - break;
> - default:
> - ret = ERROR_INVAL;
> - goto out;
> - }
> - ret = libxl__build_post(gc, domid, info, state, vments, localents);
> - if (ret)
> - goto out;
> -
> - if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
> - ret = asprintf(&state->saved_state,
> - XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
> - ret = (ret < 0) ? ERROR_FAIL : 0;
> - }
> -
> -out:
> - if (info->type == LIBXL_DOMAIN_TYPE_PV) {
> - libxl__file_reference_unmap(&state->pv_kernel);
> - libxl__file_reference_unmap(&state->pv_ramdisk);
> - }
> -
> - esave = errno;
> -
> - flags = fcntl(fd, F_GETFL);
> - if (flags == -1) {
> - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on
> restore fd");
> - } else {
> - flags &= ~O_NONBLOCK;
> - if (fcntl(fd, F_SETFL, flags) == -1)
> - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore
> fd"
> - " back to blocking mode");
> - }
> -
> - errno = esave;
> - return ret;
> -}
> -
> int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info,
> uint32_t *domid)
> {
> @@ -580,10 +496,13 @@ static void
> domcreate_bootloader_console_available(libxl__egc *egc,
> static void domcreate_bootloader_done(libxl__egc *egc,
> libxl__bootloader_state *bl,
> int rc);
> -
> static void domcreate_console_available(libxl__egc *egc,
> libxl__domain_create_state *dcs);
>
> +static void domcreate_rebuild_done(libxl__egc *egc,
> + libxl__domain_create_state *dcs,
> + int ret);
> +
> /* Our own function to clean up and call the user's callback.
> * The final call in the sequence. */
> static void domcreate_complete(libxl__egc *egc,
> @@ -671,20 +590,20 @@ static void domcreate_console_available(libxl__egc *egc,
>
> static void domcreate_bootloader_done(libxl__egc *egc,
> libxl__bootloader_state *bl,
> - int ret)
> + int rc)
> {
> libxl__domain_create_state *dcs = CONTAINER_OF(bl, *dcs, bl);
> STATE_AO_GC(bl->ao);
> - int i;
>
> /* convenience aliases */
> const uint32_t domid = dcs->guest_domid;
> libxl_domain_config *const d_config = dcs->guest_config;
> + libxl_domain_build_info *const info = &d_config->b_info;
> const int restore_fd = dcs->restore_fd;
> libxl__domain_build_state *const state = &dcs->build_state;
> - libxl_ctx *const ctx = CTX;
> + struct restore_callbacks *const callbacks = &dcs->callbacks;
>
> - if (ret) goto error_out;
> + if (rc) domcreate_rebuild_done(egc, dcs, rc);
>
> /* consume bootloader outputs. state->pv_{kernel,ramdisk} have
> * been initialised by the bootloader already.
> @@ -700,12 +619,153 @@ static void domcreate_bootloader_done(libxl__egc *egc,
> dcs->dmss.dm.callback = domcreate_devmodel_started;
> dcs->dmss.callback = domcreate_devmodel_started;
>
> - if ( restore_fd >= 0 ) {
> - ret = domain_restore(gc, &d_config->b_info, domid, restore_fd,
> state);
> + if ( restore_fd < 0 ) {
> + rc = libxl__domain_build(gc, &d_config->b_info, domid, state);
> + domcreate_rebuild_done(egc, dcs, rc);
> + return;
> + }
> +
> + /* Restore */
> +
> + rc = libxl__build_pre(gc, domid, info, state);
> + if (rc)
> + goto out;
> +
> + /* read signature */
> + int hvm, pae, superpages;
> + int no_incr_generationid;
> + switch (info->type) {
> + case LIBXL_DOMAIN_TYPE_HVM:
> + hvm = 1;
> + superpages = 1;
> + pae = libxl_defbool_val(info->u.hvm.pae);
> + no_incr_generationid =
> !libxl_defbool_val(info->u.hvm.incr_generationid);
> + callbacks->toolstack_restore = libxl__toolstack_restore;
> + callbacks->data = gc;
> + break;
> + case LIBXL_DOMAIN_TYPE_PV:
> + hvm = 0;
> + superpages = 0;
> + pae = 1;
> + no_incr_generationid = 0;
> + break;
> + default:
> + rc = ERROR_INVAL;
> + goto out;
> + }
> + libxl__xc_domain_restore(egc, dcs,
> + hvm, pae, superpages, no_incr_generationid);
> + return;
> +
> + out:
> + libxl__xc_domain_restore_done(egc, dcs, rc, 0, 0);
> +}
> +
> +void libxl__xc_domain_restore_done(libxl__egc *egc,
> + libxl__domain_create_state *dcs,
> + int ret, int retval, int errnoval)
> +{
> + STATE_AO_GC(dcs->ao);
> + libxl_ctx *ctx = libxl__gc_owner(gc);
> + char **vments = NULL, **localents = NULL;
> + struct timeval start_time;
> + int i, esave, flags;
> +
> + /* convenience aliases */
> + const uint32_t domid = dcs->guest_domid;
> + libxl_domain_config *const d_config = dcs->guest_config;
> + libxl_domain_build_info *const info = &d_config->b_info;
> + libxl__domain_build_state *const state = &dcs->build_state;
> + const int fd = dcs->restore_fd;
> +
> + if (ret)
> + goto out;
> +
> + if (retval) {
> + LOGEV(ERROR, errnoval, "restoring domain");
> + ret = ERROR_FAIL;
> + goto out;
> + }
> +
> + gettimeofday(&start_time, NULL);
> +
> + switch (info->type) {
> + case LIBXL_DOMAIN_TYPE_HVM:
> + vments = libxl__calloc(gc, 7, sizeof(char *));
> + vments[0] = "rtc/timeoffset";
> + vments[1] = (info->u.hvm.timeoffset) ? info->u.hvm.timeoffset : "";
> + vments[2] = "image/ostype";
> + vments[3] = "hvm";
> + vments[4] = "start_time";
> + vments[5] = libxl__sprintf(gc, "%lu.%02d",
> start_time.tv_sec,(int)start_time.tv_usec/10000);
> + break;
> + case LIBXL_DOMAIN_TYPE_PV:
> + vments = libxl__calloc(gc, 11, sizeof(char *));
> + i = 0;
> + vments[i++] = "image/ostype";
> + vments[i++] = "linux";
> + vments[i++] = "image/kernel";
> + vments[i++] = (char *) state->pv_kernel.path;
> + vments[i++] = "start_time";
> + vments[i++] = libxl__sprintf(gc, "%lu.%02d",
> start_time.tv_sec,(int)start_time.tv_usec/10000);
> + if (state->pv_ramdisk.path) {
> + vments[i++] = "image/ramdisk";
> + vments[i++] = (char *) state->pv_ramdisk.path;
> + }
> + if (state->pv_cmdline) {
> + vments[i++] = "image/cmdline";
> + vments[i++] = (char *) state->pv_cmdline;
> + }
> + break;
> + default:
> + ret = ERROR_INVAL;
> + goto out;
> + }
> + ret = libxl__build_post(gc, domid, info, state, vments, localents);
> + if (ret)
> + goto out;
> +
> + if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
> + ret = asprintf(&state->saved_state,
> + XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
> + ret = (ret < 0) ? ERROR_FAIL : 0;
> + }
> +
> +out:
> + if (info->type == LIBXL_DOMAIN_TYPE_PV) {
> + libxl__file_reference_unmap(&state->pv_kernel);
> + libxl__file_reference_unmap(&state->pv_ramdisk);
> + }
> +
> + esave = errno;
> +
> + flags = fcntl(fd, F_GETFL);
> + if (flags == -1) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on
> restore fd");
> } else {
> - ret = libxl__domain_build(gc, &d_config->b_info, domid, state);
> + flags &= ~O_NONBLOCK;
> + if (fcntl(fd, F_SETFL, flags) == -1)
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore
> fd"
> + " back to blocking mode");
> }
>
> + errno = esave;
> + domcreate_rebuild_done(egc, dcs, ret);
> +}
> +
> +static void domcreate_rebuild_done(libxl__egc *egc,
> + libxl__domain_create_state *dcs,
> + int ret)
> +{
> + STATE_AO_GC(dcs->ao);
> + int i;
> +
> + /* convenience aliases */
> + const uint32_t domid = dcs->guest_domid;
> + libxl_domain_config *const d_config = dcs->guest_config;
> + libxl__domain_build_state *const state = &dcs->build_state;
> + libxl_ctx *const ctx = CTX;
> +
> if (ret) {
> LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot (re-)build domain: %d",
> ret);
> ret = ERROR_FAIL;
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index e90030d..1d4e809 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -19,7 +19,6 @@
>
> #include <xenctrl.h>
> #include <xc_dom.h>
> -#include <xenguest.h>
>
> #include <xen/hvm/hvm_info_table.h>
>
> @@ -467,7 +466,7 @@ static inline char *restore_helper(libxl__gc *gc,
> uint32_t domid,
> domid, phys_offset, node);
> }
>
> -static int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
> +int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
> uint32_t size, void *data)
> {
> libxl__gc *gc = data;
> @@ -522,48 +521,6 @@ static int libxl__toolstack_restore(uint32_t domid,
> const uint8_t *buf,
> return 0;
> }
>
> -int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
> - libxl_domain_build_info *info,
> - libxl__domain_build_state *state,
> - int fd)
> -{
> - libxl_ctx *ctx = libxl__gc_owner(gc);
> - /* read signature */
> - int rc;
> - int hvm, pae, superpages;
> - struct restore_callbacks callbacks[1];
> - int no_incr_generationid;
> - switch (info->type) {
> - case LIBXL_DOMAIN_TYPE_HVM:
> - hvm = 1;
> - superpages = 1;
> - pae = libxl_defbool_val(info->u.hvm.pae);
> - no_incr_generationid =
> !libxl_defbool_val(info->u.hvm.incr_generationid);
> - callbacks->toolstack_restore = libxl__toolstack_restore;
> - callbacks->data = gc;
> - break;
> - case LIBXL_DOMAIN_TYPE_PV:
> - hvm = 0;
> - superpages = 0;
> - pae = 1;
> - no_incr_generationid = 0;
> - break;
> - default:
> - return ERROR_INVAL;
> - }
> - rc = xc_domain_restore(ctx->xch, fd, domid,
> - state->store_port, &state->store_mfn,
> - state->store_domid, state->console_port,
> - &state->console_mfn, state->console_domid,
> - hvm, pae, superpages, no_incr_generationid,
> - &state->vm_generationid_addr, callbacks);
> - if ( rc ) {
> - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "restoring domain");
> - return ERROR_FAIL;
> - }
> - return 0;
> -}
> -
> static int libxl__domain_suspend_common_switch_qemu_logdirty
> (int domid, unsigned int enable, void *data)
> {
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index f22bf94..28478ea 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -46,6 +46,7 @@
>
> #include <xenstore.h>
> #include <xenctrl.h>
> +#include <xenguest.h>
>
> #include "xentoollog.h"
>
> @@ -782,10 +783,8 @@ _hidden int libxl__domain_rename(libxl__gc *gc, uint32_t
> domid,
> const char *old_name, const char *new_name,
> xs_transaction_t trans);
>
> -_hidden int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
> - libxl_domain_build_info *info,
> - libxl__domain_build_state *state,
> - int fd);
> +_hidden int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
> + uint32_t size, void *data);
> _hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t
> domid);
> _hidden int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t
> domid);
> _hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
> @@ -1899,6 +1898,7 @@ struct libxl__domain_create_state {
> libxl__stub_dm_spawn_state dmss;
> /* If we're not doing stubdom, we use only dmss.dm,
> * for the non-stubdom device model. */
> + struct restore_callbacks callbacks;
> };
>
> /*----- Domain suspend (save) functions -----*/
> @@ -1908,6 +1908,17 @@ _hidden int libxl__domain_suspend_common(libxl__gc
> *gc, uint32_t domid, int fd,
> int live, int debug,
> const libxl_domain_remus_info
> *r_info);
>
> +/* calls libxl__xc_domain_restore_done when done */
> +_hidden void libxl__xc_domain_restore(libxl__egc *egc,
> + libxl__domain_create_state *dcs,
> + int hvm, int pae, int superpages,
> + int no_incr_generationid);
> +/* If rc==0 then retval is the return value from xc_domain_save
> + * and errnoval is the errno value it provided.
> + * If rc!=0, retval and errnoval are undefined. */
> +_hidden void libxl__xc_domain_restore_done(libxl__egc *egc,
> + libxl__domain_create_state *dcs,
> + int rc, int retval, int errnoval);
>
>
> /*
> diff --git a/tools/libxl/libxl_save_callout.c
> b/tools/libxl/libxl_save_callout.c
> new file mode 100644
> index 0000000..2f8db9f
> --- /dev/null
> +++ b/tools/libxl/libxl_save_callout.c
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (C) 2012 Citrix Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_osdeps.h"
> +
> +#include "libxl_internal.h"
> +
> +void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state
> *dcs,
> + int hvm, int pae, int superpages,
> + int no_incr_generationid)
> +{
> + STATE_AO_GC(dcs->ao);
> +
> + /* Convenience aliases */
> + const uint32_t domid = dcs->guest_domid;
> + const int restore_fd = dcs->restore_fd;
> + libxl__domain_build_state *const state = &dcs->build_state;
> +
> + int r = xc_domain_restore(CTX->xch, restore_fd, domid,
> + state->store_port, &state->store_mfn,
> + state->store_domid, state->console_port,
> + &state->console_mfn, state->console_domid,
> + hvm, pae, superpages, no_incr_generationid,
> + &state->vm_generationid_addr, &dcs->callbacks);
> + libxl__xc_domain_restore_done(egc, dcs, 0, r, errno);
> +}
> --
> 1.7.2.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |