|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4 of 5] tools: set migration constraints from cmdline
On Wed, 2013-03-06 at 16:48 +0000, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@xxxxxxxxx>
> # Date 1362585914 -3600
> # Node ID 29c66a248f5bb153ae6ac157afcdd91c2390c6a9
> # Parent 1ea501d602649c58412f73e83e24115eb3d8fe44
> tools: set migration constraints from cmdline
>
> Add new options to xm/xl migrate to control the process of migration.
> The intention is to optionally abort the migration if it takes too long
> to migrate a busy guest due to the high number of dirty pages. Currently
> the guest is suspended to transfer the remaining dirty pages. This
> transfer can take too long, which can confuse the guest if its suspended
> for too long.
>
> -M <number> Number of iterations before final suspend (default: 30)
Do the defaults here reflect the current behaviour or are they in
themselves a change?
> --max_iters <number>
I'm really not a fan of underscores in command line arguments, but I
suppose there is precedent in --tslice_ms in other places.
>
> -m <factor> Max amount of memory to transfer before final suspend (default:
> 3*RAM)
> --max_factor <factor>
>
> -A Abort migration instead of doing final suspend.
... if <what>
> --abort_if_busy
>
>
>
> The changes to libxl change the API, handle LIBXL_API_VERSION == 0x040200.
>
> TODO:
> eventually add also --min_remaining (default value 50) in a seperate patch
>
> v6:
> - update the LIBXL_API_VERSION handling for libxl_domain_suspend
> change it to an inline function if LIBXL_API_VERSION is defined to 4.2.0
> - rename libxl_save_properties to libxl_domain_suspend_properties
> - rename ->xlflags to ->flags within that struct
>
> v5:
> - adjust libxl_domain_suspend prototype, move flags, max_iters,
> max_factor into a new, optional struct libxl_save_properties
> - rename XCFLAGS_DOMSAVE_NOSUSPEND to XCFLAGS_DOMSAVE_ABORT_IF_BUSY
> - rename LIBXL_SUSPEND_NO_FINAL_SUSPEND to LIBXL_SUSPEND_ABORT_IF_BUSY
> - rename variables no_suspend to abort_if_busy
> - rename option -N/--no_suspend to -A/--abort_if_busy
> - update xl.1, extend description of -A option
>
> v4:
> - update default for no_suspend from None to 0 in XendCheckpoint.py:save
> - update logoutput in setMigrateConstraints
> - change xm migrate defaults from None to 0
> - add new options to xl.1
> - fix syntax error in XendDomain.py:domain_migrate_constraints_set
> - fix xm migrate -N option name to match xl migrate
>
> v3:
> - move logic errors in libxl__domain_suspend and fixed help text in
> cmd_table to separate patches
> - fix syntax error in XendCheckpoint.py
> - really pass max_iters and max_factor in libxl__xc_domain_save
> - make libxl_domain_suspend_0x040200 declaration globally visible
> - bump libxenlight.so SONAME from 2.0 to 2.1 due to changed
> libxl_domain_suspend
>
> v2:
> - use LIBXL_API_VERSION and define libxl_domain_suspend_0x040200
> - fix logic error in min_reached check in xc_domain_save
> - add longopts
> - update --help text
> - correct description of migrate --help text
>
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
>
> diff -r 1ea501d60264 -r 29c66a248f5b docs/man/xl.pod.1
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -391,6 +391,21 @@ Send <config> instead of config file fro
>
> Print huge (!) amount of debug during the migration process.
>
> +=item B<-M> I<number>, B<--max_iters> I<number>
> +
> +Number of iterations before final suspend (default: 30)
> +
> +=item B<-m> I<factor>, B<--max_factor> I<factor>
> +
> +Max amount of memory to transfer before final suspend (default: 3*RAM)
> +
> +=item B<-A>, B<--abort_if_busy>
> +
> +Abort migration instead of doing final suspend/transfer/resume if the
> +guest has still dirty pages after the number of iterations and/or the
> +amount of RAM transfered. This avoids long periods of time were the
transferred where
> +guest is suspended.
> +
> =back
>
> =item B<remus> [I<OPTIONS>] I<domain-id> I<host>
> diff -r 1ea501d60264 -r 29c66a248f5b tools/libxc/xc_domain_save.c
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
> @@ -804,6 +804,7 @@ int xc_domain_save(xc_interface *xch, in
> int rc = 1, frc, i, j, last_iter = 0, iter = 0;
> int live = (flags & XCFLAGS_LIVE);
> int debug = (flags & XCFLAGS_DEBUG);
> + int abort_if_busy = (flags & XCFLAGS_DOMSAVE_ABORT_IF_BUSY);
> int superpages = !!hvm;
> int race = 0, sent_last_iter, skip_this_iter = 0;
> unsigned int sent_this_iter = 0;
> @@ -1527,10 +1528,20 @@ int xc_domain_save(xc_interface *xch, in
>
> if ( live )
> {
> + int min_reached = sent_this_iter + skip_this_iter < 50;
> if ( (iter >= max_iters) ||
> - (sent_this_iter+skip_this_iter < 50) ||
> + min_reached ||
> (total_sent > dinfo->p2m_size*max_factor) )
> {
> + if ( !min_reached && abort_if_busy )
> + {
> + ERROR("Live migration aborted, as requested. (guest too
> busy?)"
> + " total_sent %lu iter %d, max_iters %u max_factor %u",
> + total_sent, iter, max_iters, max_factor);
> + rc = 1;
> + goto out;
> + }
> +
> DPRINTF("Start last iteration\n");
> last_iter = 1;
>
> diff -r 1ea501d60264 -r 29c66a248f5b tools/libxc/xenguest.h
> --- a/tools/libxc/xenguest.h
> +++ b/tools/libxc/xenguest.h
> @@ -28,6 +28,7 @@
> #define XCFLAGS_HVM (1 << 2)
> #define XCFLAGS_STDVGA (1 << 3)
> #define XCFLAGS_CHECKPOINT_COMPRESS (1 << 4)
> +#define XCFLAGS_DOMSAVE_ABORT_IF_BUSY (1 << 5)
>
> #define X86_64_B_SIZE 64
> #define X86_32_B_SIZE 32
> diff -r 1ea501d60264 -r 29c66a248f5b tools/libxl/Makefile
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -5,7 +5,7 @@
> XEN_ROOT = $(CURDIR)/../..
> include $(XEN_ROOT)/tools/Rules.mk
>
> -MAJOR = 2.0
> +MAJOR = 2.1
> MINOR = 0
>
> XLUMAJOR = 1.0
> diff -r 1ea501d60264 -r 29c66a248f5b tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -756,7 +756,8 @@ static void domain_suspend_cb(libxl__egc
>
> }
>
> -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
> +int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> + const libxl_domain_suspend_properties *props,
> const libxl_asyncop_how *ao_how)
> {
> AO_CREATE(ctx, domid, ao_how);
> @@ -777,8 +778,13 @@ int libxl_domain_suspend(libxl_ctx *ctx,
> dss->domid = domid;
> dss->fd = fd;
> dss->type = type;
> - dss->live = flags & LIBXL_SUSPEND_LIVE;
> - dss->debug = flags & LIBXL_SUSPEND_DEBUG;
> + if (props) {
> + dss->live = props->flags & LIBXL_SUSPEND_LIVE;
> + dss->debug = props->flags & LIBXL_SUSPEND_DEBUG;
> + dss->max_iters = props->max_iters;
> + dss->max_factor = props->max_factor;
> + dss->xlflags = props->flags;
> + }
Do these things all get sane defaults if !props? Or is !props
disallowed? (in which case error return required)
>
> libxl__domain_suspend(egc, dss);
> return AO_INPROGRESS;
> @@ -3769,13 +3778,17 @@ int main_migrate(int argc, char **argv)
> char *rune = NULL;
> char *host;
> int opt, daemonize = 1, monitor = 1, debug = 0;
> + int max_iters = 0, max_factor = 0, abort_if_busy = 0;
> static struct option opts[] = {
> {"debug", 0, 0, 0x100},
> + {"max_iters", 1, 0, 'M'},
> + {"max_factor", 1, 0, 'm'},
Wouldn't I or i and F or f be more descriptive than M and m (without
looking, tell me which is which? ;-) )
I wouldn't especially object if these were long only options, I expect
using them will be something only a tiny minority of users even think
of, unless some higher level entity does it for them (in which case the
length of the option is irrelevant).
> + {"abort_if_busy", 0, 0, 'A'},
> COMMON_LONG_OPTS,
> {0, 0, 0, 0}
> };
[...]
> diff -r 1ea501d60264 -r 29c66a248f5b tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -147,14 +147,19 @@ struct cmd_spec cmd_table[] = {
> &main_migrate, 0, 1,
> "Migrate a domain to another host",
> "[options] <Domain> <host>",
> - "-h Print this help.\n"
> - "-C <config> Send <config> instead of config file from creation.\n"
> - "-s <sshcommand> Use <sshcommand> instead of ssh. String will be
> passed\n"
> - " to sh. If empty, run <host> instead of ssh <host>
> xl\n"
> - " migrate-receive [-d -e]\n"
> - "-e Do not wait in the background (on <host>) for the
> death\n"
> - " of the domain.\n"
> - "--debug Print huge (!) amount of debug during the migration
> process."
> + "-h Print this help.\n"
> + "-C <config> Send <config> instead of config file from
> creation.\n"
> + "-s <sshcommand> Use <sshcommand> instead of ssh. String will be
> passed\n"
> + " to sh. If empty, run <host> instead of ssh
> <host> xl\n"
> + " migrate-receive [-d -e]\n"
> + "-e Do not wait in the background (on <host>) for
> the death\n"
> + " of the domain.\n"
> + "--debug Print huge (!) amount of debug during the
> migration process.\n"
> + "-M <number> Number of iterations before final suspend
> (default: 30)\n"
> + "--max_iters <number>\n"
> + "-m <factor> Max amount of memory to transfer before final
> suspend (default: 3*RAM).\n"
> + "--max_factor <factor>\n"
> + "-A, --abort_if_busy Abort migration instead of doing final suspend."
As with other similar locations the text of -A needs to say under what
conditions it aborts, I think.
> },
> { "dump-core",
> &main_dump_core, 0, 1,
> diff -r 1ea501d60264 -r 29c66a248f5b tools/python/xen/xend/XendCheckpoint.py
> --- a/tools/python/xen/xend/XendCheckpoint.py
> +++ b/tools/python/xen/xend/XendCheckpoint.py
...
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |