[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH RFC v2 07/23] libxc/migration: tidy the xc_domain_save()/restore() interface
From: Joshua Otto <jtotto@xxxxxxxxxxxx> Both xc_domain_save() and xc_domain_restore() have a high number of parameters, including a number of boolean parameters that are split between a bitfield flags argument and separate individual boolean arguments. Further, many of these arguments are dead/ignored. Tidy the interface to these functions by collecting the parameters into a structure assembled by the caller and passed by pointer, and drop the dead parameters. No functional change. Signed-off-by: Joshua Otto <jtotto@xxxxxxxxxxxx> --- tools/libxc/include/xenguest.h | 68 ++++++++++++++++++++-------------------- tools/libxc/xc_nomigrate.c | 16 +++------- tools/libxc/xc_sr_common.h | 4 +-- tools/libxc/xc_sr_restore.c | 47 +++++++++++++-------------- tools/libxc/xc_sr_save.c | 54 +++++++++++++++---------------- tools/libxl/libxl_dom_save.c | 11 ------- tools/libxl/libxl_internal.h | 1 - tools/libxl/libxl_save_callout.c | 12 +++---- tools/libxl/libxl_save_helper.c | 61 ++++++++++++++++------------------- 9 files changed, 122 insertions(+), 152 deletions(-) diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h index aa8cc8b..d1f97b9 100644 --- a/tools/libxc/include/xenguest.h +++ b/tools/libxc/include/xenguest.h @@ -22,16 +22,9 @@ #ifndef XENGUEST_H #define XENGUEST_H -#define XC_NUMA_NO_NODE (~0U) - -#define XCFLAGS_LIVE (1 << 0) -#define XCFLAGS_DEBUG (1 << 1) -#define XCFLAGS_HVM (1 << 2) -#define XCFLAGS_STDVGA (1 << 3) -#define XCFLAGS_CHECKPOINT_COMPRESS (1 << 4) +#include <stdbool.h> -#define X86_64_B_SIZE 64 -#define X86_32_B_SIZE 32 +#define XC_NUMA_NO_NODE (~0U) /* * User not using xc_suspend_* / xc_await_suspent may not want to @@ -90,20 +83,26 @@ typedef enum { XC_MIG_STREAM_COLO, } xc_migration_stream_t; +struct domain_save_params { + uint32_t dom; /* the id of the domain */ + int save_fd; /* the fd to save the domain to */ + int recv_fd; /* the fd to receive live protocol responses */ + uint32_t max_iters; /* how many precopy iterations before we give up? */ + bool live; /* is this a live migration? */ + bool debug; /* are we in debug mode? */ + xc_migration_stream_t stream_type; /* is there checkpointing involved? */ +}; + /** * This function will save a running domain. * * @parm xch a handle to an open hypervisor interface - * @parm fd the file descriptor to save a domain to - * @parm dom the id of the domain - * @param stream_type XC_MIG_STREAM_NONE if the far end of the stream - * doesn't use checkpointing + * @parm params a description of the requested save/migration + * @parm callbacks hooks for delegated steps of the save procedure * @return 0 on success, -1 on failure */ -int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, - uint32_t max_factor, uint32_t flags /* XCFLAGS_xxx */, - struct save_callbacks* callbacks, int hvm, - xc_migration_stream_t stream_type, int recv_fd); +int xc_domain_save(xc_interface *xch, const struct domain_save_params *params, + const struct save_callbacks *callbacks); /* callbacks provided by xc_domain_restore */ struct restore_callbacks { @@ -145,31 +144,32 @@ struct restore_callbacks { void* data; }; +struct domain_restore_params { + uint32_t dom; /* the id of the domain */ + int recv_fd; /* the fd to restore the domain from */ + int send_back_fd; /* the fd to send live protocol responses */ + unsigned int store_evtchn; /* the store event channel */ + xen_pfn_t *store_gfn; /* OUT - the gfn of the store page */ + domid_t store_domid; /* the store domain id */ + unsigned int console_evtchn; /* the console event channel */ + xen_pfn_t *console_gfn; /* OUT - the gfn of the console page */ + domid_t console_domid; /* the console domain id */ + xc_migration_stream_t stream_type; /* is there checkpointing involved? */ +}; + /** * This function will restore a saved domain. * * Domain is restored in a suspended state ready to be unpaused. * * @parm xch a handle to an open hypervisor interface - * @parm fd the file descriptor to restore a domain from - * @parm dom the id of the domain - * @parm store_evtchn the store event channel for this domain to use - * @parm store_mfn returned with the mfn of the store page - * @parm hvm non-zero if this is a HVM restore - * @parm pae non-zero if this HVM domain has PAE support enabled - * @parm superpages non-zero to allocate guest memory with superpages - * @parm stream_type non-zero if the far end of the stream is using checkpointing - * @parm callbacks non-NULL to receive a callback to restore toolstack - * specific data + * @parm params a description of the requested restore operation + * @parm callbacks hooks for delegated steps of the restore procedure * @return 0 on success, -1 on failure */ -int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, - unsigned int store_evtchn, unsigned long *store_mfn, - domid_t store_domid, unsigned int console_evtchn, - unsigned long *console_mfn, domid_t console_domid, - unsigned int hvm, unsigned int pae, int superpages, - xc_migration_stream_t stream_type, - struct restore_callbacks *callbacks, int send_back_fd); +int xc_domain_restore(xc_interface *xch, + const struct domain_restore_params *params, + const struct restore_callbacks *callbacks); /** * This function will create a domain for a paravirtualized Linux diff --git a/tools/libxc/xc_nomigrate.c b/tools/libxc/xc_nomigrate.c index 15c838f..50cd318 100644 --- a/tools/libxc/xc_nomigrate.c +++ b/tools/libxc/xc_nomigrate.c @@ -20,22 +20,16 @@ #include <xenctrl.h> #include <xenguest.h> -int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, - uint32_t max_factor, uint32_t flags, - struct save_callbacks* callbacks, int hvm, - xc_migration_stream_t stream_type, int recv_fd) +int xc_domain_save(xc_interface *xch, const struct domain_save_params *params, + const struct save_callbacks *callbacks) { errno = ENOSYS; return -1; } -int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, - unsigned int store_evtchn, unsigned long *store_mfn, - domid_t store_domid, unsigned int console_evtchn, - unsigned long *console_mfn, domid_t console_domid, - unsigned int hvm, unsigned int pae, int superpages, - xc_migration_stream_t stream_type, - struct restore_callbacks *callbacks, int send_back_fd) +int xc_domain_restore(xc_interface *xch, + const struct domain_restore_params *params, + const struct restore_callbacks *callbacks) { errno = ENOSYS; return -1; diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index b1aa88e..f192654 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -187,7 +187,7 @@ struct xc_sr_context int recv_fd; struct xc_sr_save_ops ops; - struct save_callbacks *callbacks; + const struct save_callbacks *callbacks; /* Live migrate vs non live suspend. */ bool live; @@ -214,7 +214,7 @@ struct xc_sr_context struct /* Restore data. */ { struct xc_sr_restore_ops ops; - struct restore_callbacks *callbacks; + const struct restore_callbacks *callbacks; int send_back_fd; unsigned long p2m_size; diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index 00fad7d..51532aa 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -817,32 +817,28 @@ static int restore(struct xc_sr_context *ctx) return rc; } -int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, - unsigned int store_evtchn, unsigned long *store_mfn, - domid_t store_domid, unsigned int console_evtchn, - unsigned long *console_gfn, domid_t console_domid, - unsigned int hvm, unsigned int pae, int superpages, - xc_migration_stream_t stream_type, - struct restore_callbacks *callbacks, int send_back_fd) +int xc_domain_restore(xc_interface *xch, + const struct domain_restore_params *params, + const struct restore_callbacks *callbacks) { xen_pfn_t nr_pfns; struct xc_sr_context ctx = { .xch = xch, - .fd = io_fd, + .fd = params->recv_fd, }; /* GCC 4.4 (of CentOS 6.x vintage) can' t initialise anonymous unions. */ - ctx.restore.console_evtchn = console_evtchn; - ctx.restore.console_domid = console_domid; - ctx.restore.xenstore_evtchn = store_evtchn; - ctx.restore.xenstore_domid = store_domid; - ctx.restore.checkpointed = stream_type; + ctx.restore.console_evtchn = params->console_evtchn; + ctx.restore.console_domid = params->console_domid; + ctx.restore.xenstore_evtchn = params->store_evtchn; + ctx.restore.xenstore_domid = params->store_domid; + ctx.restore.checkpointed = params->stream_type; ctx.restore.callbacks = callbacks; - ctx.restore.send_back_fd = send_back_fd; + ctx.restore.send_back_fd = params->send_back_fd; /* Sanity checks for callbacks. */ - if ( stream_type ) + if ( params->stream_type ) assert(callbacks->checkpoint); if ( ctx.restore.checkpointed == XC_MIG_STREAM_COLO ) @@ -854,28 +850,27 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, callbacks->restore_results); } - DPRINTF("fd %d, dom %u, hvm %u, pae %u, superpages %d" - ", stream_type %d", io_fd, dom, hvm, pae, - superpages, stream_type); - - if ( xc_domain_getinfo(xch, dom, 1, &ctx.dominfo) != 1 ) + if ( xc_domain_getinfo(xch, params->dom, 1, &ctx.dominfo) != 1 ) { PERROR("Failed to get domain info"); return -1; } - if ( ctx.dominfo.domid != dom ) + if ( ctx.dominfo.domid != params->dom ) { - ERROR("Domain %u does not exist", dom); + ERROR("Domain %u does not exist", params->dom); return -1; } - ctx.domid = dom; + DPRINTF("fd %d, dom %u, hvm %d, stream_type %d", params->recv_fd, + params->dom, ctx.dominfo.hvm, params->stream_type); + + ctx.domid = params->dom; if ( read_headers(&ctx) ) return -1; - if ( xc_domain_nr_gpfns(xch, dom, &nr_pfns) < 0 ) + if ( xc_domain_nr_gpfns(xch, ctx.domid, &nr_pfns) < 0 ) { PERROR("Unable to obtain the guest p2m size"); return -1; @@ -906,8 +901,8 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, ctx.restore.console_domid, ctx.restore.console_evtchn); - *console_gfn = ctx.restore.console_gfn; - *store_mfn = ctx.restore.xenstore_gfn; + *params->console_gfn = ctx.restore.console_gfn; + *params->store_gfn = ctx.restore.xenstore_gfn; return 0; } diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index b1a24b7..0ab86c3 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -915,28 +915,26 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) return rc; }; -int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, - uint32_t max_iters, uint32_t max_factor, uint32_t flags, - struct save_callbacks* callbacks, int hvm, - xc_migration_stream_t stream_type, int recv_fd) +int xc_domain_save(xc_interface *xch, const struct domain_save_params *params, + const struct save_callbacks* callbacks) { struct xc_sr_context ctx = { .xch = xch, - .fd = io_fd, + .fd = params->save_fd, }; /* GCC 4.4 (of CentOS 6.x vintage) can' t initialise anonymous unions. */ ctx.save.callbacks = callbacks; - ctx.save.live = !!(flags & XCFLAGS_LIVE); - ctx.save.debug = !!(flags & XCFLAGS_DEBUG); - ctx.save.checkpointed = stream_type; - ctx.save.recv_fd = recv_fd; + ctx.save.live = params->live; + ctx.save.debug = params->debug; + ctx.save.checkpointed = params->stream_type; + ctx.save.recv_fd = params->recv_fd; /* If altering migration_stream update this assert too. */ - assert(stream_type == XC_MIG_STREAM_NONE || - stream_type == XC_MIG_STREAM_REMUS || - stream_type == XC_MIG_STREAM_COLO); + assert(params->stream_type == XC_MIG_STREAM_NONE || + params->stream_type == XC_MIG_STREAM_REMUS || + params->stream_type == XC_MIG_STREAM_COLO); /* * TODO: Find some time to better tweak the live migration algorithm. @@ -947,30 +945,32 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, ctx.save.max_iterations = 5; ctx.save.dirty_threshold = 50; - /* Sanity checks for callbacks. */ - if ( hvm ) - assert(callbacks->switch_qemu_logdirty); - if ( ctx.save.checkpointed ) - assert(callbacks->checkpoint && callbacks->aftercopy); - if ( ctx.save.checkpointed == XC_MIG_STREAM_COLO ) - assert(callbacks->wait_checkpoint); - - DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d", - io_fd, dom, max_iters, max_factor, flags, hvm); - - if ( xc_domain_getinfo(xch, dom, 1, &ctx.dominfo) != 1 ) + if ( xc_domain_getinfo(xch, params->dom, 1, &ctx.dominfo) != 1 ) { PERROR("Failed to get domain info"); return -1; } - if ( ctx.dominfo.domid != dom ) + if ( ctx.dominfo.domid != params->dom ) { - ERROR("Domain %u does not exist", dom); + ERROR("Domain %u does not exist", params->dom); return -1; } - ctx.domid = dom; + /* Sanity checks for callbacks. */ + if ( ctx.dominfo.hvm ) + assert(callbacks->switch_qemu_logdirty); + if ( ctx.save.checkpointed ) + assert(callbacks->checkpoint && callbacks->aftercopy); + if ( ctx.save.checkpointed == XC_MIG_STREAM_COLO ) + assert(callbacks->wait_checkpoint); + + ctx.domid = params->dom; + + DPRINTF("fd %d, dom %u, max_iterations %u, dirty_threshold %u, live %d, " + "debug %d, type %d, hvm %d", ctx.fd, ctx.domid, + ctx.save.max_iterations, ctx.save.dirty_threshold, ctx.save.live, + ctx.save.debug, ctx.save.checkpointed, ctx.dominfo.hvm); if ( ctx.dominfo.hvm ) { diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c index 77fe30e..c27813a 100644 --- a/tools/libxl/libxl_dom_save.c +++ b/tools/libxl/libxl_dom_save.c @@ -338,8 +338,6 @@ void libxl__domain_save(libxl__egc *egc, libxl__domain_save_state *dss) /* Convenience aliases */ const uint32_t domid = dss->domid; const libxl_domain_type type = dss->type; - const int live = dss->live; - const int debug = dss->debug; const libxl_domain_remus_info *const r_info = dss->remus; libxl__srm_save_autogen_callbacks *const callbacks = &dss->sws.shs.callbacks.save.a; @@ -374,10 +372,6 @@ void libxl__domain_save(libxl__egc *egc, libxl__domain_save_state *dss) abort(); } - dss->xcflags = (live ? XCFLAGS_LIVE : 0) - | (debug ? XCFLAGS_DEBUG : 0) - | (dss->hvm ? XCFLAGS_HVM : 0); - /* Disallow saving a guest with vNUMA configured because migration * stream does not preserve node information. * @@ -393,11 +387,6 @@ void libxl__domain_save(libxl__egc *egc, libxl__domain_save_state *dss) goto out; } - if (dss->checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_REMUS) { - if (libxl_defbool_val(r_info->compression)) - dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS; - } - if (dss->checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_NONE) callbacks->suspend = libxl__domain_suspend_callback; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index afe6652..89de86b 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -3306,7 +3306,6 @@ struct libxl__domain_save_state { /* private */ int rc; int hvm; - int xcflags; libxl__domain_suspend_state dsps; union { /* for Remus */ diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c index 46b892c..0852bcf 100644 --- a/tools/libxl/libxl_save_callout.c +++ b/tools/libxl/libxl_save_callout.c @@ -59,10 +59,11 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, const unsigned long argnums[] = { domid, state->store_port, - state->store_domid, state->console_port, + state->store_domid, + state->console_port, state->console_domid, - hvm, pae, superpages, - cbflags, dcs->restore_params.checkpointed_stream, + dcs->restore_params.checkpointed_stream, + cbflags }; shs->ao = ao; @@ -76,7 +77,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, shs->caller_state = dcs; shs->need_results = 1; - run_helper(egc, shs, "--restore-domain", restore_fd, send_back_fd, 0, 0, + run_helper(egc, shs, "--restore-domain", restore_fd, send_back_fd, NULL, 0, argnums, ARRAY_SIZE(argnums)); } @@ -89,8 +90,7 @@ void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_save_state *dss, libxl__srm_callout_enumcallbacks_save(&shs->callbacks.save.a); const unsigned long argnums[] = { - dss->domid, 0, 0, dss->xcflags, dss->hvm, - cbflags, dss->checkpointed_stream, + dss->domid, 0, dss->live, dss->debug, dss->checkpointed_stream, cbflags }; shs->ao = ao; diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c index d3def6b..887b6a2 100644 --- a/tools/libxl/libxl_save_helper.c +++ b/tools/libxl/libxl_save_helper.c @@ -239,7 +239,6 @@ static struct restore_callbacks helper_restore_callbacks; int main(int argc, char **argv) { int r; - int send_back_fd, recv_fd; #define NEXTARG (++argv, assert(*argv), *argv) @@ -248,15 +247,15 @@ int main(int argc, char **argv) if (!strcmp(mode,"--save-domain")) { - io_fd = atoi(NEXTARG); - recv_fd = atoi(NEXTARG); - uint32_t dom = strtoul(NEXTARG,0,10); - uint32_t max_iters = strtoul(NEXTARG,0,10); - uint32_t max_factor = strtoul(NEXTARG,0,10); - uint32_t flags = strtoul(NEXTARG,0,10); - int hvm = atoi(NEXTARG); - unsigned cbflags = strtoul(NEXTARG,0,10); - xc_migration_stream_t stream_type = strtoul(NEXTARG,0,10); + struct domain_save_params params; + params.save_fd = io_fd = atoi(NEXTARG); + params.recv_fd = atoi(NEXTARG); + params.dom = strtoul(NEXTARG,0,10); + params.max_iters = strtoul(NEXTARG,0,10); + params.live = strtoul(NEXTARG,0,10); + params.debug = strtoul(NEXTARG,0,10); + params.stream_type = strtoul(NEXTARG,0,10); + unsigned cbflags = strtoul(NEXTARG,0,10); assert(!*++argv); helper_setcallbacks_save(&helper_save_callbacks, cbflags); @@ -264,41 +263,35 @@ int main(int argc, char **argv) startup("save"); setup_signals(save_signal_handler); - r = xc_domain_save(xch, io_fd, dom, max_iters, max_factor, flags, - &helper_save_callbacks, hvm, stream_type, - recv_fd); + r = xc_domain_save(xch, ¶ms, &helper_save_callbacks); complete(r); } else if (!strcmp(mode,"--restore-domain")) { - io_fd = atoi(NEXTARG); - send_back_fd = atoi(NEXTARG); - uint32_t dom = strtoul(NEXTARG,0,10); - unsigned store_evtchn = strtoul(NEXTARG,0,10); - domid_t store_domid = strtoul(NEXTARG,0,10); - unsigned console_evtchn = strtoul(NEXTARG,0,10); - domid_t console_domid = strtoul(NEXTARG,0,10); - unsigned int hvm = strtoul(NEXTARG,0,10); - unsigned int pae = strtoul(NEXTARG,0,10); - int superpages = strtoul(NEXTARG,0,10); - unsigned cbflags = strtoul(NEXTARG,0,10); - xc_migration_stream_t stream_type = strtoul(NEXTARG,0,10); + xen_pfn_t store_gfn = 0; + xen_pfn_t console_gfn = 0; + + struct domain_restore_params params; + params.recv_fd = io_fd = atoi(NEXTARG); + params.send_back_fd = atoi(NEXTARG); + params.dom = strtoul(NEXTARG,0,10); + params.store_evtchn = strtoul(NEXTARG,0,10); + params.store_gfn = &store_gfn; + params.store_domid = strtoul(NEXTARG,0,10); + params.console_evtchn = strtoul(NEXTARG,0,10); + params.console_gfn = &console_gfn; + params.console_domid = strtoul(NEXTARG,0,10); + params.stream_type = strtoul(NEXTARG,0,10); + unsigned cbflags = strtoul(NEXTARG,0,10); assert(!*++argv); helper_setcallbacks_restore(&helper_restore_callbacks, cbflags); - unsigned long store_mfn = 0; - unsigned long console_mfn = 0; - startup("restore"); setup_signals(SIG_DFL); - r = xc_domain_restore(xch, io_fd, dom, store_evtchn, &store_mfn, - store_domid, console_evtchn, &console_mfn, - console_domid, hvm, pae, superpages, - stream_type, - &helper_restore_callbacks, send_back_fd); - helper_stub_restore_results(store_mfn,console_mfn,0); + r = xc_domain_restore(xch, ¶ms, &helper_restore_callbacks); + helper_stub_restore_results(store_gfn, console_gfn, 0); complete(r); } else { -- 2.7.4 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |