mbox series

[v4,0/7] Exposing backing-chain allocation over NBD

Message ID 20201009215533.1194742-1-eblake@redhat.com
Headers show
Series Exposing backing-chain allocation over NBD | expand

Message

Eric Blake Oct. 9, 2020, 9:55 p.m. UTC
v3 was here:
https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg02305.html

Since then:
rebase to master
- patch 1 is new, fixing a theoretical bug in QAPI interaction and
 simplifying later patches
- patch 2, 4, and 6 are renamed to favor the term 'metadata context'
 [Markus], sadly 'git backport-diff' can't see through renames

Based-on: <20201009200720.1169904-1-eblake@redhat.com>
([PULL v3 0/8] NBD patches through 2020-10-08)

Also available at:
https://repo.or.cz/qemu/ericb.git/shortlog/refs/tags/nbd-alloc-depth-v4

001/7:[down] 'nbd: Utilize QAPI_CLONE for type conversion'
002/7:[down] 'nbd: Add new qemu:allocation-depth metadata context'
003/7:[0042] [FC] 'nbd: Add 'qemu-nbd -A' to expose allocation depth'
004/7:[down] 'nbd: Update qapi to support exporting multiple bitmaps'
005/7:[----] [-C] 'nbd: Simplify qemu bitmap context name'
006/7:[down] 'nbd: Refactor counting of metadata contexts'
007/7:[----] [-C] 'nbd: Allow export of multiple bitmaps for one device'

Eric Blake (7):
  nbd: Utilize QAPI_CLONE for type conversion
  nbd: Add new qemu:allocation-depth metadata context
  nbd: Add 'qemu-nbd -A' to expose allocation depth
  nbd: Update qapi to support exporting multiple bitmaps
  nbd: Simplify qemu bitmap context name
  nbd: Refactor counting of metadata contexts
  nbd: Allow export of multiple bitmaps for one device

 docs/interop/nbd.txt       |  27 ++++-
 docs/tools/qemu-nbd.rst    |   8 +-
 qapi/block-core.json       |   7 +-
 qapi/block-export.json     |  22 +++-
 include/block/nbd.h        |  12 ++-
 blockdev-nbd.c             |  29 +++--
 nbd/server.c               | 210 +++++++++++++++++++++++++++++--------
 qemu-nbd.c                 |  33 ++++--
 tests/qemu-iotests/291     |   6 +-
 tests/qemu-iotests/309     |  73 +++++++++++++
 tests/qemu-iotests/309.out |  22 ++++
 tests/qemu-iotests/group   |   1 +
 12 files changed, 364 insertions(+), 86 deletions(-)
 create mode 100755 tests/qemu-iotests/309
 create mode 100644 tests/qemu-iotests/309.out

Comments

Eric Blake Oct. 27, 2020, 3:33 p.m. UTC | #1
On 10/16/20 10:23 AM, Eric Blake wrote:
> A rather trivial decoding; we may enhance it further if qemu extends
> things to give an integer depth alongside its tri-state encoding.
> ---
> 
> I'll wait to push this to libnbd until the counterpart qemu patches
> land upstream, although it looks like I've got positive review.

Whoops, I accidentally pushed this before qemu stuff landed upstream,
and in the meantime, we changed our minds on what to expose over
qemu:allocation-depth to be a bare integer rather than a tri-state.
I'll push this followup (but this time, wait for the actual qemu patch
to land).  In fact, I should probably add test-suite coverage...


From eba8734654e6fd340e18b3e07c3213ed1a0ab9e8 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Tue, 27 Oct 2020 10:27:25 -0500
Subject: [libnbd PATCH] info: Adjust to actual 'qemu-nbd -A' semantics

Review on the qemu list has led to an altered definition of what
'qemu:allocation-depth' should report: rather than a tri-state value,
it is an actual depth.  It's time to match what actually got committed
into qemu, which in turn means a slight refactoring to use a malloc'd
string for a description.

Fixes: 71455c021
---
 info/nbdinfo.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index 2b22f51..b152f28 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -767,28 +767,30 @@ get_content (struct nbd_handle *nbd, int64_t size)
 }

 /* Callback handling --map. */
