|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4] tools: set migration constraints from cmdline
On Fri, 2013-02-01 at 19:34 +0000, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@xxxxxxxxx>
> # Date 1359746832 -3600
> # Node ID 785c8f34e1f802106e53ca4d2c54dce55c8ee166
> # Parent fd711ebdce9a58556d62c2daaf5d49ab06de4a3c
> 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)
> --max_iters <number>
>
> -m <factor> Max amount of memory to transfer before final suspend (default:
> 3*RAM)
> --max_factor <factor>
>
> -N Abort migration instead of doing final suspend.
> --no_suspend
Is this last one a debug option? (Having read the patch I think not, but
the description here doesn't really explain it)
> The changes to libxl change the API, is that approach acceptable?
I think this is now along the right lines (see below though).
> 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 fd711ebdce9a -r 785c8f34e1f8 docs/man/xl.pod.1
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -391,6 +391,18 @@ 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<-N>, B<--no_suspend>
> +
> +Abort migration instead of doing final suspend.
> +
> =back
>
> =item B<remus> [I<OPTIONS>] I<domain-id> I<host>
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxc/xc_domain_save.c
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
The changes to this file only seem to implement the -N and not the other
two?
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
> void libxl_domain_config_init(libxl_domain_config *d_config);
> void libxl_domain_config_dispose(libxl_domain_config *d_config);
>
> -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> +int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
> int flags, /* LIBXL_SUSPEND_* */
> const libxl_asyncop_how *ao_how)
> LIBXL_EXTERNAL_CALLERS_ONLY;
> +#ifdef LIBXL_API_VERSION
> +#if LIBXL_API_VERSION == 0x040200
> +#define libxl_domain_suspend libxl_domain_suspend_0x040200
int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
int flags, /* LIBXL_SUSPEND_* */
int max_iters, int max_factor,
const libxl_asyncop_how *ao_how)
LIBXL_EXTERNAL_CALLERS_ONLY;
#ifdef LIBXL_API_VERSION
#if LIBXL_API_VERSION == 0x040200
#define libxl_domain_suspend(ctx, domid, fd, flags, ao_how) \
libxl_domain_suspend(ctx, domid, fd, flags, 0, 0, ao_how)
#endif /* LIBXL_API_VERSION == 0x040200 */
#endif /* defined(LIBXL_API_VERSION) */
Should work?
Not sure if
#if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION == 0x040200
works in CPP.
Maybe we should
#ifndef LIBXL_API_VERSION
#define LIBXL_API_VERSION LIBXL_API_VERSION_LATEST
#endif
?
> +
> #define LIBXL_SUSPEND_DEBUG 1
> #define LIBXL_SUSPEND_LIVE 2
> +#define LIBXL_SUSPEND_NO_FINAL_SUSPEND 4
(These should really be in the IDL, but this is a pre-existing
condition)
The name of this new option doesn't quite describe what it does, since
it doesn't disable the final suspend always, only under certain
condition.
LIBXL_SUSPEND_ABORT_ON_<xxx>
Where the <xxx> is tricky ;-)
<xxx> == DIRTYING_TOO_FAST
<xxx> == GUEST_TOO_BUSY
<xxx> == YOUR_SUGGESTIONS_HERE
>
> /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
> * If this parameter is true, use co-operative resume. The guest
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
>
> dss->xcflags = (live ? XCFLAGS_LIVE : 0)
> | (debug ? XCFLAGS_DEBUG : 0)
> + | (dss->xlflags & LIBXL_SUSPEND_NO_FINAL_SUSPEND ?
> XCFLAGS_DOMSAVE_NOSUSPEND : 0)
> | (dss->hvm ? XCFLAGS_HVM : 0);
>
> dss->suspend_eventchn = -1;
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
>
> save_domain_core_writeconfig(fd, filename, config_data, config_len);
>
> - int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
> + int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0, NULL);
Doesn't this need to pass down the selected values?
> @@ -3753,8 +3757,16 @@ int main_migrate(int argc, char **argv)
> char *rune = NULL;
> char *host;
> int opt, daemonize = 1, monitor = 1, debug = 0;
> -
> - SWITCH_FOREACH_OPT(opt, "FC:s:ed", NULL, "migrate", 2) {
> + int max_iters = 0, max_factor = 0, no_suspend = 0;
> + static struct option opts[] = {
> + {"max_iters", 1, 0, 'M'},
> + {"max_factor", 1, 0, 'm'},
> + {"no_suspend", 0, 0, 'N'},
I think _ in arguments is pretty ugly... But I see we have others
already.
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/python/xen/xend/XendCheckpoint.py
> --- a/tools/python/xen/xend/XendCheckpoint.py
> +++ b/tools/python/xen/xend/XendCheckpoint.py
I didn't look at any of the python stuff.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |