[PATCH] docs/man: add shutdown reasons to xl (list) man page

[PATCH] docs/man: add shutdown reasons to xl (list) man page

[zithro / Cyril Rébert]

Add the shutdown reasons to the paragraph of "xl list" concerning the
shutdown status.
I have copy/pasted the explanations from the source code :

 - tools/xl/xl_info.c (L379)
 - xen/include/public/sched.h (starting L158)

Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Cyril Rébert / zithro <slack@rabbit.lu>
---
 docs/man/xl.1.pod.in | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index bed8393473..2b046f97f1 100644
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -370,6 +370,40 @@ scheduling by the Xen hypervisor.
 The guest OS has shut down (SCHEDOP_shutdown has been called) but the
 domain is not dying yet.
 
+There are six shutdown reasons, which will be displayed after the "s" : B<-rscwS>.
+
+=over 4
+
+=over 4
+
+=item s- : poweroff
+
+Domain exited normally. Clean up and kill.
+
+=item sr : reboot
+
+Clean up, kill, and then restart.
+
+=item ss : suspend
+
+The domain is suspended. Clean up, save suspend info, kill.
+
+=item sc : crash
+
+Tell controller we've crashed.
+
+=item sw : watchdog
+
+Restart because watchdog time expired.
+
+=item sS : soft_reset
+
+Domain asked to perform 'soft reset' for it. The expected behavior is to reset internal Xen state for the domain returning it to the point where it was created but leaving the domain's memory contents and vCPU contexts intact. This will allow the domain to start over and set up all Xen specific interfaces again.
+
+=back
+
+=back
+
 =item B<c - crashed>
 
 The domain has crashed, which is always a violent ending.  Usually
-- 
2.39.2

Re: [PATCH] docs/man: add shutdown reasons to xl (list) man page

[Anthony PERARD]

On Tue, Mar 05, 2024 at 11:45:13PM +0100, zithro / Cyril Rébert wrote:
> Add the shutdown reasons to the paragraph of "xl list" concerning the
> shutdown status.
> I have copy/pasted the explanations from the source code :
> 
>  - tools/xl/xl_info.c (L379)

Instead of a line number, how about the function name?
 - tools/xl/xl_info.c (list_domains())

>  - xen/include/public/sched.h (starting L158)

And here, I think that would be "sched_shutdown_reason".

Line number tend to change as we add more code, which mean that the line
number is only valid at the time it is written into the patch
description. But functions and struct name are less likely to be
renamed.

> 
> Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Cyril Rébert / zithro <slack@rabbit.lu>
> ---
>  docs/man/xl.1.pod.in | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
> index bed8393473..2b046f97f1 100644
> --- a/docs/man/xl.1.pod.in
> +++ b/docs/man/xl.1.pod.in
> @@ -370,6 +370,40 @@ scheduling by the Xen hypervisor.
>  The guest OS has shut down (SCHEDOP_shutdown has been called) but the
>  domain is not dying yet.
>  
> +There are six shutdown reasons, which will be displayed after the "s" : B<-rscwS>.
> +
> +=over 4
> +
> +=over 4
> +
> +=item s- : poweroff

It might look nicer to keep using B<> like the other states, that is
B<s- : poweroff>.

> +
> +Domain exited normally. Clean up and kill.

I don't think we should copy the explanation from "sched.h", they seems
mostly been written for someone writing a toolstack.

A user of `xl` isn't expected to "clean up" an domain in that state,
beside maybe running `xl destroy` because they've added
"on_poweroff=preserve".

> +
> +=item sr : reboot
> +
> +Clean up, kill, and then restart.

Same thing here as an other example, the only action a user can do I
think is `xl destroy`. The "restart" bit is definitely targeted at the
toolstack. There's no way to restart a vm, beside `xl destroy; xl
create`.

> +=item ss : suspend
> +
> +The domain is suspended. Clean up, save suspend info, kill.
> +
> +=item sc : crash
> +
> +Tell controller we've crashed.
> +
> +=item sw : watchdog
> +
> +Restart because watchdog time expired.
> +
> +=item sS : soft_reset
> +
> +Domain asked to perform 'soft reset' for it. The expected behavior is to reset internal Xen state for the domain returning it to the point where it was created but leaving the domain's memory contents and vCPU contexts intact. This will allow the domain to start over and set up all Xen specific interfaces again.
> +
> +=back
> +
> +=back
> +
>  =item B<c - crashed>

This entry is now a duplicate of "sc", I think we can remove it. And
probably keep the explanation.

>  The domain has crashed, which is always a violent ending.  Usually

Thanks,

-- 
Anthony PERARD

Re: [PATCH] docs/man: add shutdown reasons to xl (list) man page

[zithro]

On 06 Mar 2024 16:43, Anthony PERARD wrote:
> On Tue, Mar 05, 2024 at 11:45:13PM +0100, zithro / Cyril Rébert wrote:
>> Add the shutdown reasons to the paragraph of "xl list" concerning the
>> shutdown status.
>> I have copy/pasted the explanations from the source code :
>>
>>   - tools/xl/xl_info.c (L379)
> 
> Instead of a line number, how about the function name?
>   - tools/xl/xl_info.c (list_domains())
> 
>>   - xen/include/public/sched.h (starting L158)
> 
> And here, I think that would be "sched_shutdown_reason".
> 
> Line number tend to change as we add more code, which mean that the line
> number is only valid at the time it is written into the patch
> description. But functions and struct name are less likely to be
> renamed.

Agreed, and for all other remarks too, so will send a v2.
(I'll also add Roger's remark about wrapping the long line).

> <snip>

Re: [PATCH] docs/man: add shutdown reasons to xl (list) man page

[Roger Pau Monné]

On Tue, Mar 05, 2024 at 11:45:13PM +0100, zithro / Cyril Rébert wrote:
> Add the shutdown reasons to the paragraph of "xl list" concerning the
> shutdown status.
> I have copy/pasted the explanations from the source code :
> 
>  - tools/xl/xl_info.c (L379)
>  - xen/include/public/sched.h (starting L158)
> 
> Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Cyril Rébert / zithro <slack@rabbit.lu>

Thanks for doing this, Anthony already provided some feedback.

> ---
>  docs/man/xl.1.pod.in | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
> index bed8393473..2b046f97f1 100644
> --- a/docs/man/xl.1.pod.in
> +++ b/docs/man/xl.1.pod.in
> @@ -370,6 +370,40 @@ scheduling by the Xen hypervisor.
>  The guest OS has shut down (SCHEDOP_shutdown has been called) but the
>  domain is not dying yet.
>  
> +There are six shutdown reasons, which will be displayed after the "s" : B<-rscwS>.
> +
> +=over 4
> +
> +=over 4
> +
> +=item s- : poweroff
> +
> +Domain exited normally. Clean up and kill.
> +
> +=item sr : reboot
> +
> +Clean up, kill, and then restart.
> +
> +=item ss : suspend
> +
> +The domain is suspended. Clean up, save suspend info, kill.
> +
> +=item sc : crash
> +
> +Tell controller we've crashed.
> +
> +=item sw : watchdog
> +
> +Restart because watchdog time expired.
> +
> +=item sS : soft_reset
> +
> +Domain asked to perform 'soft reset' for it. The expected behavior is to reset internal Xen state for the domain returning it to the point where it was created but leaving the domain's memory contents and vCPU contexts intact. This will allow the domain to start over and set up all Xen specific interfaces again.

The man page sources are kept at 80 columns, can you please wrap this
text so it doesn't exceed the 80 col limit?

Roger.