[Qemu-devel] [PATCH v2 0/6] Fixes for buildbot failures

[Qemu-devel] [PATCH v2 0/6] Fixes for buildbot failures

[Stefan Hajnoczi]

v2:
 * Use setenv(3) instead of prepending environment variables to shell
   command-line [Markus]
 * 'error' should be 'warning' in QEMU_AUDIO_DRV=none commit description [Peter]
 * Avoid all() in tracetool [Riku]
 * Avoid GSequence in qemu-img.c (RHEL 5 glib does not support it)

I just did a sweep of the buildbot at
http://buildbot.b1-systems.de/qemu/builders.

These patches solve issues on RHEL5 and OpenBSD buildslaves.

Stefan Hajnoczi (6):
  qapi.py: avoid Python 2.5+ any() function
  libqtest: launch QEMU with QEMU_AUDIO_DRV=none
  trace: avoid Python 2.5 all() in tracetool
  mirror: fix uninitialized variable delay_ns warnings
  block: sort formats alphabetically in bdrv_iterate_format()
  Revert "qemu-img: sort block formats in help message"

 block.c                               | 14 +++++++++++++-
 block/mirror.c                        |  4 +---
 qemu-img.c                            | 25 +++----------------------
 scripts/qapi.py                       |  8 ++++----
 scripts/tracetool/backend/__init__.py |  3 ++-
 tests/libqtest.c                      |  1 +
 6 files changed, 24 insertions(+), 31 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v2 1/6] qapi.py: avoid Python 2.5+ any() function

[Stefan Hajnoczi]

There is one instance of any() in qapi.py that breaks builds on older
distros that ship Python 2.4 (like RHEL5):

  GEN   qmp-commands.h
Traceback (most recent call last):
  File "build/scripts/qapi-commands.py", line 445, in ?
    exprs = parse_schema(input_file)
  File "build/scripts/qapi.py", line 329, in parse_schema
    schema = QAPISchema(open(input_file, "r"))
  File "build/scripts/qapi.py", line 110, in __init__
    if any(include_path == elem[1]
NameError: global name 'any' is not defined

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/qapi.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index f2c6d1f..77d46aa 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -107,10 +107,10 @@ class QAPISchema:
                                         'Expected a file name (string), got: %s'
                                         % include)
                 include_path = os.path.join(self.input_dir, include)
-                if any(include_path == elem[1]
-                       for elem in self.include_hist):
-                    raise QAPIExprError(expr_info, "Inclusion loop for %s"
-                                        % include)
+                for elem in self.include_hist:
+                    if include_path == elem[1]:
+                        raise QAPIExprError(expr_info, "Inclusion loop for %s"
+                                            % include)
                 # skip multiple include of the same file
                 if include_path in previously_included:
                     continue
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 1/6] qapi.py: avoid Python 2.5+ any() function

[Benoît Canet]

The Wednesday 27 Aug 2014 à 12:08:51 (+0100), Stefan Hajnoczi wrote :
> There is one instance of any() in qapi.py that breaks builds on older
> distros that ship Python 2.4 (like RHEL5):
> 
>   GEN   qmp-commands.h
> Traceback (most recent call last):
>   File "build/scripts/qapi-commands.py", line 445, in ?
>     exprs = parse_schema(input_file)
>   File "build/scripts/qapi.py", line 329, in parse_schema
>     schema = QAPISchema(open(input_file, "r"))
>   File "build/scripts/qapi.py", line 110, in __init__
>     if any(include_path == elem[1]
> NameError: global name 'any' is not defined
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  scripts/qapi.py | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index f2c6d1f..77d46aa 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -107,10 +107,10 @@ class QAPISchema:
>                                          'Expected a file name (string), got: %s'
>                                          % include)
>                  include_path = os.path.join(self.input_dir, include)
> -                if any(include_path == elem[1]
> -                       for elem in self.include_hist):
> -                    raise QAPIExprError(expr_info, "Inclusion loop for %s"
> -                                        % include)
> +                for elem in self.include_hist:
> +                    if include_path == elem[1]:
> +                        raise QAPIExprError(expr_info, "Inclusion loop for %s"
> +                                            % include)
>                  # skip multiple include of the same file
>                  if include_path in previously_included:
>                      continue
> -- 
> 1.9.3
> 
> 
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>

[Qemu-devel] [PATCH v2 2/6] libqtest: launch QEMU with QEMU_AUDIO_DRV=none

[Stefan Hajnoczi]

No test case actually uses the audio backend.  Disable audio to prevent
warnings on hosts with no sound hardware present:

  GTESTER check-qtest-aarch64
  sdl: SDL_OpenAudio failed
  sdl: Reason: No available audio device
  sdl: SDL_OpenAudio failed
  sdl: Reason: No available audio device
  audio: Failed to create voice `lm4549.out'

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqtest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index ed55686..5e458e8 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -165,6 +165,7 @@ QTestState *qtest_init(const char *extra_args)
 
     s->qemu_pid = fork();
     if (s->qemu_pid == 0) {
+        setenv("QEMU_AUDIO_DRV", "none", true);
         command = g_strdup_printf("exec %s "
                                   "-qtest unix:%s,nowait "
                                   "-qtest-log %s "
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 2/6] libqtest: launch QEMU with QEMU_AUDIO_DRV=none

[Gerd Hoffmann]

On Mi, 2014-08-27 at 12:08 +0100, Stefan Hajnoczi wrote:

> No test case actually uses the audio backend.  Disable audio to
> prevent
> warnings on hosts with no sound hardware present:
> 
>   GTESTER check-qtest-aarch64
>   sdl: SDL_OpenAudio failed
>   sdl: Reason: No available audio device
>   sdl: SDL_OpenAudio failed
>   sdl: Reason: No available audio device
>   audio: Failed to create voice `lm4549.out'
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

[Qemu-devel] [PATCH v2 3/6] trace: avoid Python 2.5 all() in tracetool

[Stefan Hajnoczi]

Red Hat Enterprise Linux 5 ships Python 2.4.3.  The all() function was
added in Python 2.5 so we cannot use it.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/backend/__init__.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
index 5bfa1ef..d4b6dab 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -102,7 +102,8 @@ class Wrapper:
     def __init__(self, backends, format):
         self._backends = [backend.replace("-", "_") for backend in backends]
         self._format = format.replace("-", "_")
-        assert all(exists(backend) for backend in self._backends)
+        for backend in self._backends:
+            assert exists(backend)
         assert tracetool.format.exists(self._format)
 
     def _run_function(self, name, *args, **kwargs):
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 3/6] trace: avoid Python 2.5 all() in tracetool

[Benoît Canet]

The Wednesday 27 Aug 2014 à 12:08:53 (+0100), Stefan Hajnoczi wrote :
> Red Hat Enterprise Linux 5 ships Python 2.4.3.  The all() function was
> added in Python 2.5 so we cannot use it.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  scripts/tracetool/backend/__init__.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
> index 5bfa1ef..d4b6dab 100644
> --- a/scripts/tracetool/backend/__init__.py
> +++ b/scripts/tracetool/backend/__init__.py
> @@ -102,7 +102,8 @@ class Wrapper:
>      def __init__(self, backends, format):
>          self._backends = [backend.replace("-", "_") for backend in backends]
>          self._format = format.replace("-", "_")
> -        assert all(exists(backend) for backend in self._backends)
> +        for backend in self._backends:
> +            assert exists(backend)
>          assert tracetool.format.exists(self._format)
>  
>      def _run_function(self, name, *args, **kwargs):
> -- 
> 1.9.3
> 
> 
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>

[Qemu-devel] [PATCH v2 4/6] mirror: fix uninitialized variable delay_ns warnings

[Stefan Hajnoczi]

The gcc 4.1.2 compiler warns that delay_ns may be uninitialized in
mirror_iteration().

There are two break statements in the do ... while loop that skip over
the delay_ns assignment.  These are probably the cause of the warning.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/mirror.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 5e7a166..18b18e0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -157,7 +157,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
     BlockDriverState *source = s->common.bs;
     int nb_sectors, sectors_per_chunk, nb_chunks;
     int64_t end, sector_num, next_chunk, next_sector, hbitmap_next_sector;
-    uint64_t delay_ns;
+    uint64_t delay_ns = 0;
     MirrorOp *op;
 
     s->sector_num = hbitmap_iter_next(&s->hbi);
@@ -247,8 +247,6 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         next_chunk += added_chunks;
         if (!s->synced && s->common.speed) {
             delay_ns = ratelimit_calculate_delay(&s->limit, added_sectors);
-        } else {
-            delay_ns = 0;
         }
     } while (delay_ns == 0 && next_sector < end);
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 4/6] mirror: fix uninitialized variable delay_ns warnings

[Benoît Canet]

The Wednesday 27 Aug 2014 à 12:08:54 (+0100), Stefan Hajnoczi wrote :
> The gcc 4.1.2 compiler warns that delay_ns may be uninitialized in
> mirror_iteration().
> 
> There are two break statements in the do ... while loop that skip over
> the delay_ns assignment.  These are probably the cause of the warning.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/mirror.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 5e7a166..18b18e0 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -157,7 +157,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>      BlockDriverState *source = s->common.bs;
>      int nb_sectors, sectors_per_chunk, nb_chunks;
>      int64_t end, sector_num, next_chunk, next_sector, hbitmap_next_sector;
> -    uint64_t delay_ns;
> +    uint64_t delay_ns = 0;
>      MirrorOp *op;
>  
>      s->sector_num = hbitmap_iter_next(&s->hbi);
> @@ -247,8 +247,6 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>          next_chunk += added_chunks;
>          if (!s->synced && s->common.speed) {
>              delay_ns = ratelimit_calculate_delay(&s->limit, added_sectors);
> -        } else {
> -            delay_ns = 0;
>          }
>      } while (delay_ns == 0 && next_sector < end);
>  
> -- 
> 1.9.3
> 
> 
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>

[Qemu-devel] [PATCH v2 5/6] block: sort formats alphabetically in bdrv_iterate_format()

[Stefan Hajnoczi]

Format names are best consumed in alphabetical order.  This makes
human-readable output easy to produce.

bdrv_iterate_format() already has an array of format strings.  Sort them
before invoking the iteration callback.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index e9380f6..1df13ac 100644
--- a/block.c
+++ b/block.c
@@ -3744,11 +3744,17 @@ const char *bdrv_get_format_name(BlockDriverState *bs)
     return bs->drv ? bs->drv->format_name : NULL;
 }
 
+static int qsort_strcmp(const void *a, const void *b)
+{
+    return strcmp(a, b);
+}
+
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
                          void *opaque)
 {
     BlockDriver *drv;
     int count = 0;
+    int i;
     const char **formats = NULL;
 
     QLIST_FOREACH(drv, &bdrv_drivers, list) {
@@ -3762,10 +3768,16 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
             if (!found) {
                 formats = g_renew(const char *, formats, count + 1);
                 formats[count++] = drv->format_name;
-                it(opaque, drv->format_name);
             }
         }
     }
+
+    qsort(formats, count, sizeof(formats[0]), qsort_strcmp);
+
+    for (i = 0; i < count; i++) {
+        it(opaque, formats[i]);
+    }
+
     g_free(formats);
 }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 5/6] block: sort formats alphabetically in bdrv_iterate_format()

[Benoît Canet]

The Wednesday 27 Aug 2014 à 12:08:55 (+0100), Stefan Hajnoczi wrote :
> Format names are best consumed in alphabetical order.  This makes
> human-readable output easy to produce.
> 
> bdrv_iterate_format() already has an array of format strings.  Sort them
> before invoking the iteration callback.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index e9380f6..1df13ac 100644
> --- a/block.c
> +++ b/block.c
> @@ -3744,11 +3744,17 @@ const char *bdrv_get_format_name(BlockDriverState *bs)
>      return bs->drv ? bs->drv->format_name : NULL;
>  }
>  
> +static int qsort_strcmp(const void *a, const void *b)
> +{
> +    return strcmp(a, b);
> +}
> +
>  void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
>                           void *opaque)
>  {
>      BlockDriver *drv;
>      int count = 0;
> +    int i;
>      const char **formats = NULL;
>  
>      QLIST_FOREACH(drv, &bdrv_drivers, list) {
> @@ -3762,10 +3768,16 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
>              if (!found) {
>                  formats = g_renew(const char *, formats, count + 1);
>                  formats[count++] = drv->format_name;
> -                it(opaque, drv->format_name);
>              }
>          }
>      }
> +
> +    qsort(formats, count, sizeof(formats[0]), qsort_strcmp);

We are lucky this is not using qsort_r which was added in 2008 after RHEL 5 release :)

Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>

> +
> +    for (i = 0; i < count; i++) {
> +        it(opaque, formats[i]);
> +    }
> +
>      g_free(formats);
>  }
>  
> -- 
> 1.9.3
> 
> 

[Qemu-devel] [PATCH v2 6/6] Revert "qemu-img: sort block formats in help message"

[Stefan Hajnoczi]

This reverts commit 1a443c1b8b4314d365e82bddeb1de5b4b1c15fb3 and the
later commit 395071a76328189f50c778f4dee6dabb90503dd9.

GSequence was introduced in glib 2.14.  RHEL 5 fails to compile since it
uses glib 2.12.3.

Now that bdrv_iterate_format() invokes the iteration callback in sorted
order these commits are unnecessary.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-img.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index c843420..2052b14 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -32,7 +32,6 @@
 #include "block/block_int.h"
 #include "block/qapi.h"
 #include <getopt.h>
-#include <glib.h>
 
 #define QEMU_IMG_VERSION "qemu-img version " QEMU_VERSION \
                           ", Copyright (c) 2004-2008 Fabrice Bellard\n"
@@ -56,22 +55,9 @@ typedef enum OutputFormat {
 #define BDRV_O_FLAGS BDRV_O_CACHE_WB
 #define BDRV_DEFAULT_CACHE "writeback"
 
-static gint compare_data(gconstpointer a, gconstpointer b, gpointer user)
+static void format_print(void *opaque, const char *name)
 {
-    return g_strcmp0(a, b);
-}
-
-static void print_format(gpointer data, gpointer user)
-{
-    printf(" %s", (char *)data);
-}
-
-static void add_format_to_seq(void *opaque, const char *fmt_name)
-{
-    GSequence *seq = opaque;
-
-    g_sequence_insert_sorted(seq, (gpointer)fmt_name,
-                             compare_data, NULL);
+    printf(" %s", name);
 }
 
 static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
@@ -157,15 +143,10 @@ static void QEMU_NORETURN help(void)
            "  '-f' first image format\n"
            "  '-F' second image format\n"
            "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
-    GSequence *seq;
 
     printf("%s\nSupported formats:", help_msg);
-    seq = g_sequence_new(NULL);
-    bdrv_iterate_format(add_format_to_seq, seq);
-    g_sequence_foreach(seq, print_format, NULL);
+    bdrv_iterate_format(format_print, NULL);
     printf("\n");
-    g_sequence_free(seq);
-
     exit(EXIT_SUCCESS);
 }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 6/6] Revert "qemu-img: sort block formats in help message"

[Benoît Canet]

The Wednesday 27 Aug 2014 à 12:08:56 (+0100), Stefan Hajnoczi wrote :
> This reverts commit 1a443c1b8b4314d365e82bddeb1de5b4b1c15fb3 and the
> later commit 395071a76328189f50c778f4dee6dabb90503dd9.
> 
> GSequence was introduced in glib 2.14.  RHEL 5 fails to compile since it
> uses glib 2.12.3.
> 
> Now that bdrv_iterate_format() invokes the iteration callback in sorted
> order these commits are unnecessary.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qemu-img.c | 25 +++----------------------
>  1 file changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index c843420..2052b14 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -32,7 +32,6 @@
>  #include "block/block_int.h"
>  #include "block/qapi.h"
>  #include <getopt.h>
> -#include <glib.h>
>  
>  #define QEMU_IMG_VERSION "qemu-img version " QEMU_VERSION \
>                            ", Copyright (c) 2004-2008 Fabrice Bellard\n"
> @@ -56,22 +55,9 @@ typedef enum OutputFormat {
>  #define BDRV_O_FLAGS BDRV_O_CACHE_WB
>  #define BDRV_DEFAULT_CACHE "writeback"
>  
> -static gint compare_data(gconstpointer a, gconstpointer b, gpointer user)
> +static void format_print(void *opaque, const char *name)
>  {
> -    return g_strcmp0(a, b);
> -}
> -
> -static void print_format(gpointer data, gpointer user)
> -{
> -    printf(" %s", (char *)data);
> -}
> -
> -static void add_format_to_seq(void *opaque, const char *fmt_name)
> -{
> -    GSequence *seq = opaque;
> -
> -    g_sequence_insert_sorted(seq, (gpointer)fmt_name,
> -                             compare_data, NULL);
> +    printf(" %s", name);
>  }
>  
>  static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
> @@ -157,15 +143,10 @@ static void QEMU_NORETURN help(void)
>             "  '-f' first image format\n"
>             "  '-F' second image format\n"
>             "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
> -    GSequence *seq;
>  
>      printf("%s\nSupported formats:", help_msg);
> -    seq = g_sequence_new(NULL);
> -    bdrv_iterate_format(add_format_to_seq, seq);
> -    g_sequence_foreach(seq, print_format, NULL);
> +    bdrv_iterate_format(format_print, NULL);
>      printf("\n");
> -    g_sequence_free(seq);
> -
>      exit(EXIT_SUCCESS);
>  }
>  
> -- 
> 1.9.3
> 
> 
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>