* [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal
@ 2024-08-07 17:23 Claudio Fontana
2024-08-12 15:33 ` Stefan Hajnoczi
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Claudio Fontana @ 2024-08-07 17:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, Stefan Hajnoczi, Claudio Fontana, Dario Faggioli,
Fabiano Rosas
For the -l and -L options (logging mode), replace the use of the
KeyboardInterrupt exception to gracefully terminate in favor
of handling the SIGINT and SIGTERM signals.
This allows the program to be run from scripts and still be
signaled to gracefully terminate without an interactive terminal.
Before this change, something like this script:
kvm_stat -p 85896 -d -t -s 1 -c -L kvm_stat_85896.csv &
sleep 10
pkill -TERM -P $$
would yield an empty log:
-rw-r--r-- 1 root root 0 Aug 7 16:17 kvm_stat_85896.csv
after this commit:
-rw-r--r-- 1 root root 13466 Aug 7 16:57 kvm_stat_85896.csv
Signed-off-by: Claudio Fontana <cfontana@suse.de>
Cc: Dario Faggioli <dfaggioli@suse.com>
Cc: Fabiano Rosas <farosas@suse.de>
---
tools/kvm/kvm_stat/kvm_stat | 64 ++++++++++++++++-----------------
tools/kvm/kvm_stat/kvm_stat.txt | 12 +++++++
2 files changed, 44 insertions(+), 32 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 15bf00e79e3f..2cf2da3ed002 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -297,8 +297,6 @@ IOCTL_NUMBERS = {
'RESET': 0x00002403,
}
-signal_received = False
-
ENCODING = locale.getpreferredencoding(False)
TRACE_FILTER = re.compile(r'^[^\(]*$')
@@ -1598,7 +1596,19 @@ class CSVFormat(object):
def log(stats, opts, frmt, keys):
"""Prints statistics as reiterating key block, multiple value blocks."""
- global signal_received
+ signal_received = defaultdict(bool)
+
+ def handle_signal(sig, frame):
+ nonlocal signal_received
+ signal_received[sig] = True
+ return
+
+
+ signal.signal(signal.SIGINT, handle_signal)
+ signal.signal(signal.SIGTERM, handle_signal)
+ if opts.log_to_file:
+ signal.signal(signal.SIGHUP, handle_signal)
+
line = 0
banner_repeat = 20
f = None
@@ -1624,39 +1634,31 @@ def log(stats, opts, frmt, keys):
do_banner(opts)
banner_printed = True
while True:
- try:
- time.sleep(opts.set_delay)
- if signal_received:
- banner_printed = True
- line = 0
- f.close()
- do_banner(opts)
- signal_received = False
- if (line % banner_repeat == 0 and not banner_printed and
- not (opts.log_to_file and isinstance(frmt, CSVFormat))):
- do_banner(opts)
- banner_printed = True
- values = stats.get()
- if (not opts.skip_zero_records or
- any(values[k].delta != 0 for k in keys)):
- do_statline(opts, values)
- line += 1
- banner_printed = False
- except KeyboardInterrupt:
+ time.sleep(opts.set_delay)
+ # Do not use the KeyboardInterrupt exception, because we may be running without a terminal
+ if (signal_received[signal.SIGINT] or signal_received[signal.SIGTERM]):
break
+ if signal_received[signal.SIGHUP]:
+ banner_printed = True
+ line = 0
+ f.close()
+ do_banner(opts)
+ signal_received[signal.SIGHUP] = False
+ if (line % banner_repeat == 0 and not banner_printed and
+ not (opts.log_to_file and isinstance(frmt, CSVFormat))):
+ do_banner(opts)
+ banner_printed = True
+ values = stats.get()
+ if (not opts.skip_zero_records or
+ any(values[k].delta != 0 for k in keys)):
+ do_statline(opts, values)
+ line += 1
+ banner_printed = False
if opts.log_to_file:
f.close()
-def handle_signal(sig, frame):
- global signal_received
-
- signal_received = True
-
- return
-
-
def is_delay_valid(delay):
"""Verify delay is in valid value range."""
msg = None
@@ -1869,8 +1871,6 @@ def main():
sys.exit(0)
if options.log or options.log_to_file:
- if options.log_to_file:
- signal.signal(signal.SIGHUP, handle_signal)
keys = sorted(stats.get().keys())
if options.csv:
frmt = CSVFormat(keys)
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index 3a9f2037bd23..4a99a111a93c 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -115,6 +115,18 @@ OPTIONS
--skip-zero-records::
omit records with all zeros in logging mode
+
+SIGNALS
+-------
+when kvm_stat is running in logging mode (either with -l or with -L),
+it handles the following signals:
+
+SIGHUP - closes and reopens the log file (-L only), then continues.
+
+SIGINT - closes the log file and terminates.
+SIGTERM - closes the log file and terminates.
+
+
SEE ALSO
--------
'perf'(1), 'trace-cmd'(1)
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal
2024-08-07 17:23 [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal Claudio Fontana
@ 2024-08-12 15:33 ` Stefan Hajnoczi
2024-10-21 13:26 ` Juergen Gross
2025-04-07 11:03 ` Juergen Gross
2 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2024-08-12 15:33 UTC (permalink / raw)
To: Claudio Fontana; +Cc: Paolo Bonzini, kvm, Dario Faggioli, Fabiano Rosas
[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]
On Wed, Aug 07, 2024 at 07:23:34PM +0200, Claudio Fontana wrote:
> For the -l and -L options (logging mode), replace the use of the
> KeyboardInterrupt exception to gracefully terminate in favor
> of handling the SIGINT and SIGTERM signals.
>
> This allows the program to be run from scripts and still be
> signaled to gracefully terminate without an interactive terminal.
>
> Before this change, something like this script:
>
> kvm_stat -p 85896 -d -t -s 1 -c -L kvm_stat_85896.csv &
> sleep 10
> pkill -TERM -P $$
>
> would yield an empty log:
> -rw-r--r-- 1 root root 0 Aug 7 16:17 kvm_stat_85896.csv
>
> after this commit:
> -rw-r--r-- 1 root root 13466 Aug 7 16:57 kvm_stat_85896.csv
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> Cc: Dario Faggioli <dfaggioli@suse.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> ---
> tools/kvm/kvm_stat/kvm_stat | 64 ++++++++++++++++-----------------
> tools/kvm/kvm_stat/kvm_stat.txt | 12 +++++++
> 2 files changed, 44 insertions(+), 32 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal
2024-08-07 17:23 [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal Claudio Fontana
2024-08-12 15:33 ` Stefan Hajnoczi
@ 2024-10-21 13:26 ` Juergen Gross
2024-11-27 16:30 ` Claudio Fontana
2025-04-07 11:03 ` Juergen Gross
2 siblings, 1 reply; 7+ messages in thread
From: Juergen Gross @ 2024-10-21 13:26 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, Stefan Hajnoczi, Dario Faggioli, Fabiano Rosas,
Claudio Fontana
[-- Attachment #1.1.1: Type: text/plain, Size: 5449 bytes --]
Any reason not to commit this patch? It has got a Reviewed-by: tag from
Stefan more than 2 months ago...
On 07.08.24 19:23, Claudio Fontana wrote:
> For the -l and -L options (logging mode), replace the use of the
> KeyboardInterrupt exception to gracefully terminate in favor
> of handling the SIGINT and SIGTERM signals.
>
> This allows the program to be run from scripts and still be
> signaled to gracefully terminate without an interactive terminal.
>
> Before this change, something like this script:
>
> kvm_stat -p 85896 -d -t -s 1 -c -L kvm_stat_85896.csv &
> sleep 10
> pkill -TERM -P $$
>
> would yield an empty log:
> -rw-r--r-- 1 root root 0 Aug 7 16:17 kvm_stat_85896.csv
>
> after this commit:
> -rw-r--r-- 1 root root 13466 Aug 7 16:57 kvm_stat_85896.csv
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> Cc: Dario Faggioli <dfaggioli@suse.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> ---
> tools/kvm/kvm_stat/kvm_stat | 64 ++++++++++++++++-----------------
> tools/kvm/kvm_stat/kvm_stat.txt | 12 +++++++
> 2 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index 15bf00e79e3f..2cf2da3ed002 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -297,8 +297,6 @@ IOCTL_NUMBERS = {
> 'RESET': 0x00002403,
> }
>
> -signal_received = False
> -
> ENCODING = locale.getpreferredencoding(False)
> TRACE_FILTER = re.compile(r'^[^\(]*$')
>
> @@ -1598,7 +1596,19 @@ class CSVFormat(object):
>
> def log(stats, opts, frmt, keys):
> """Prints statistics as reiterating key block, multiple value blocks."""
> - global signal_received
> + signal_received = defaultdict(bool)
> +
> + def handle_signal(sig, frame):
> + nonlocal signal_received
> + signal_received[sig] = True
> + return
> +
> +
> + signal.signal(signal.SIGINT, handle_signal)
> + signal.signal(signal.SIGTERM, handle_signal)
> + if opts.log_to_file:
> + signal.signal(signal.SIGHUP, handle_signal)
> +
> line = 0
> banner_repeat = 20
> f = None
> @@ -1624,39 +1634,31 @@ def log(stats, opts, frmt, keys):
> do_banner(opts)
> banner_printed = True
> while True:
> - try:
> - time.sleep(opts.set_delay)
> - if signal_received:
> - banner_printed = True
> - line = 0
> - f.close()
> - do_banner(opts)
> - signal_received = False
> - if (line % banner_repeat == 0 and not banner_printed and
> - not (opts.log_to_file and isinstance(frmt, CSVFormat))):
> - do_banner(opts)
> - banner_printed = True
> - values = stats.get()
> - if (not opts.skip_zero_records or
> - any(values[k].delta != 0 for k in keys)):
> - do_statline(opts, values)
> - line += 1
> - banner_printed = False
> - except KeyboardInterrupt:
> + time.sleep(opts.set_delay)
> + # Do not use the KeyboardInterrupt exception, because we may be running without a terminal
> + if (signal_received[signal.SIGINT] or signal_received[signal.SIGTERM]):
> break
> + if signal_received[signal.SIGHUP]:
> + banner_printed = True
> + line = 0
> + f.close()
> + do_banner(opts)
> + signal_received[signal.SIGHUP] = False
> + if (line % banner_repeat == 0 and not banner_printed and
> + not (opts.log_to_file and isinstance(frmt, CSVFormat))):
> + do_banner(opts)
> + banner_printed = True
> + values = stats.get()
> + if (not opts.skip_zero_records or
> + any(values[k].delta != 0 for k in keys)):
> + do_statline(opts, values)
> + line += 1
> + banner_printed = False
>
> if opts.log_to_file:
> f.close()
>
>
> -def handle_signal(sig, frame):
> - global signal_received
> -
> - signal_received = True
> -
> - return
> -
> -
> def is_delay_valid(delay):
> """Verify delay is in valid value range."""
> msg = None
> @@ -1869,8 +1871,6 @@ def main():
> sys.exit(0)
>
> if options.log or options.log_to_file:
> - if options.log_to_file:
> - signal.signal(signal.SIGHUP, handle_signal)
> keys = sorted(stats.get().keys())
> if options.csv:
> frmt = CSVFormat(keys)
> diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
> index 3a9f2037bd23..4a99a111a93c 100644
> --- a/tools/kvm/kvm_stat/kvm_stat.txt
> +++ b/tools/kvm/kvm_stat/kvm_stat.txt
> @@ -115,6 +115,18 @@ OPTIONS
> --skip-zero-records::
> omit records with all zeros in logging mode
>
> +
> +SIGNALS
> +-------
> +when kvm_stat is running in logging mode (either with -l or with -L),
> +it handles the following signals:
> +
> +SIGHUP - closes and reopens the log file (-L only), then continues.
> +
> +SIGINT - closes the log file and terminates.
> +SIGTERM - closes the log file and terminates.
> +
> +
> SEE ALSO
> --------
> 'perf'(1), 'trace-cmd'(1)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal
2024-10-21 13:26 ` Juergen Gross
@ 2024-11-27 16:30 ` Claudio Fontana
2025-04-03 14:01 ` Claudio Fontana
0 siblings, 1 reply; 7+ messages in thread
From: Claudio Fontana @ 2024-11-27 16:30 UTC (permalink / raw)
To: Juergen Gross, Paolo Bonzini
Cc: kvm, Stefan Hajnoczi, Dario Faggioli, Fabiano Rosas
Hi, according to MAINTAINERS file:
M: Paolo Bonzini <pbonzini@redhat.com>
L: kvm@vger.kernel.org
F: tools/kvm/
Paolo, is kvm_stat something you still care about, or should it be set to unmaintained?
In this case I think someone at SUSE could pick it up.
Ciao,
Claudio
On 10/21/24 15:26, Juergen Gross wrote:
> Any reason not to commit this patch? It has got a Reviewed-by: tag from
> Stefan more than 2 months ago...
>
> On 07.08.24 19:23, Claudio Fontana wrote:
>> For the -l and -L options (logging mode), replace the use of the
>> KeyboardInterrupt exception to gracefully terminate in favor
>> of handling the SIGINT and SIGTERM signals.
>>
>> This allows the program to be run from scripts and still be
>> signaled to gracefully terminate without an interactive terminal.
>>
>> Before this change, something like this script:
>>
>> kvm_stat -p 85896 -d -t -s 1 -c -L kvm_stat_85896.csv &
>> sleep 10
>> pkill -TERM -P $$
>>
>> would yield an empty log:
>> -rw-r--r-- 1 root root 0 Aug 7 16:17 kvm_stat_85896.csv
>>
>> after this commit:
>> -rw-r--r-- 1 root root 13466 Aug 7 16:57 kvm_stat_85896.csv
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> Cc: Dario Faggioli <dfaggioli@suse.com>
>> Cc: Fabiano Rosas <farosas@suse.de>
>> ---
>> tools/kvm/kvm_stat/kvm_stat | 64 ++++++++++++++++-----------------
>> tools/kvm/kvm_stat/kvm_stat.txt | 12 +++++++
>> 2 files changed, 44 insertions(+), 32 deletions(-)
>>
>> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
>> index 15bf00e79e3f..2cf2da3ed002 100755
>> --- a/tools/kvm/kvm_stat/kvm_stat
>> +++ b/tools/kvm/kvm_stat/kvm_stat
>> @@ -297,8 +297,6 @@ IOCTL_NUMBERS = {
>> 'RESET': 0x00002403,
>> }
>>
>> -signal_received = False
>> -
>> ENCODING = locale.getpreferredencoding(False)
>> TRACE_FILTER = re.compile(r'^[^\(]*$')
>>
>> @@ -1598,7 +1596,19 @@ class CSVFormat(object):
>>
>> def log(stats, opts, frmt, keys):
>> """Prints statistics as reiterating key block, multiple value blocks."""
>> - global signal_received
>> + signal_received = defaultdict(bool)
>> +
>> + def handle_signal(sig, frame):
>> + nonlocal signal_received
>> + signal_received[sig] = True
>> + return
>> +
>> +
>> + signal.signal(signal.SIGINT, handle_signal)
>> + signal.signal(signal.SIGTERM, handle_signal)
>> + if opts.log_to_file:
>> + signal.signal(signal.SIGHUP, handle_signal)
>> +
>> line = 0
>> banner_repeat = 20
>> f = None
>> @@ -1624,39 +1634,31 @@ def log(stats, opts, frmt, keys):
>> do_banner(opts)
>> banner_printed = True
>> while True:
>> - try:
>> - time.sleep(opts.set_delay)
>> - if signal_received:
>> - banner_printed = True
>> - line = 0
>> - f.close()
>> - do_banner(opts)
>> - signal_received = False
>> - if (line % banner_repeat == 0 and not banner_printed and
>> - not (opts.log_to_file and isinstance(frmt, CSVFormat))):
>> - do_banner(opts)
>> - banner_printed = True
>> - values = stats.get()
>> - if (not opts.skip_zero_records or
>> - any(values[k].delta != 0 for k in keys)):
>> - do_statline(opts, values)
>> - line += 1
>> - banner_printed = False
>> - except KeyboardInterrupt:
>> + time.sleep(opts.set_delay)
>> + # Do not use the KeyboardInterrupt exception, because we may be running without a terminal
>> + if (signal_received[signal.SIGINT] or signal_received[signal.SIGTERM]):
>> break
>> + if signal_received[signal.SIGHUP]:
>> + banner_printed = True
>> + line = 0
>> + f.close()
>> + do_banner(opts)
>> + signal_received[signal.SIGHUP] = False
>> + if (line % banner_repeat == 0 and not banner_printed and
>> + not (opts.log_to_file and isinstance(frmt, CSVFormat))):
>> + do_banner(opts)
>> + banner_printed = True
>> + values = stats.get()
>> + if (not opts.skip_zero_records or
>> + any(values[k].delta != 0 for k in keys)):
>> + do_statline(opts, values)
>> + line += 1
>> + banner_printed = False
>>
>> if opts.log_to_file:
>> f.close()
>>
>>
>> -def handle_signal(sig, frame):
>> - global signal_received
>> -
>> - signal_received = True
>> -
>> - return
>> -
>> -
>> def is_delay_valid(delay):
>> """Verify delay is in valid value range."""
>> msg = None
>> @@ -1869,8 +1871,6 @@ def main():
>> sys.exit(0)
>>
>> if options.log or options.log_to_file:
>> - if options.log_to_file:
>> - signal.signal(signal.SIGHUP, handle_signal)
>> keys = sorted(stats.get().keys())
>> if options.csv:
>> frmt = CSVFormat(keys)
>> diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
>> index 3a9f2037bd23..4a99a111a93c 100644
>> --- a/tools/kvm/kvm_stat/kvm_stat.txt
>> +++ b/tools/kvm/kvm_stat/kvm_stat.txt
>> @@ -115,6 +115,18 @@ OPTIONS
>> --skip-zero-records::
>> omit records with all zeros in logging mode
>>
>> +
>> +SIGNALS
>> +-------
>> +when kvm_stat is running in logging mode (either with -l or with -L),
>> +it handles the following signals:
>> +
>> +SIGHUP - closes and reopens the log file (-L only), then continues.
>> +
>> +SIGINT - closes the log file and terminates.
>> +SIGTERM - closes the log file and terminates.
>> +
>> +
>> SEE ALSO
>> --------
>> 'perf'(1), 'trace-cmd'(1)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal
2024-11-27 16:30 ` Claudio Fontana
@ 2025-04-03 14:01 ` Claudio Fontana
2025-04-03 14:24 ` Claudio Fontana
0 siblings, 1 reply; 7+ messages in thread
From: Claudio Fontana @ 2025-04-03 14:01 UTC (permalink / raw)
To: Juergen Gross, Paolo Bonzini
Cc: kvm, Stefan Hajnoczi, Dario Faggioli, Fabiano Rosas
Gentle ping,
we currently need to carry this downstream, could you include this fix?
Thanks,
CLaudio
On 11/27/24 17:30, Claudio Fontana wrote:
> Hi, according to MAINTAINERS file:
>
> M: Paolo Bonzini <pbonzini@redhat.com>
> L: kvm@vger.kernel.org
> F: tools/kvm/
>
> Paolo, is kvm_stat something you still care about, or should it be set to unmaintained?
> In this case I think someone at SUSE could pick it up.
>
> Ciao,
>
> Claudio
>
>
> On 10/21/24 15:26, Juergen Gross wrote:
>> Any reason not to commit this patch? It has got a Reviewed-by: tag from
>> Stefan more than 2 months ago...
>>
>> On 07.08.24 19:23, Claudio Fontana wrote:
>>> For the -l and -L options (logging mode), replace the use of the
>>> KeyboardInterrupt exception to gracefully terminate in favor
>>> of handling the SIGINT and SIGTERM signals.
>>>
>>> This allows the program to be run from scripts and still be
>>> signaled to gracefully terminate without an interactive terminal.
>>>
>>> Before this change, something like this script:
>>>
>>> kvm_stat -p 85896 -d -t -s 1 -c -L kvm_stat_85896.csv &
>>> sleep 10
>>> pkill -TERM -P $$
>>>
>>> would yield an empty log:
>>> -rw-r--r-- 1 root root 0 Aug 7 16:17 kvm_stat_85896.csv
>>>
>>> after this commit:
>>> -rw-r--r-- 1 root root 13466 Aug 7 16:57 kvm_stat_85896.csv
>>>
>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>> Cc: Dario Faggioli <dfaggioli@suse.com>
>>> Cc: Fabiano Rosas <farosas@suse.de>
>>> ---
>>> tools/kvm/kvm_stat/kvm_stat | 64 ++++++++++++++++-----------------
>>> tools/kvm/kvm_stat/kvm_stat.txt | 12 +++++++
>>> 2 files changed, 44 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
>>> index 15bf00e79e3f..2cf2da3ed002 100755
>>> --- a/tools/kvm/kvm_stat/kvm_stat
>>> +++ b/tools/kvm/kvm_stat/kvm_stat
>>> @@ -297,8 +297,6 @@ IOCTL_NUMBERS = {
>>> 'RESET': 0x00002403,
>>> }
>>>
>>> -signal_received = False
>>> -
>>> ENCODING = locale.getpreferredencoding(False)
>>> TRACE_FILTER = re.compile(r'^[^\(]*$')
>>>
>>> @@ -1598,7 +1596,19 @@ class CSVFormat(object):
>>>
>>> def log(stats, opts, frmt, keys):
>>> """Prints statistics as reiterating key block, multiple value blocks."""
>>> - global signal_received
>>> + signal_received = defaultdict(bool)
>>> +
>>> + def handle_signal(sig, frame):
>>> + nonlocal signal_received
>>> + signal_received[sig] = True
>>> + return
>>> +
>>> +
>>> + signal.signal(signal.SIGINT, handle_signal)
>>> + signal.signal(signal.SIGTERM, handle_signal)
>>> + if opts.log_to_file:
>>> + signal.signal(signal.SIGHUP, handle_signal)
>>> +
>>> line = 0
>>> banner_repeat = 20
>>> f = None
>>> @@ -1624,39 +1634,31 @@ def log(stats, opts, frmt, keys):
>>> do_banner(opts)
>>> banner_printed = True
>>> while True:
>>> - try:
>>> - time.sleep(opts.set_delay)
>>> - if signal_received:
>>> - banner_printed = True
>>> - line = 0
>>> - f.close()
>>> - do_banner(opts)
>>> - signal_received = False
>>> - if (line % banner_repeat == 0 and not banner_printed and
>>> - not (opts.log_to_file and isinstance(frmt, CSVFormat))):
>>> - do_banner(opts)
>>> - banner_printed = True
>>> - values = stats.get()
>>> - if (not opts.skip_zero_records or
>>> - any(values[k].delta != 0 for k in keys)):
>>> - do_statline(opts, values)
>>> - line += 1
>>> - banner_printed = False
>>> - except KeyboardInterrupt:
>>> + time.sleep(opts.set_delay)
>>> + # Do not use the KeyboardInterrupt exception, because we may be running without a terminal
>>> + if (signal_received[signal.SIGINT] or signal_received[signal.SIGTERM]):
>>> break
>>> + if signal_received[signal.SIGHUP]:
>>> + banner_printed = True
>>> + line = 0
>>> + f.close()
>>> + do_banner(opts)
>>> + signal_received[signal.SIGHUP] = False
>>> + if (line % banner_repeat == 0 and not banner_printed and
>>> + not (opts.log_to_file and isinstance(frmt, CSVFormat))):
>>> + do_banner(opts)
>>> + banner_printed = True
>>> + values = stats.get()
>>> + if (not opts.skip_zero_records or
>>> + any(values[k].delta != 0 for k in keys)):
>>> + do_statline(opts, values)
>>> + line += 1
>>> + banner_printed = False
>>>
>>> if opts.log_to_file:
>>> f.close()
>>>
>>>
>>> -def handle_signal(sig, frame):
>>> - global signal_received
>>> -
>>> - signal_received = True
>>> -
>>> - return
>>> -
>>> -
>>> def is_delay_valid(delay):
>>> """Verify delay is in valid value range."""
>>> msg = None
>>> @@ -1869,8 +1871,6 @@ def main():
>>> sys.exit(0)
>>>
>>> if options.log or options.log_to_file:
>>> - if options.log_to_file:
>>> - signal.signal(signal.SIGHUP, handle_signal)
>>> keys = sorted(stats.get().keys())
>>> if options.csv:
>>> frmt = CSVFormat(keys)
>>> diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
>>> index 3a9f2037bd23..4a99a111a93c 100644
>>> --- a/tools/kvm/kvm_stat/kvm_stat.txt
>>> +++ b/tools/kvm/kvm_stat/kvm_stat.txt
>>> @@ -115,6 +115,18 @@ OPTIONS
>>> --skip-zero-records::
>>> omit records with all zeros in logging mode
>>>
>>> +
>>> +SIGNALS
>>> +-------
>>> +when kvm_stat is running in logging mode (either with -l or with -L),
>>> +it handles the following signals:
>>> +
>>> +SIGHUP - closes and reopens the log file (-L only), then continues.
>>> +
>>> +SIGINT - closes the log file and terminates.
>>> +SIGTERM - closes the log file and terminates.
>>> +
>>> +
>>> SEE ALSO
>>> --------
>>> 'perf'(1), 'trace-cmd'(1)
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal
2025-04-03 14:01 ` Claudio Fontana
@ 2025-04-03 14:24 ` Claudio Fontana
0 siblings, 0 replies; 7+ messages in thread
From: Claudio Fontana @ 2025-04-03 14:24 UTC (permalink / raw)
To: Juergen Gross, Paolo Bonzini
Cc: kvm, Stefan Hajnoczi, Dario Faggioli, Fabiano Rosas
(and my offer to maintain this script remains) - Thanks. C
On 4/3/25 16:01, Claudio Fontana wrote:
> Gentle ping,
>
> we currently need to carry this downstream, could you include this fix?
>
> Thanks,
>
> CLaudio
>
> On 11/27/24 17:30, Claudio Fontana wrote:
>> Hi, according to MAINTAINERS file:
>>
>> M: Paolo Bonzini <pbonzini@redhat.com>
>> L: kvm@vger.kernel.org
>> F: tools/kvm/
>>
>> Paolo, is kvm_stat something you still care about, or should it be set to unmaintained?
>> In this case I think someone at SUSE could pick it up.
>>
>> Ciao,
>>
>> Claudio
>>
>>
>> On 10/21/24 15:26, Juergen Gross wrote:
>>> Any reason not to commit this patch? It has got a Reviewed-by: tag from
>>> Stefan more than 2 months ago...
>>>
>>> On 07.08.24 19:23, Claudio Fontana wrote:
>>>> For the -l and -L options (logging mode), replace the use of the
>>>> KeyboardInterrupt exception to gracefully terminate in favor
>>>> of handling the SIGINT and SIGTERM signals.
>>>>
>>>> This allows the program to be run from scripts and still be
>>>> signaled to gracefully terminate without an interactive terminal.
>>>>
>>>> Before this change, something like this script:
>>>>
>>>> kvm_stat -p 85896 -d -t -s 1 -c -L kvm_stat_85896.csv &
>>>> sleep 10
>>>> pkill -TERM -P $$
>>>>
>>>> would yield an empty log:
>>>> -rw-r--r-- 1 root root 0 Aug 7 16:17 kvm_stat_85896.csv
>>>>
>>>> after this commit:
>>>> -rw-r--r-- 1 root root 13466 Aug 7 16:57 kvm_stat_85896.csv
>>>>
>>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>>> Cc: Dario Faggioli <dfaggioli@suse.com>
>>>> Cc: Fabiano Rosas <farosas@suse.de>
>>>> ---
>>>> tools/kvm/kvm_stat/kvm_stat | 64 ++++++++++++++++-----------------
>>>> tools/kvm/kvm_stat/kvm_stat.txt | 12 +++++++
>>>> 2 files changed, 44 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
>>>> index 15bf00e79e3f..2cf2da3ed002 100755
>>>> --- a/tools/kvm/kvm_stat/kvm_stat
>>>> +++ b/tools/kvm/kvm_stat/kvm_stat
>>>> @@ -297,8 +297,6 @@ IOCTL_NUMBERS = {
>>>> 'RESET': 0x00002403,
>>>> }
>>>>
>>>> -signal_received = False
>>>> -
>>>> ENCODING = locale.getpreferredencoding(False)
>>>> TRACE_FILTER = re.compile(r'^[^\(]*$')
>>>>
>>>> @@ -1598,7 +1596,19 @@ class CSVFormat(object):
>>>>
>>>> def log(stats, opts, frmt, keys):
>>>> """Prints statistics as reiterating key block, multiple value blocks."""
>>>> - global signal_received
>>>> + signal_received = defaultdict(bool)
>>>> +
>>>> + def handle_signal(sig, frame):
>>>> + nonlocal signal_received
>>>> + signal_received[sig] = True
>>>> + return
>>>> +
>>>> +
>>>> + signal.signal(signal.SIGINT, handle_signal)
>>>> + signal.signal(signal.SIGTERM, handle_signal)
>>>> + if opts.log_to_file:
>>>> + signal.signal(signal.SIGHUP, handle_signal)
>>>> +
>>>> line = 0
>>>> banner_repeat = 20
>>>> f = None
>>>> @@ -1624,39 +1634,31 @@ def log(stats, opts, frmt, keys):
>>>> do_banner(opts)
>>>> banner_printed = True
>>>> while True:
>>>> - try:
>>>> - time.sleep(opts.set_delay)
>>>> - if signal_received:
>>>> - banner_printed = True
>>>> - line = 0
>>>> - f.close()
>>>> - do_banner(opts)
>>>> - signal_received = False
>>>> - if (line % banner_repeat == 0 and not banner_printed and
>>>> - not (opts.log_to_file and isinstance(frmt, CSVFormat))):
>>>> - do_banner(opts)
>>>> - banner_printed = True
>>>> - values = stats.get()
>>>> - if (not opts.skip_zero_records or
>>>> - any(values[k].delta != 0 for k in keys)):
>>>> - do_statline(opts, values)
>>>> - line += 1
>>>> - banner_printed = False
>>>> - except KeyboardInterrupt:
>>>> + time.sleep(opts.set_delay)
>>>> + # Do not use the KeyboardInterrupt exception, because we may be running without a terminal
>>>> + if (signal_received[signal.SIGINT] or signal_received[signal.SIGTERM]):
>>>> break
>>>> + if signal_received[signal.SIGHUP]:
>>>> + banner_printed = True
>>>> + line = 0
>>>> + f.close()
>>>> + do_banner(opts)
>>>> + signal_received[signal.SIGHUP] = False
>>>> + if (line % banner_repeat == 0 and not banner_printed and
>>>> + not (opts.log_to_file and isinstance(frmt, CSVFormat))):
>>>> + do_banner(opts)
>>>> + banner_printed = True
>>>> + values = stats.get()
>>>> + if (not opts.skip_zero_records or
>>>> + any(values[k].delta != 0 for k in keys)):
>>>> + do_statline(opts, values)
>>>> + line += 1
>>>> + banner_printed = False
>>>>
>>>> if opts.log_to_file:
>>>> f.close()
>>>>
>>>>
>>>> -def handle_signal(sig, frame):
>>>> - global signal_received
>>>> -
>>>> - signal_received = True
>>>> -
>>>> - return
>>>> -
>>>> -
>>>> def is_delay_valid(delay):
>>>> """Verify delay is in valid value range."""
>>>> msg = None
>>>> @@ -1869,8 +1871,6 @@ def main():
>>>> sys.exit(0)
>>>>
>>>> if options.log or options.log_to_file:
>>>> - if options.log_to_file:
>>>> - signal.signal(signal.SIGHUP, handle_signal)
>>>> keys = sorted(stats.get().keys())
>>>> if options.csv:
>>>> frmt = CSVFormat(keys)
>>>> diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
>>>> index 3a9f2037bd23..4a99a111a93c 100644
>>>> --- a/tools/kvm/kvm_stat/kvm_stat.txt
>>>> +++ b/tools/kvm/kvm_stat/kvm_stat.txt
>>>> @@ -115,6 +115,18 @@ OPTIONS
>>>> --skip-zero-records::
>>>> omit records with all zeros in logging mode
>>>>
>>>> +
>>>> +SIGNALS
>>>> +-------
>>>> +when kvm_stat is running in logging mode (either with -l or with -L),
>>>> +it handles the following signals:
>>>> +
>>>> +SIGHUP - closes and reopens the log file (-L only), then continues.
>>>> +
>>>> +SIGINT - closes the log file and terminates.
>>>> +SIGTERM - closes the log file and terminates.
>>>> +
>>>> +
>>>> SEE ALSO
>>>> --------
>>>> 'perf'(1), 'trace-cmd'(1)
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal
2024-08-07 17:23 [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal Claudio Fontana
2024-08-12 15:33 ` Stefan Hajnoczi
2024-10-21 13:26 ` Juergen Gross
@ 2025-04-07 11:03 ` Juergen Gross
2 siblings, 0 replies; 7+ messages in thread
From: Juergen Gross @ 2025-04-07 11:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, Stefan Hajnoczi, Dario Faggioli, Fabiano Rosas, lkml,
Claudio Fontana
[-- Attachment #1.1.1: Type: text/plain, Size: 5847 bytes --]
Below patch was Acked in August last year, and despite a ping by me
in October it wasn't committed yet.
Pablo, do you have any concerns with this patch? Or are you just
no longer interested in maintaining kvm_stat?
As this patch is fixing a real bug which we at SUSE would like to
see fixed, please take the patch or tell us what to do to get it
upstream.
In case you just don't mind, I could offer to merge it via the Xen
tree, even if this patch doesn't really belong there.
Juergen
On 07.08.24 19:23, Claudio Fontana wrote:
> For the -l and -L options (logging mode), replace the use of the
> KeyboardInterrupt exception to gracefully terminate in favor
> of handling the SIGINT and SIGTERM signals.
>
> This allows the program to be run from scripts and still be
> signaled to gracefully terminate without an interactive terminal.
>
> Before this change, something like this script:
>
> kvm_stat -p 85896 -d -t -s 1 -c -L kvm_stat_85896.csv &
> sleep 10
> pkill -TERM -P $$
>
> would yield an empty log:
> -rw-r--r-- 1 root root 0 Aug 7 16:17 kvm_stat_85896.csv
>
> after this commit:
> -rw-r--r-- 1 root root 13466 Aug 7 16:57 kvm_stat_85896.csv
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> Cc: Dario Faggioli <dfaggioli@suse.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> ---
> tools/kvm/kvm_stat/kvm_stat | 64 ++++++++++++++++-----------------
> tools/kvm/kvm_stat/kvm_stat.txt | 12 +++++++
> 2 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index 15bf00e79e3f..2cf2da3ed002 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -297,8 +297,6 @@ IOCTL_NUMBERS = {
> 'RESET': 0x00002403,
> }
>
> -signal_received = False
> -
> ENCODING = locale.getpreferredencoding(False)
> TRACE_FILTER = re.compile(r'^[^\(]*$')
>
> @@ -1598,7 +1596,19 @@ class CSVFormat(object):
>
> def log(stats, opts, frmt, keys):
> """Prints statistics as reiterating key block, multiple value blocks."""
> - global signal_received
> + signal_received = defaultdict(bool)
> +
> + def handle_signal(sig, frame):
> + nonlocal signal_received
> + signal_received[sig] = True
> + return
> +
> +
> + signal.signal(signal.SIGINT, handle_signal)
> + signal.signal(signal.SIGTERM, handle_signal)
> + if opts.log_to_file:
> + signal.signal(signal.SIGHUP, handle_signal)
> +
> line = 0
> banner_repeat = 20
> f = None
> @@ -1624,39 +1634,31 @@ def log(stats, opts, frmt, keys):
> do_banner(opts)
> banner_printed = True
> while True:
> - try:
> - time.sleep(opts.set_delay)
> - if signal_received:
> - banner_printed = True
> - line = 0
> - f.close()
> - do_banner(opts)
> - signal_received = False
> - if (line % banner_repeat == 0 and not banner_printed and
> - not (opts.log_to_file and isinstance(frmt, CSVFormat))):
> - do_banner(opts)
> - banner_printed = True
> - values = stats.get()
> - if (not opts.skip_zero_records or
> - any(values[k].delta != 0 for k in keys)):
> - do_statline(opts, values)
> - line += 1
> - banner_printed = False
> - except KeyboardInterrupt:
> + time.sleep(opts.set_delay)
> + # Do not use the KeyboardInterrupt exception, because we may be running without a terminal
> + if (signal_received[signal.SIGINT] or signal_received[signal.SIGTERM]):
> break
> + if signal_received[signal.SIGHUP]:
> + banner_printed = True
> + line = 0
> + f.close()
> + do_banner(opts)
> + signal_received[signal.SIGHUP] = False
> + if (line % banner_repeat == 0 and not banner_printed and
> + not (opts.log_to_file and isinstance(frmt, CSVFormat))):
> + do_banner(opts)
> + banner_printed = True
> + values = stats.get()
> + if (not opts.skip_zero_records or
> + any(values[k].delta != 0 for k in keys)):
> + do_statline(opts, values)
> + line += 1
> + banner_printed = False
>
> if opts.log_to_file:
> f.close()
>
>
> -def handle_signal(sig, frame):
> - global signal_received
> -
> - signal_received = True
> -
> - return
> -
> -
> def is_delay_valid(delay):
> """Verify delay is in valid value range."""
> msg = None
> @@ -1869,8 +1871,6 @@ def main():
> sys.exit(0)
>
> if options.log or options.log_to_file:
> - if options.log_to_file:
> - signal.signal(signal.SIGHUP, handle_signal)
> keys = sorted(stats.get().keys())
> if options.csv:
> frmt = CSVFormat(keys)
> diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
> index 3a9f2037bd23..4a99a111a93c 100644
> --- a/tools/kvm/kvm_stat/kvm_stat.txt
> +++ b/tools/kvm/kvm_stat/kvm_stat.txt
> @@ -115,6 +115,18 @@ OPTIONS
> --skip-zero-records::
> omit records with all zeros in logging mode
>
> +
> +SIGNALS
> +-------
> +when kvm_stat is running in logging mode (either with -l or with -L),
> +it handles the following signals:
> +
> +SIGHUP - closes and reopens the log file (-L only), then continues.
> +
> +SIGINT - closes the log file and terminates.
> +SIGTERM - closes the log file and terminates.
> +
> +
> SEE ALSO
> --------
> 'perf'(1), 'trace-cmd'(1)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-04-07 11:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-07 17:23 [PATCH] tools/kvm_stat: fix termination behavior when not on a terminal Claudio Fontana
2024-08-12 15:33 ` Stefan Hajnoczi
2024-10-21 13:26 ` Juergen Gross
2024-11-27 16:30 ` Claudio Fontana
2025-04-03 14:01 ` Claudio Fontana
2025-04-03 14:24 ` Claudio Fontana
2025-04-07 11:03 ` Juergen Gross
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox