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

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

[Mike Day]

The help message for qemu-img lists the supported block formats, of
which there are 27 as of version 2.0.50. The formats are printed in
the order of their driver's position in a linked list, which appears
random. This patch prints the formats in sorted order, making it
easier to read and to find a specific format in the list.

v2: Incorporated feedback from Stefan Hajnoczi

Signed-off-by: Mike Day <ncmike@ncultra.org>
---
 qemu-img.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 96f4463..a559108 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -32,6 +32,7 @@
 #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"
@@ -55,9 +56,25 @@ typedef enum OutputFormat {
 #define BDRV_O_FLAGS BDRV_O_CACHE_WB
 #define BDRV_DEFAULT_CACHE "writeback"
 
-static void format_print(void *opaque, const char *name)
+static gint compare_data(gconstpointer a, gconstpointer b, gpointer user)
 {
-    printf(" %s", 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 = (GSequence *)opaque;
+
+    if (!g_sequence_lookup(seq, (gpointer)fmt_name,
+                           compare_data, NULL)) {
+        g_sequence_insert_sorted(seq, (gpointer)fmt_name,
+                                 compare_data, NULL);
+    }
 }
 
 static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
@@ -144,8 +161,12 @@ static void QEMU_NORETURN help(void)
            "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
 
     printf("%s\nSupported formats:", help_msg);
-    bdrv_iterate_format(format_print, NULL);
+    GSequence *seq = g_sequence_new(NULL);
+    bdrv_iterate_format(add_format_to_seq, seq);
+    g_sequence_foreach(seq, print_format, NULL);
     printf("\n");
+    g_sequence_free(seq);
+
     exit(EXIT_SUCCESS);
 }
 
-- 
1.9.0

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

[Fam Zheng]

On Mon, 05/05 12:53, Mike Day wrote:
> The help message for qemu-img lists the supported block formats, of
> which there are 27 as of version 2.0.50. The formats are printed in
> the order of their driver's position in a linked list, which appears
> random. This patch prints the formats in sorted order, making it
> easier to read and to find a specific format in the list.
> 

Revision note should go below --- , so the line won't get into the commit log
when maintainers apply the patch, like:

> 
> Signed-off-by: Mike Day <ncmike@ncultra.org>
> ---

v2: Incorporated feedback from Stefan Hajnoczi

>  qemu-img.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 96f4463..a559108 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -32,6 +32,7 @@
>  #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"
> @@ -55,9 +56,25 @@ typedef enum OutputFormat {
>  #define BDRV_O_FLAGS BDRV_O_CACHE_WB
>  #define BDRV_DEFAULT_CACHE "writeback"
>  
> -static void format_print(void *opaque, const char *name)
> +static gint compare_data(gconstpointer a, gconstpointer b, gpointer user)
>  {
> -    printf(" %s", 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 = (GSequence *)opaque;

Stefan pointed in comments for v1:

       GSequence *seq = opaque;

> +
> +    if (!g_sequence_lookup(seq, (gpointer)fmt_name,
> +                           compare_data, NULL)) {
> +        g_sequence_insert_sorted(seq, (gpointer)fmt_name,
> +                                 compare_data, NULL);
> +    }
>  }
>  
>  static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
> @@ -144,8 +161,12 @@ static void QEMU_NORETURN help(void)
>             "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
>  
>      printf("%s\nSupported formats:", help_msg);
> -    bdrv_iterate_format(format_print, NULL);
> +    GSequence *seq = g_sequence_new(NULL);

Let's put this variable declaration above statements, or move the printf above
down.

Thanks,
Fam

> +    bdrv_iterate_format(add_format_to_seq, seq);
> +    g_sequence_foreach(seq, print_format, NULL);
>      printf("\n");
> +    g_sequence_free(seq);
> +
>      exit(EXIT_SUCCESS);
>  }
>  
> -- 
> 1.9.0
> 
> 

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

[Stefan Hajnoczi]

On Mon, May 05, 2014 at 12:53:34PM -0400, Mike Day wrote:
> The help message for qemu-img lists the supported block formats, of
> which there are 27 as of version 2.0.50. The formats are printed in
> the order of their driver's position in a linked list, which appears
> random. This patch prints the formats in sorted order, making it
> easier to read and to find a specific format in the list.
> 
> v2: Incorporated feedback from Stefan Hajnoczi
> 
> Signed-off-by: Mike Day <ncmike@ncultra.org>
> ---
>  qemu-img.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)

Thanks, applied to my block tree with Fam's suggestions squashed in:
https://github.com/stefanha/qemu/commits/block

Stefan

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

[Laurent Desnogues]

Hi,

just noticed the use of some too recent glib features: g_strcmp0
and GSequence related ones.

Thanks,

Laurent

On Mon, May 5, 2014 at 6:53 PM, Mike Day <ncmike@ncultra.org> wrote:
> The help message for qemu-img lists the supported block formats, of
> which there are 27 as of version 2.0.50. The formats are printed in
> the order of their driver's position in a linked list, which appears
> random. This patch prints the formats in sorted order, making it
> easier to read and to find a specific format in the list.
>
> v2: Incorporated feedback from Stefan Hajnoczi
>
> Signed-off-by: Mike Day <ncmike@ncultra.org>
> ---
>  qemu-img.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 96f4463..a559108 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -32,6 +32,7 @@
>  #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"
> @@ -55,9 +56,25 @@ typedef enum OutputFormat {
>  #define BDRV_O_FLAGS BDRV_O_CACHE_WB
>  #define BDRV_DEFAULT_CACHE "writeback"
>
> -static void format_print(void *opaque, const char *name)
> +static gint compare_data(gconstpointer a, gconstpointer b, gpointer user)
>  {
> -    printf(" %s", 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 = (GSequence *)opaque;
> +
> +    if (!g_sequence_lookup(seq, (gpointer)fmt_name,
> +                           compare_data, NULL)) {
> +        g_sequence_insert_sorted(seq, (gpointer)fmt_name,
> +                                 compare_data, NULL);
> +    }
>  }
>
>  static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
> @@ -144,8 +161,12 @@ static void QEMU_NORETURN help(void)
>             "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
>
>      printf("%s\nSupported formats:", help_msg);
> -    bdrv_iterate_format(format_print, NULL);
> +    GSequence *seq = g_sequence_new(NULL);
> +    bdrv_iterate_format(add_format_to_seq, seq);
> +    g_sequence_foreach(seq, print_format, NULL);
>      printf("\n");
> +    g_sequence_free(seq);
> +
>      exit(EXIT_SUCCESS);
>  }
>
> --
> 1.9.0
>
>

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

[Mike Day]

On Fri, May 16, 2014 at 9:59 AM, Laurent Desnogues
<laurent.desnogues@gmail.com> wrote:
>
> just noticed the use of some too recent glib features: g_strcmp0
> and GSequence related ones.

There is a patch upstream that removes g_sequence_lookup, which was
not linking on one platform. I haven't seen any other reports of
breakage. g_strcmp would be trivial to replace, of course.

thanks!

Mike