diff mbox

[1/2] Add dd-style SIGUSR1 progress reporting

Message ID 1303907511-4092-2-git-send-email-Jes.Sorensen@redhat.com
State New
Headers show

Commit Message

Jes Sorensen April 27, 2011, 12:31 p.m. UTC 
From: Jes Sorensen <Jes.Sorensen@redhat.com>

This introduces support for dd-style progress reporting on POSIX
systems, if the user hasn't specified -p to report progress. If sent a
SIGUSR1, qemu-img will report current progress for commands that
support progress reporting.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 qemu-progress.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 48 insertions(+), 5 deletions(-)

Comments

Markus Armbruster April 27, 2011, 4:14 p.m. UTC | #1 
Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> This introduces support for dd-style progress reporting on POSIX
> systems, if the user hasn't specified -p to report progress. If sent a
> SIGUSR1, qemu-img will report current progress for commands that
> support progress reporting.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  qemu-progress.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/qemu-progress.c b/qemu-progress.c
> index 656e065..b4b751c 100644
> --- a/qemu-progress.c
> +++ b/qemu-progress.c
> @@ -26,12 +26,15 @@
>  #include "osdep.h"
>  #include "sysemu.h"
>  #include <stdio.h>
> +#include <signal.h>
>  
>  struct progress_state {
>      int enabled;
>      float current;
>      float last_print;
>      float min_skip;
> +    void (*print)(void);
> +    void (*end)(void);
>  };
>  
>  static struct progress_state state;
> @@ -51,20 +54,60 @@ static void progress_simple_print(void)
>  
>  static void progress_simple_end(void)
>  {
> -    if (state.enabled) {
> -        printf("\n");
> -    }
> +    printf("\n");
> +}
> +
> +static void progress_simple_init(void)
> +{
> +    state.print = progress_simple_print;
> +    state.end = progress_simple_end;
> +}
> +
> +#ifdef CONFIG_POSIX
> +static void sigusr_print(int signal)
> +{
> +    printf("    (%3.2f/100%%)\n", state.current);

printf() is not async-signal-safe.  I don't think you can safely call it
in a signal handler.

> +}
> +#endif
[...]
Jes Sorensen April 28, 2011, 7:18 a.m. UTC | #2 
On 04/27/11 18:14, Markus Armbruster wrote:
>> +static void progress_simple_init(void)
>> +{
>> +    state.print = progress_simple_print;
>> +    state.end = progress_simple_end;
>> +}
>> +
>> +#ifdef CONFIG_POSIX
>> +static void sigusr_print(int signal)
>> +{
>> +    printf("    (%3.2f/100%%)\n", state.current);
> 
> printf() is not async-signal-safe.  I don't think you can safely call it
> in a signal handler.

Grrrr, you're absolutely right! Back to the drawing board!

If someone locates my lost marbles, would you mind returning them? I
need them urgently!

Cheers,
Jes
Paolo Bonzini April 28, 2011, 12:04 p.m. UTC | #3 
On 04/28/2011 09:18 AM, Jes Sorensen wrote:
> On 04/27/11 18:14, Markus Armbruster wrote:
>>> +static void progress_simple_init(void)
>>> +{
>>> +    state.print = progress_simple_print;
>>> +    state.end = progress_simple_end;
>>> +}
>>> +
>>> +#ifdef CONFIG_POSIX
>>> +static void sigusr_print(int signal)
>>> +{
>>> +    printf("    (%3.2f/100%%)\n", state.current);
>>
>> printf() is not async-signal-safe.  I don't think you can safely call it
>> in a signal handler.
>
> Grrrr, you're absolutely right! Back to the drawing board!

Let's add our own version of strtol to QEMU. :)

Paolo
diff mbox

Patch

diff --git a/qemu-progress.c b/qemu-progress.c
index 656e065..b4b751c 100644
--- a/qemu-progress.c
+++ b/qemu-progress.c
@@ -26,12 +26,15 @@ 
 #include "osdep.h"
 #include "sysemu.h"
 #include <stdio.h>
+#include <signal.h>
 
 struct progress_state {
     int enabled;
     float current;
     float last_print;
     float min_skip;
+    void (*print)(void);
+    void (*end)(void);
 };
 
 static struct progress_state state;
@@ -51,20 +54,60 @@  static void progress_simple_print(void)
 
 static void progress_simple_end(void)
 {
-    if (state.enabled) {
-        printf("\n");
-    }
+    printf("\n");
+}
+
+static void progress_simple_init(void)
+{
+    state.print = progress_simple_print;
+    state.end = progress_simple_end;
+}
+
+#ifdef CONFIG_POSIX
+static void sigusr_print(int signal)
+{
+    printf("    (%3.2f/100%%)\n", state.current);
+}
+#endif
+
+static void progress_dummy_print(void)
+{
+}
+
+static void progress_dummy_end(void)
+{
+}
+
+static void progress_dummy_init(void)
+{
+#ifdef CONFIG_POSIX
+    struct sigaction action;
+
+    memset(&action, 0, sizeof(action));
+    sigfillset(&action.sa_mask);
+    action.sa_handler = sigusr_print;
+    action.sa_flags = 0;
+    sigaction(SIGUSR1, &action, NULL);
+#endif
+
+    state.print = progress_dummy_print;
+    state.end = progress_dummy_end;
 }
 
 void qemu_progress_init(int enabled, float min_skip)
 {
     state.enabled = enabled;
     state.min_skip = min_skip;
+    if (enabled) {
+        progress_simple_init();
+    } else {
+        progress_dummy_init();
+    }
 }
 
 void qemu_progress_end(void)
 {
-    progress_simple_end();
+    state.end();
 }
 
 void qemu_progress_print(float percent, int max)
@@ -84,6 +127,6 @@  void qemu_progress_print(float percent, int max)
     if (current > (state.last_print + state.min_skip) ||
         (current == 100) || (current == 0)) {
         state.last_print = state.current;
-        progress_simple_print();
+        state.print();
     }
 }