| Message ID | 20210209154536.10851-5-olaf@aepfle.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools changes | expand |
Olaf Hering writes ("[PATCH v20210209 4/4] xl: disable --debug option for xl migrate"):
> xl migrate --debug used to track every pfn in every batch of pages.
>
> Since commit cfa955591caea5d7ec505cdcbf4442f2d6e889e1 from Xen 4.6 the
> debug flag changed meaning from "verify transferred memory during live
> migration" to "verify transferred memory in remus/colo". At least xl
> will not be able to trigger execution of the verifying code in
> send_domain_memory_live anymore.
>
> Remove "--debug" from "xl migrate -h" output.
> Remove "--debug" from from xl man page.
> Do not send "-d" as potential option for "xl migrate-receive" anymore.
> Do not pass the flag LIBXL_SUSPEND_DEBUG anymore to libxl_domain_suspend.
> Continue to recognize "--debug" as valid option for "xl migrate".
> Continue to recognize "-d" as valid option for "xl migrate-receive".
It seems to me that something is definitely a bug here but I want to
understand from Andy what the best thing to do is. I'm hesitant to
release-ack removing this at this stage.
Wouldn't it be better to just fix the docs like in your previously
suggested patch ?
Also, Olaf, please CC Andy on these migration-related patches.
Thanks,
Ian.
Am Tue, 9 Feb 2021 17:12:28 +0000
schrieb Ian Jackson <iwj@xenproject.org>:
> Also, Olaf, please CC Andy on these migration-related patches.
Can this be automated via MAINTAINERS, so that scripts/get_maintainer.pl addresses the people who feel responsible for it? Right now it ends up in your queue due to 'tools/*'.
Olaf
Am Tue, 9 Feb 2021 17:12:28 +0000 schrieb Ian Jackson <iwj@xenproject.org>: > It seems to me that something is definitely a bug here but I want to > understand from Andy what the best thing to do is. I'm hesitant to > release-ack removing this at this stage. > > Wouldn't it be better to just fix the docs like in your previously > suggested patch ? To make that initial patch accurate, this additional change is required: --- a/tools/libs/guest/xg_sr_save.c +++ b/tools/libs/guest/xg_sr_save.c @@ -752,7 +752,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx) if ( rc ) goto out; - if ( ctx->save.debug && ctx->stream_type != XC_STREAM_PLAIN ) + if ( ctx->save.debug ) { rc = verify_frames(ctx); if ( rc ) Otherwise there is no way for "xl" to trigger a call into verify_frames. I will test this patch today. Olaf
Am Wed, 10 Feb 2021 10:06:06 +0100 schrieb Olaf Hering <olaf@aepfle.de>: > - if ( ctx->save.debug && ctx->stream_type != XC_STREAM_PLAIN ) > + if ( ctx->save.debug ) This will do the verification, and finds many errors: 2021-02-10 02:37:03 MST [2149] xc: error: verify pfn 0xfda9 failed (type 0): Internal error As Andrew said elsewhere, something writes into the memory of the idle, but suspended domU. Likely the netback, since it can not know that the domU will never come back. No idea if verify_frames would have ways to figure out what purpose a given pfn really has. Olaf
diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in index e2176bd696..b14196ccfe 100644 --- a/docs/man/xl.1.pod.in +++ b/docs/man/xl.1.pod.in @@ -479,10 +479,6 @@ domain. See the corresponding option of the I<create> subcommand. Send the specified <config> file instead of the file used on creation of the domain. -=item B<--debug> - -Display huge (!) amount of debug information during the migration process. - =item B<-p> Leave the domain on the receive side paused after migration. diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c index 07f54daabe..150f4cd1d3 100644 --- a/tools/xl/xl_cmdtable.c +++ b/tools/xl/xl_cmdtable.c @@ -171,7 +171,6 @@ struct cmd_spec cmd_table[] = { " 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" "-p Do not unpause domain after migrating it.\n" "-D Preserve the domain id" }, diff --git a/tools/xl/xl_migrate.c b/tools/xl/xl_migrate.c index b8594f44a5..e4e4f918c7 100644 --- a/tools/xl/xl_migrate.c +++ b/tools/xl/xl_migrate.c @@ -177,8 +177,7 @@ static void migrate_do_preamble(int send_fd, int recv_fd, pid_t child, } static void migrate_domain(uint32_t domid, int preserve_domid, - const char *rune, int debug, - const char *override_config_file) + const char *rune, const char *override_config_file) { pid_t child = -1; int rc; @@ -204,8 +203,6 @@ static void migrate_domain(uint32_t domid, int preserve_domid, xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS, 0); - if (debug) - flags |= LIBXL_SUSPEND_DEBUG; rc = libxl_domain_suspend(ctx, domid, send_fd, flags, NULL); if (rc) { fprintf(stderr, "migration sender: libxl_domain_suspend failed" @@ -500,6 +497,7 @@ int main_migrate_receive(int argc, char **argv) monitor = 0; break; case 'd': + /* For compatibility with older variants of xl */ debug = 1; break; case 'r': @@ -537,7 +535,7 @@ int main_migrate(int argc, char **argv) const char *ssh_command = "ssh"; char *rune = NULL; char *host; - int opt, daemonize = 1, monitor = 1, debug = 0, pause_after_migration = 0; + int opt, daemonize = 1, monitor = 1, pause_after_migration = 0; int preserve_domid = 0; static struct option opts[] = { {"debug", 0, 0, 0x100}, @@ -566,7 +564,7 @@ int main_migrate(int argc, char **argv) preserve_domid = 1; break; case 0x100: /* --debug */ - debug = 1; + /* ignored for compatibility with older variants of xl */ break; case 0x200: /* --live */ /* ignored for compatibility with xm */ @@ -592,17 +590,16 @@ int main_migrate(int argc, char **argv) } else { verbose_len = (minmsglevel_default - minmsglevel) + 2; } - xasprintf(&rune, "exec %s %s xl%s%s%.*s migrate-receive%s%s%s", + xasprintf(&rune, "exec %s %s xl%s%s%.*s migrate-receive%s%s", ssh_command, host, pass_tty_arg ? " -t" : "", timestamps ? " -T" : "", verbose_len, verbose_buf, daemonize ? "" : " -e", - debug ? " -d" : "", pause_after_migration ? " -p" : ""); } - migrate_domain(domid, preserve_domid, rune, debug, config_filename); + migrate_domain(domid, preserve_domid, rune, config_filename); return EXIT_SUCCESS; }
xl migrate --debug used to track every pfn in every batch of pages. Since commit cfa955591caea5d7ec505cdcbf4442f2d6e889e1 from Xen 4.6 the debug flag changed meaning from "verify transferred memory during live migration" to "verify transferred memory in remus/colo". At least xl will not be able to trigger execution of the verifying code in send_domain_memory_live anymore. Remove "--debug" from "xl migrate -h" output. Remove "--debug" from from xl man page. Do not send "-d" as potential option for "xl migrate-receive" anymore. Do not pass the flag LIBXL_SUSPEND_DEBUG anymore to libxl_domain_suspend. Continue to recognize "--debug" as valid option for "xl migrate". Continue to recognize "-d" as valid option for "xl migrate-receive". Signed-off-by: Olaf Hering <olaf@aepfle.de> --- docs/man/xl.1.pod.in | 4 ---- tools/xl/xl_cmdtable.c | 1 - tools/xl/xl_migrate.c | 15 ++++++--------- 3 files changed, 6 insertions(+), 14 deletions(-)