-static const char *
+static char *
 extent_description (const char *metacontext, uint32_t type)
 {
+  char *ret;
+
   if (strcmp (metacontext, "base:allocation") == 0) {
     switch (type) {
-    case 0: return "allocated";
-    case 1: return "hole";
-    case 2: return "zero";
-    case 3: return "hole,zero";
+    case 0: return strdup ("allocated");
+    case 1: return strdup ("hole");
+    case 2: return strdup ("zero");
+    case 3: return strdup ("hole,zero");
     }
   }
   else if (strncmp (metacontext, "qemu:dirty-bitmap:", 18) == 0) {
     switch (type) {
-    case 0: return "clean";
-    case 1: return "dirty";
+    case 0: return strdup ("clean");
+    case 1: return strdup ("dirty");
     }
   }
   else if (strcmp (metacontext, "qemu:allocation-depth") == 0) {
-    switch (type & 3) {
-    case 0: return "unallocated";
-    case 1: return "local";
-    case 2: return "backing";
+    switch (type) {
+    case 0: return strdup ("unallocated");
+    case 1: return strdup ("local");
+    case 2: asprintf (&ret, "backing depth %d", type); return ret;
     }
   }

@@ -810,7 +812,7 @@ extent_callback (void *user_data, const char
*metacontext,

   /* Print the entries received. */
   for (i = 0; i < nr_entries; i += 2) {
-    const char *descr = extent_description (map, entries[i+1]);
+    char *descr = extent_description (map, entries[i+1]);

     if (!json_output) {
       fprintf (fp, "%10" PRIu64 "  "
@@ -837,6 +839,7 @@ extent_callback (void *user_data, const char
*metacontext,
       comma = true;
     }

+    free (descr);
     offset += entries[i];
   }
Richard W.M. Jones Oct. 27, 2020, 7:40 p.m. UTC | #2
On Tue, Oct 27, 2020 at 10:33:48AM -0500, Eric Blake wrote:
> On 10/16/20 10:23 AM, Eric Blake wrote:
> > A rather trivial decoding; we may enhance it further if qemu extends
> > things to give an integer depth alongside its tri-state encoding.
> > ---
> > 
> > I'll wait to push this to libnbd until the counterpart qemu patches
> > land upstream, although it looks like I've got positive review.
> 
> Whoops, I accidentally pushed this before qemu stuff landed upstream,
> and in the meantime, we changed our minds on what to expose over
> qemu:allocation-depth to be a bare integer rather than a tri-state.
> I'll push this followup (but this time, wait for the actual qemu patch
> to land).  In fact, I should probably add test-suite coverage...

ACK.  I have a patch which touches this file but it's a simple merge
to combine the two changes.

Rich.

> >From eba8734654e6fd340e18b3e07c3213ed1a0ab9e8 Mon Sep 17 00:00:00 2001
> From: Eric Blake <eblake@redhat.com>
> Date: Tue, 27 Oct 2020 10:27:25 -0500
> Subject: [libnbd PATCH] info: Adjust to actual 'qemu-nbd -A' semantics
> 
> Review on the qemu list has led to an altered definition of what
> 'qemu:allocation-depth' should report: rather than a tri-state value,
> it is an actual depth.  It's time to match what actually got committed
> into qemu, which in turn means a slight refactoring to use a malloc'd
> string for a description.
> 
> Fixes: 71455c021
> ---
>  info/nbdinfo.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/info/nbdinfo.c b/info/nbdinfo.c
> index 2b22f51..b152f28 100644
> --- a/info/nbdinfo.c
> +++ b/info/nbdinfo.c
> @@ -767,28 +767,30 @@ get_content (struct nbd_handle *nbd, int64_t size)
>  }
> 
>  /* Callback handling --map. */
> -static const char *
> +static char *
>  extent_description (const char *metacontext, uint32_t type)
>  {
> +  char *ret;
> +
>    if (strcmp (metacontext, "base:allocation") == 0) {
>      switch (type) {
> -    case 0: return "allocated";
> -    case 1: return "hole";
> -    case 2: return "zero";
> -    case 3: return "hole,zero";
> +    case 0: return strdup ("allocated");
> +    case 1: return strdup ("hole");
> +    case 2: return strdup ("zero");
> +    case 3: return strdup ("hole,zero");
>      }
>    }
>    else if (strncmp (metacontext, "qemu:dirty-bitmap:", 18) == 0) {
>      switch (type) {
> -    case 0: return "clean";
> -    case 1: return "dirty";
> +    case 0: return strdup ("clean");
> +    case 1: return strdup ("dirty");
>      }
>    }
>    else if (strcmp (metacontext, "qemu:allocation-depth") == 0) {
> -    switch (type & 3) {
> -    case 0: return "unallocated";
> -    case 1: return "local";
> -    case 2: return "backing";
> +    switch (type) {
> +    case 0: return strdup ("unallocated");
> +    case 1: return strdup ("local");
> +    case 2: asprintf (&ret, "backing depth %d", type); return ret;
>      }
>    }
> 
> @@ -810,7 +812,7 @@ extent_callback (void *user_data, const char
> *metacontext,
> 
>    /* Print the entries received. */
>    for (i = 0; i < nr_entries; i += 2) {
> -    const char *descr = extent_description (map, entries[i+1]);
> +    char *descr = extent_description (map, entries[i+1]);
> 
>      if (!json_output) {
>        fprintf (fp, "%10" PRIu64 "  "
> @@ -837,6 +839,7 @@ extent_callback (void *user_data, const char
> *metacontext,
>        comma = true;
>      }
> 
> +    free (descr);
>      offset += entries[i];
>    }
> 
> -- 
> 2.29.0
> 
> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs