Re: [libnbd PATCH v3 15/22] info: Update nbdinfo --map to use 64-bit block status

[Richard W.M. Jones]

On Thu, May 25, 2023 at 08:01:01AM -0500, Eric Blake wrote:
> Although we usually map "base:allocation" which doesn't require the
> use of the 64-bit API for flags, this application IS intended to map
> out other metacontexts that might have 64-bit flags.  And when
> extended headers are in use, we might as well ask for the server to
> give us extents as large as it wants, rather than breaking things up
> at 4G boundaries.
> 
> At the time this patch was written, there are no known servers that
> actually provide a metacontext with 64-bit flags.  However, that is
> planned for the nbdkit v3 protocol.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  info/map.c | 65 ++++++++++++++++++++++++++++--------------------------
>  1 file changed, 34 insertions(+), 31 deletions(-)
> 
> diff --git a/info/map.c b/info/map.c
> index 1169fce4..50b058f2 100644
> --- a/info/map.c
> +++ b/info/map.c
> @@ -36,13 +36,13 @@
> 
>  #include "nbdinfo.h"
> 
> -DEFINE_VECTOR_TYPE (uint32_vector, uint32_t);
> +DEFINE_VECTOR_TYPE (uint64_vector, uint64_t);
> 
> -static void print_extents (uint32_vector *entries);
> -static void print_totals (uint32_vector *entries, int64_t size);
> +static void print_extents (uint64_vector *entries);
> +static void print_totals (uint64_vector *entries, int64_t size);
>  static int extent_callback (void *user_data, const char *metacontext,
>                              uint64_t offset,
> -                            uint32_t *entries, size_t nr_entries,
> +                            nbd_extent *entries, size_t nr_entries,
>                              int *error);
> 
>  void
> @@ -50,7 +50,7 @@ do_map (void)
>  {
>    size_t i;
>    int64_t size;
> -  uint32_vector entries = empty_vector;
> +  uint64_vector entries = empty_vector;
>    uint64_t offset, align, max_len;
>    size_t prev_entries_size;
> 
> @@ -69,14 +69,16 @@ do_map (void)
>      fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
>      exit (EXIT_FAILURE);
>    }
> +  if (nbd_get_extended_headers_negotiated (nbd) == 1)
> +    max_len = size;
> 
>    for (offset = 0; offset < size;) {
>      prev_entries_size = entries.len;
> -    if (nbd_block_status (nbd, MIN (size - offset, max_len), offset,
> -                          (nbd_extent_callback) {
> -                            .callback = extent_callback,
> -                            .user_data = &entries },
> -                          0) == -1) {
> +    if (nbd_block_status_64 (nbd, MIN (size - offset, max_len), offset,
> +                             (nbd_extent64_callback) {
> +                               .callback = extent_callback,
> +                               .user_data = &entries },
> +                             0) == -1) {
>        fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
>        exit (EXIT_FAILURE);
>      }
> @@ -99,18 +101,18 @@ do_map (void)
>  }
> 
>  /* Callback handling --map. */
> -static void print_one_extent (uint64_t offset, uint64_t len, uint32_t type);
> -static void extent_description (const char *metacontext, uint32_t type,
> +static void print_one_extent (uint64_t offset, uint64_t len, uint64_t type);
> +static void extent_description (const char *metacontext, uint64_t type,
>                                  char **descr, bool *free_descr,
>                                  const char **fg, const char **bg);
> 
>  static int
>  extent_callback (void *user_data, const char *metacontext,
>                   uint64_t offset,
> -                 uint32_t *entries, size_t nr_entries,
> +                 nbd_extent *entries, size_t nr_entries,
>                   int *error)
>  {
> -  uint32_vector *list = user_data;
> +  uint64_vector *list = user_data;
>    size_t i;
> 
>    if (strcmp (metacontext, map) != 0)
> @@ -120,7 +122,8 @@ extent_callback (void *user_data, const char *metacontext,
>     * print_extents below.
>     */
>    for (i = 0; i < nr_entries; ++i) {
> -    if (uint32_vector_append (list, entries[i]) == -1) {
> +    if (uint64_vector_append (list, entries[i].length) == -1 ||
> +        uint64_vector_append (list, entries[i].flags) == -1) {
>        perror ("realloc");
>        exit (EXIT_FAILURE);
>      }
> @@ -129,7 +132,7 @@ extent_callback (void *user_data, const char *metacontext,
>  }
> 
>  static void
> -print_extents (uint32_vector *entries)
> +print_extents (uint64_vector *entries)
>  {
>    size_t i, j;
>    uint64_t offset = 0;          /* end of last extent printed + 1 */
> @@ -138,7 +141,7 @@ print_extents (uint32_vector *entries)
>    if (json_output) fprintf (fp, "[\n");
> 
>    for (i = 0; i < entries->len; i += 2) {
> -    uint32_t type = entries->ptr[last+1];
> +    uint64_t type = entries->ptr[last+1];
> 
>      /* If we're coalescing and the current type is different from the
>       * previous one then we should print everything up to this entry.
> @@ -157,7 +160,7 @@ print_extents (uint32_vector *entries)
> 
>    /* Print the last extent if there is one. */
>    if (last != i) {
> -    uint32_t type = entries->ptr[last+1];
> +    uint64_t type = entries->ptr[last+1];
>      uint64_t len;
> 
>      for (j = last, len = 0; j < i; j += 2)
> @@ -169,7 +172,7 @@ print_extents (uint32_vector *entries)
>  }
> 
>  static void
> -print_one_extent (uint64_t offset, uint64_t len, uint32_t type)
> +print_one_extent (uint64_t offset, uint64_t len, uint64_t type)
>  {
>    static bool comma = false;
>    char *descr;
> @@ -185,7 +188,7 @@ print_one_extent (uint64_t offset, uint64_t len, uint32_t 
> type)
>        ansi_colour (bg, fp);
>      fprintf (fp, "%10" PRIu64 "  "
>               "%10" PRIu64 "  "
> -             "%3" PRIu32,
> +             "%3" PRIu64,
>               offset, len, type);
>      if (descr)
>        fprintf (fp, "  %s", descr);
> @@ -199,7 +202,7 @@ print_one_extent (uint64_t offset, uint64_t len, uint32_t 
> type)
> 
>      fprintf (fp, "{ \"offset\": %" PRIu64 ", "
>               "\"length\": %" PRIu64 ", "
> -             "\"type\": %" PRIu32,
> +             "\"type\": %" PRIu64,
>               offset, len, type);
>      if (descr) {
>        fprintf (fp, ", \"description\": ");
> @@ -215,9 +218,9 @@ print_one_extent (uint64_t offset, uint64_t len, uint32_t 
> type)
> 
>  /* --map --totals suboption */
>  static void
> -print_totals (uint32_vector *entries, int64_t size)
> +print_totals (uint64_vector *entries, int64_t size)
>  {
> -  uint32_t type;
> +  uint64_t type;
>    bool comma = false;
> 
>    /* This is necessary to avoid a divide by zero below, but if the
> @@ -237,16 +240,16 @@ print_totals (uint32_vector *entries, int64_t size)
>     */
>    type = 0;
>    for (;;) {
> -    uint64_t next_type = (uint64_t)UINT32_MAX + 1;
> +    uint64_t next_type = 0;
>      uint64_t c = 0;
>      size_t i;
> 
>      for (i = 0; i < entries->len; i += 2) {
> -      uint32_t t = entries->ptr[i+1];
> +      uint64_t t = entries->ptr[i+1];
> 
>        if (t == type)
>          c += entries->ptr[i];
> -      else if (type < t && t < next_type)
> +      else if (type < t && (next_type == 0 || t < next_type))
>          next_type = t;
>      }
> 
> @@ -263,7 +266,7 @@ print_totals (uint32_vector *entries, int64_t size)
>            ansi_colour (fg, fp);
>          if (bg)
>            ansi_colour (bg, fp);
> -        fprintf (fp, "%10" PRIu64 " %5.1f%% %3" PRIu32,
> +        fprintf (fp, "%10" PRIu64 " %5.1f%% %3" PRIu64,
>                   c, percent, type);
>          if (descr)
>            fprintf (fp, " %s", descr);
> @@ -278,7 +281,7 @@ print_totals (uint32_vector *entries, int64_t size)
>          fprintf (fp,
>                   "{ \"size\": %" PRIu64 ", "
>                   "\"percent\": %g, "
> -                 "\"type\": %" PRIu32,
> +                 "\"type\": %" PRIu64,
>                   c, percent, type);
>          if (descr) {
>            fprintf (fp, ", \"description\": ");
> @@ -292,7 +295,7 @@ print_totals (uint32_vector *entries, int64_t size)
>          free (descr);
>      }
> 
> -    if (next_type == (uint64_t)UINT32_MAX + 1)
> +    if (next_type == 0)
>        break;
>      type = next_type;
>    }
> @@ -301,7 +304,7 @@ print_totals (uint32_vector *entries, int64_t size)
>  }
> 
>  static void
> -extent_description (const char *metacontext, uint32_t type,
> +extent_description (const char *metacontext, uint64_t type,
>                      char **descr, bool *free_descr,
>                      const char **fg, const char **bg)
>  {
> @@ -348,7 +351,7 @@ extent_description (const char *metacontext, uint32_t 
> type,
>        *fg = ANSI_FG_BRIGHT_WHITE; *bg = ANSI_BG_BLACK;
>        return;
>      default:
> -      if (asprintf (descr, "backing depth %u", type) == -1) {
> +      if (asprintf (descr, "backing depth %" PRIu64, type) == -1) {
>          perror ("asprintf");
>          exit (EXIT_FAILURE);
>        }

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit