[Qemu-devel] [PATCH v2 qemu-block 0/2] Add dd-style SIGUSR1 progress reporting

[Qemu-devel] [PATCH v2 qemu-block 0/2] Add dd-style SIGUSR1 progress reporting

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This introduces support for dd-style progress reporting, 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.

v2 fixes the mingw32 build problems, there is no change to the code on
POSIX systems. It should be a drop-in replacement for the previous
patch.

Jes Sorensen (2):
  Add dd-style SIGUSR1 progress reporting
  Remove obsolete 'enabled' variable from progress state

 qemu-progress.c |   61 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 50 insertions(+), 11 deletions(-)

-- 
1.7.4.4

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

[Jes.Sorensen]

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);
+}
+#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();
     }
 }
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH 1/2] Add dd-style SIGUSR1 progress reporting

[Markus Armbruster]

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
[...]

Re: [Qemu-devel] [PATCH 1/2] Add dd-style SIGUSR1 progress reporting

[Jes Sorensen]

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

Re: [Qemu-devel] [PATCH 1/2] Add dd-style SIGUSR1 progress reporting

[Paolo Bonzini]

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

[Qemu-devel] [PATCH 2/2] Remove obsolete 'enabled' variable from progress state

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-progress.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/qemu-progress.c b/qemu-progress.c
index b4b751c..e1feb89 100644
--- a/qemu-progress.c
+++ b/qemu-progress.c
@@ -29,7 +29,6 @@
 #include <signal.h>
 
 struct progress_state {
-    int enabled;
     float current;
     float last_print;
     float min_skip;
@@ -46,10 +45,8 @@ static struct progress_state state;
  */
 static void progress_simple_print(void)
 {
-    if (state.enabled) {
-        printf("    (%3.2f/100%%)\r", state.current);
-        fflush(stdout);
-    }
+    printf("    (%3.2f/100%%)\r", state.current);
+    fflush(stdout);
 }
 
 static void progress_simple_end(void)
@@ -96,7 +93,6 @@ static void progress_dummy_init(void)
 
 void qemu_progress_init(int enabled, float min_skip)
 {
-    state.enabled = enabled;
     state.min_skip = min_skip;
     if (enabled) {
         progress_simple_init();
-- 
1.7.4.4