diff mbox series

[v20210601,02/38] xl: fix description of migrate --debug

Message ID 20210601161118.18986-3-olaf@aepfle.de (mailing list archive)
State Superseded
Headers show
Series leftover from 2020 | expand

Commit Message

Olaf Hering June 1, 2021, 4:10 p.m. UTC
xl migrate --debug used to track every pfn in every batch of pages.
But these times are gone. Adjust the help text to tell what --debug
is supposed to do today.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 docs/man/xl.1.pod.in          | 4 +++-
 tools/libs/guest/xg_sr_save.c | 2 +-
 tools/xl/xl_cmdtable.c        | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

Comments

Jürgen Groß June 2, 2021, 6:09 a.m. UTC | #1
On 01.06.21 18:10, Olaf Hering wrote:
> xl migrate --debug used to track every pfn in every batch of pages.
> But these times are gone. Adjust the help text to tell what --debug
> is supposed to do today.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>   docs/man/xl.1.pod.in          | 4 +++-
>   tools/libs/guest/xg_sr_save.c | 2 +-
>   tools/xl/xl_cmdtable.c        | 2 +-
>   3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
> index e2176bd696..ed3f4dee1e 100644
> --- a/docs/man/xl.1.pod.in
> +++ b/docs/man/xl.1.pod.in
> @@ -481,7 +481,9 @@ domain.
>   
>   =item B<--debug>
>   
> -Display huge (!) amount of debug information during the migration process.
> +Verify transferred domU page data. All memory will be transferred one more
> +time to the destination host while the domU is paused, and compared with
> +the result of the inital transfer while the domU was still running.

Shouldn't you adapt (or remove?) this paragraph with patch 37?

>   
>   =item B<-p>
>   
> diff --git a/tools/libs/guest/xg_sr_save.c b/tools/libs/guest/xg_sr_save.c
> index 2ba7c3200c..51542a98c8 100644
> --- 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 )

This is no documentation change IMO. You should either mention this
modification in the commit message, or put it into a separate patch.


Juergen
Olaf Hering June 2, 2021, 10:43 a.m. UTC | #2
Am Wed, 2 Jun 2021 08:09:00 +0200
schrieb Juergen Gross <jgross@suse.com>:

> > -    if ( ctx->save.debug && ctx->stream_type != XC_STREAM_PLAIN )
> > +    if ( ctx->save.debug )  
> This is no documentation change IMO. You should either mention this
> modification in the commit message, or put it into a separate patch.

I think the conclusion was that this branch is dead code because
only the stream_type==XC_STREAM_PLAIN code branch sets debug.


So far there is a decision to be made about this code branch:
- document it
- remove it
- fix it

The latter might be impossible due to lack of APIs to query the usage of a given page.

But, perhaps remus and/or colo does not suffer from apparent corruption.
One day I should create a remus and a colo setup to exercise these code paths. 


Olaf
Jürgen Groß June 2, 2021, 11:43 a.m. UTC | #3
On 02.06.21 12:43, Olaf Hering wrote:
> Am Wed, 2 Jun 2021 08:09:00 +0200
> schrieb Juergen Gross <jgross@suse.com>:
> 
>>> -    if ( ctx->save.debug && ctx->stream_type != XC_STREAM_PLAIN )
>>> +    if ( ctx->save.debug )
>> This is no documentation change IMO. You should either mention this
>> modification in the commit message, or put it into a separate patch.
> 
> I think the conclusion was that this branch is dead code because
> only the stream_type==XC_STREAM_PLAIN code branch sets debug.
> 
> 
> So far there is a decision to be made about this code branch:
> - document it
> - remove it
> - fix it

I don't mind either way. I just pointed out an inconsistency between
patch title and content. When searching for a patch causing a regression
I usually skip documentation patches.


Juergen
diff mbox series

Patch

diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index e2176bd696..ed3f4dee1e 100644
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -481,7 +481,9 @@  domain.
 
 =item B<--debug>
 
-Display huge (!) amount of debug information during the migration process.
+Verify transferred domU page data. All memory will be transferred one more
+time to the destination host while the domU is paused, and compared with
+the result of the inital transfer while the domU was still running.
 
 =item B<-p>
 
diff --git a/tools/libs/guest/xg_sr_save.c b/tools/libs/guest/xg_sr_save.c
index 2ba7c3200c..51542a98c8 100644
--- 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 )
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 661323d488..6fd18856c0 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -172,7 +172,7 @@  const 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"
+      "--debug         Verify transferred domU page data.\n"
       "-p              Do not unpause domain after migrating it.\n"
       "-D              Preserve the domain id"
     },