[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 3/4] Add a 'send-process-signal' command to virsh



> * tools/virsh.c: Add send-process-signal
> * tools/virsh.pod: Document new command
> 
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  tools/virsh-domain.c | 103
>  +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/virsh.pod      |  27 ++++++++++++++
>  2 files changed, 130 insertions(+)

> +VIR_ENUM_DECL(virDomainProcessSignal)
> +VIR_ENUM_IMPL(virDomainProcessSignal,
> +              VIR_DOMAIN_PROCESS_SIGNAL_LAST,
> +              "nop", "hup", "int", "quit", "ill", /* 0-4 */
> +              "trap", "abrt", "bus", "fpe", "kill", /* 5-9 */
> +              "usr1", "segv", "usr2", "pipe", "alrm", /* 10-14 */
> +              "term", "stkflt", "chld", "cont", "stop", /* 15-19 */
> +              "tstp", "ttin", "ttou", "urg", "xcpu", /* 20-24 */
> +              "xfsz", "vtalrm", "prof", "winch", "poll", /* 25-29 */
> +              "pwr", "sys", "rt0","rt1", "rt2",  /* 30-34 */

This doesn't include rtmin...

> +
> +    if (STRPREFIX(signame, "sig"))
> +        signame += 3;
> +    else if (STRPREFIX(signame, "sig_"))
> +        signame += 4;

The else will never be reached.  Swap these two conditions.

> +
> +    if ((signum = virDomainProcessSignalTypeFromString(signame)) >=
> 0)
> +        goto cleanup;
> +

...and you lost your handling of rtmin+1 here...


> +++ b/tools/virsh.pod
> @@ -1393,6 +1393,33 @@ B<Examples>
>    # send a tab, held for 1 second
>    virsh send-key --holdtime 1000 0xf
>  
> +=item B<send-process-signal> I<domain-id> [I<--host-pid>] I<pid>
> I<signame>

--host-pid is unsupported; drop it from the synopsis.


> +The I<signame> argument may be either an integer signal constant
> number,
> +or one of the symbolic names:

Maybe mention case-insensitive.

> +
> +    "nop", "hup", "int", "quit", "ill",
> +    "trap", "abrt", "bus", "fpe", "kill",
> +    "usr1", "segv", "usr2", "pipe", "alrm",
> +    "term", "stkflt", "chld", "cont", "stop",
> +    "tstp", "ttin", "ttou", "urg", "xcpu",
> +    "xfsz", "vtalrm", "prof", "winch", "poll",
> +    "pwr", "sys", "rtmin"

This list doesn't mention rt0 and friends.

> +
> +The symbol name may optionally be prefixed with 'sig'. The 'rtmin'
> signal
> +also allows an optional suffix "+<number>" to refer to other real
> time
> +signals.

...so this part of the docs no longer matches the code...

> +
> +B<Examples>
> +  virsh send-process-signal myguest 1 15
> +  virsh send-process-signal myguest 1 term
> +  virsh send-process-signal myguest 1 sigterm
> +  virsh send-process-signal myguest 1 rtmin+12

...and this last example  needs fixing.

But I'm okay giving ACK; I trust you to clean things
up appropriately, and this patch only touches virsh.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]