[Qemu-devel] [PATCH v2 0/2] test NBD bitmap export

[Qemu-devel] [PATCH v2 0/2] test NBD bitmap export

[Eric Blake]

This is a less hacky version of my first attempt:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg05993.html

The worst part of the hack (taking a semicolon-separated list of
requests to hand the server) is completely dropped, the member
field is renamed to reflect what it is doing, and I've added an
iotests to help us in regression testing.

I'd REALLY like to send a pull request before soft freeze (tomorrow
morning is approaching fast!) that includes these patches, so that
qemu 3.0 at least has experimental access to all the pieces needed
for playing with incremental backups.

Eric Blake (2):
  nbd/client: Add x-dirty-bitmap to query bitmap from server
  iotests: New test 223 for exporting dirty bitmap over NBD

 qapi/block-core.json       |   7 ++-
 block/nbd-client.h         |   1 +
 include/block/nbd.h        |   1 +
 block/nbd-client.c         |   3 +
 block/nbd.c                |  10 +++-
 nbd/client.c               |   4 +-
 tests/qemu-iotests/223     | 138 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/223.out |  49 ++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 9 files changed, 209 insertions(+), 5 deletions(-)
 create mode 100755 tests/qemu-iotests/223
 create mode 100644 tests/qemu-iotests/223.out

-- 
2.14.4

[Qemu-devel] [PATCH v2 1/2] nbd/client: Add x-dirty-bitmap to query bitmap from server

[Eric Blake]

In order to test that the NBD server is properly advertising
dirty bitmaps, we need a bare minimum client that can request
and read the context.  Since feature freeze for 3.0 is imminent,
this is the smallest workable patch, which replaces the qemu
block status report with the results of the NBD server's dirty
bitmap (making it very easy to use 'qemu-img map --output=json'
to learn where the dirty portions are).  Note that the NBD
protocol defines a dirty section with the same bit but opposite
sense that normal "base:allocation" uses to report an allocated
section; so in qemu-img map output, "data":true corresponds to
clean, "data":false corresponds to dirty.

A more complete solution that allows dirty bitmaps to be queried
at the same time as normal block status will be required before
this addition can lose the x- prefix.  Until then, the fact that
this replaces normal status with dirty status means actions
like 'qemu-img convert' will likely misbehave due to treating
dirty regions of the file as if they are unallocated.

The next patch adds an iotest to exercise this new code.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

 qapi/block-core.json |  7 ++++++-
 block/nbd-client.h   |  1 +
 include/block/nbd.h  |  1 +
 block/nbd-client.c   |  3 +++
 block/nbd.c          | 10 ++++++++--
 nbd/client.c         |  4 ++--
 6 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 577ce5e9991..90e554ed0ff 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3471,12 +3471,17 @@
 #
 # @tls-creds:   TLS credentials ID
 #
+# @x-dirty-bitmap: A "qemu:dirty-bitmap:NAME" string to query in place of
+#                  traditional "base:allocation" block status (see
+#                  NBD_OPT_LIST_META_CONTEXT in the NBD protocol) (since 3.0)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsNbd',
   'data': { 'server': 'SocketAddress',
             '*export': 'str',
-            '*tls-creds': 'str' } }
+            '*tls-creds': 'str',
+            '*x-dirty-bitmap': 'str' } }

 ##
 # @BlockdevOptionsRaw:
diff --git a/block/nbd-client.h b/block/nbd-client.h
index 0ece76e5aff..cfc90550b99 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -45,6 +45,7 @@ int nbd_client_init(BlockDriverState *bs,
                     const char *export_name,
                     QCryptoTLSCreds *tlscreds,
                     const char *hostname,
+                    const char *x_dirty_bitmap,
                     Error **errp);
 void nbd_client_close(BlockDriverState *bs);

diff --git a/include/block/nbd.h b/include/block/nbd.h
index daaeae61bf9..4638c839f51 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -259,6 +259,7 @@ static inline bool nbd_reply_type_is_error(int type)
 struct NBDExportInfo {
     /* Set by client before nbd_receive_negotiate() */
     bool request_sizes;
+    char *x_dirty_bitmap;

     /* In-out fields, set by client before nbd_receive_negotiate() and
      * updated by server results during nbd_receive_negotiate() */
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 8d69eaaa32f..9686ecbd5ee 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -970,6 +970,7 @@ int nbd_client_init(BlockDriverState *bs,
                     const char *export,
                     QCryptoTLSCreds *tlscreds,
                     const char *hostname,
+                    const char *x_dirty_bitmap,
                     Error **errp)
 {
     NBDClientSession *client = nbd_get_client_session(bs);
@@ -982,9 +983,11 @@ int nbd_client_init(BlockDriverState *bs,
     client->info.request_sizes = true;
     client->info.structured_reply = true;
     client->info.base_allocation = true;
+    client->info.x_dirty_bitmap = g_strdup(x_dirty_bitmap);
     ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), export,
                                 tlscreds, hostname,
                                 &client->ioc, &client->info, errp);
+    g_free(client->info.x_dirty_bitmap);
     if (ret < 0) {
         logout("Failed to negotiate with the NBD server\n");
         return ret;
diff --git a/block/nbd.c b/block/nbd.c
index 13db4030e67..b198ad775fb 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -378,6 +378,12 @@ static QemuOptsList nbd_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "ID of the TLS credentials to use",
         },
+        {
+            .name = "x-dirty-bitmap",
+            .type = QEMU_OPT_STRING,
+            .help = "experimental: expose named dirty bitmap in place of "
+                    "block status",
+        },
         { /* end of list */ }
     },
 };
@@ -438,8 +444,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
     }

     /* NBD handshake */
-    ret = nbd_client_init(bs, sioc, s->export,
-                          tlscreds, hostname, errp);
+    ret = nbd_client_init(bs, sioc, s->export, tlscreds, hostname,
+                          qemu_opt_get(opts, "x-dirty-bitmap"), errp);
  error:
     if (sioc) {
         object_unref(OBJECT(sioc));
diff --git a/nbd/client.c b/nbd/client.c
index 232ff4f46da..40b74d9761f 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2016-2017 Red Hat, Inc.
+ *  Copyright (C) 2016-2018 Red Hat, Inc.
  *  Copyright (C) 2005  Anthony Liguori <anthony@codemonkey.ws>
  *
  *  Network Block Device Client Side
@@ -831,7 +831,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name,

             if (info->structured_reply && base_allocation) {
                 result = nbd_negotiate_simple_meta_context(
-                        ioc, name, "base:allocation",
+                        ioc, name, info->x_dirty_bitmap ?: "base:allocation",
                         &info->meta_base_allocation_id, errp);
                 if (result < 0) {
                     goto fail;
-- 
2.14.4

[Qemu-devel] [PATCH v2 2/2] iotests: New test 223 for exporting dirty bitmap over NBD

[Eric Blake]

Although this test is NOT a full test of image fleecing (as it
intentionally uses just a single block device directly exported
over NBD, rather than trying to set up a blockdev-backup job with
multiple BDS involved), it DOES prove that qemu as a server is
able to properly expose a dirty bitmap over NBD.

When coupled with image fleecing, it is then possible for a
third-party client to do an incremental backup by using
qemu-img map with the x-dirty-bitmap option to learn which parts
of the file are dirty (perhaps confusingly, they are the portions
mapped as "data":false - which is part of the reason this is
still in the x- experimental namespace), along with another
normal client (perhaps 'qemu-nbd -c' to expose the server over
/dev/nbd0 and then just use normal I/O on that block device) to
read the dirty sections.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/223     | 138 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/223.out |  49 ++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 188 insertions(+)
 create mode 100755 tests/qemu-iotests/223
 create mode 100644 tests/qemu-iotests/223.out

diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
new file mode 100755
index 00000000000..b63b7a4f9e1
--- /dev/null
+++ b/tests/qemu-iotests/223
@@ -0,0 +1,138 @@
+#!/bin/bash
+#
+# Test reading dirty bitmap over NBD
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1 # failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    _cleanup_qemu
+    rm -f "$TEST_DIR/nbd"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_proto file # uses NBD as well
+_supported_os Linux
+
+function do_run_qemu()
+{
+    echo Testing: "$@"
+    $QEMU -nographic -qmp stdio -serial none "$@"
+    echo
+}
+
+function run_qemu()
+{
+    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
+                          | _filter_qemu | _filter_imgfmt \
+                          | _filter_actual_image_size
+}
+
+echo
+echo "=== Create partially sparse image, then add dirty bitmap ==="
+echo
+
+_make_test_img 4M
+$QEMU_IO -c 'w -P 0x11 1M 2M' "$TEST_IMG" | _filter_qemu_io
+run_qemu <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "blockdev-add",
+  "arguments": {
+    "driver": "$IMGFMT",
+    "node-name": "n",
+    "file": {
+      "driver": "file",
+      "filename": "$TEST_IMG"
+    }
+  }
+}
+{ "execute": "block-dirty-bitmap-add",
+  "arguments": {
+    "node": "n",
+    "name": "b",
+    "persistent": true
+  }
+}
+{ "execute": "quit" }
+EOF
+
+echo
+echo "=== Write part of the file under active bitmap ==="
+echo
+
+$QEMU_IO -c 'w -P 0x22 2M 2M' "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "=== End dirty bitmap, and start serving image over NBD ==="
+echo
+
+_launch_qemu 2> >(_filter_nbd)
+
+silent=
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"qmp_capabilities"}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"blockdev-add",
+  "arguments":{"driver":"qcow2", "node-name":"n",
+    "file":{"driver":"file", "filename":"'"$TEST_IMG"'"}}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
+  "arguments":{"node":"n", "name":"b"}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start",
+  "arguments":{"addr":{"type":"unix",
+    "data":{"path":"'"$TEST_DIR/nbd"'"}}}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
+  "arguments":{"device":"n"}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap",
+  "arguments":{"name":"n", "bitmap":"b"}}' "return"
+
+echo
+echo "=== Contrast normal status with dirty-bitmap status ==="
+echo
+
+QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
+IMG="driver=nbd,export=n,server.type=unix,server.path=$TEST_DIR/nbd"
+$QEMU_IO -r -c 'r -P 0 0 1m' -c 'r -P 0x11 1m 1m' \
+  -c 'r -P 0x22 2m 2m' --image-opts "$IMG" | _filter_qemu_io
+$QEMU_IMG map --output=json --image-opts \
+  "$IMG" | _filter_qemu_img_map
+$QEMU_IMG map --output=json --image-opts \
+  "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b" | _filter_qemu_img_map
+
+echo
+echo "=== End NBD server ==="
+echo
+
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove",
+  "arguments":{"name":"n"}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-stop"}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"quit"}' "return"
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
new file mode 100644
index 00000000000..33021c8e6a1
--- /dev/null
+++ b/tests/qemu-iotests/223.out
@@ -0,0 +1,49 @@
+QA output created by 223
+
+=== Create partially sparse image, then add dirty bitmap ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
+wrote 2097152/2097152 bytes at offset 1048576
+2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+
+=== Write part of the file under active bitmap ===
+
+wrote 2097152/2097152 bytes at offset 2097152
+2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== End dirty bitmap, and start serving image over NBD ===
+
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+
+=== Contrast normal status with dirty-bitmap status ===
+
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2097152/2097152 bytes at offset 2097152
+2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false},
+{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true}]
+[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true},
+{ "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]
+
+=== End NBD server ===
+
+{"return": {}}
+{"return": {}}
+{"return": {}}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index eea75819d2a..a446476583e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -220,3 +220,4 @@
 218 rw auto quick
 219 rw auto
 221 rw auto quick
+223 rw auto quick
-- 
2.14.4

Re: [Qemu-devel] [PATCH v2 0/2] test NBD bitmap export

[Eric Blake]

On 07/02/2018 02:14 PM, Eric Blake wrote:
> This is a less hacky version of my first attempt:
> https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg05993.html
> 
> The worst part of the hack (taking a semicolon-separated list of
> requests to hand the server) is completely dropped, the member
> field is renamed to reflect what it is doing, and I've added an
> iotests to help us in regression testing.
> 
> I'd REALLY like to send a pull request before soft freeze (tomorrow
> morning is approaching fast!) that includes these patches, so that
> qemu 3.0 at least has experimental access to all the pieces needed
> for playing with incremental backups.
> 
> Eric Blake (2):
>    nbd/client: Add x-dirty-bitmap to query bitmap from server
>    iotests: New test 223 for exporting dirty bitmap over NBD

For patchew (maybe? does patchew run iotests?)

Based-on: 
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg06747.html
nbd/server: Fix dirty bitmap logic regression

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 2/2] iotests: New test 223 for exporting dirty bitmap over NBD

[John Snow]

On 07/02/2018 03:14 PM, Eric Blake wrote:
> Although this test is NOT a full test of image fleecing (as it
> intentionally uses just a single block device directly exported
> over NBD, rather than trying to set up a blockdev-backup job with
> multiple BDS involved), it DOES prove that qemu as a server is
> able to properly expose a dirty bitmap over NBD.
> 
> When coupled with image fleecing, it is then possible for a
> third-party client to do an incremental backup by using
> qemu-img map with the x-dirty-bitmap option to learn which parts
> of the file are dirty (perhaps confusingly, they are the portions
> mapped as "data":false - which is part of the reason this is
> still in the x- experimental namespace), along with another
> normal client (perhaps 'qemu-nbd -c' to expose the server over
> /dev/nbd0 and then just use normal I/O on that block device) to
> read the dirty sections.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/qemu-iotests/223     | 138 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/223.out |  49 ++++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 188 insertions(+)
>  create mode 100755 tests/qemu-iotests/223
>  create mode 100644 tests/qemu-iotests/223.out
> 
> diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
> new file mode 100755
> index 00000000000..b63b7a4f9e1
> --- /dev/null
> +++ b/tests/qemu-iotests/223
> @@ -0,0 +1,138 @@
> +#!/bin/bash
> +#
> +# Test reading dirty bitmap over NBD
> +#
> +# Copyright (C) 2018 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +here="$PWD"
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> +    _cleanup_test_img
> +    _cleanup_qemu
> +    rm -f "$TEST_DIR/nbd"
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.qemu
> +
> +_supported_fmt qcow2
> +_supported_proto file # uses NBD as well
> +_supported_os Linux
> +
> +function do_run_qemu()
> +{
> +    echo Testing: "$@"
> +    $QEMU -nographic -qmp stdio -serial none "$@"
> +    echo
> +}
> +
> +function run_qemu()
> +{
> +    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
> +                          | _filter_qemu | _filter_imgfmt \
> +                          | _filter_actual_image_size
> +}
> +
> +echo
> +echo "=== Create partially sparse image, then add dirty bitmap ==="
> +echo
> +
> +_make_test_img 4M
> +$QEMU_IO -c 'w -P 0x11 1M 2M' "$TEST_IMG" | _filter_qemu_io

- Write 0x11 from [1M, 3M), not recorded by a bitmap.

> +run_qemu <<EOF
> +{ "execute": "qmp_capabilities" }
> +{ "execute": "blockdev-add",
> +  "arguments": {
> +    "driver": "$IMGFMT",
> +    "node-name": "n",
> +    "file": {
> +      "driver": "file",
> +      "filename": "$TEST_IMG"
> +    }
> +  }
> +}
> +{ "execute": "block-dirty-bitmap-add",
> +  "arguments": {
> +    "node": "n",
> +    "name": "b",

Saving a few precious bytes.

> +    "persistent": true
> +  }
> +}
> +{ "execute": "quit" }
> +EOF
> +
> +echo
> +echo "=== Write part of the file under active bitmap ==="
> +echo
> +
> +$QEMU_IO -c 'w -P 0x22 2M 2M' "$TEST_IMG" | _filter_qemu_io
> +

- Write 0x22 to [2M, 4M) recorded by the bitmap.

> +echo
> +echo "=== End dirty bitmap, and start serving image over NBD ==="
> +echo
> +
> +_launch_qemu 2> >(_filter_nbd)
> +
> +silent=
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"qmp_capabilities"}' "return"
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"blockdev-add",
> +  "arguments":{"driver":"qcow2", "node-name":"n",
> +    "file":{"driver":"file", "filename":"'"$TEST_IMG"'"}}}' "return"
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
> +  "arguments":{"node":"n", "name":"b"}}' "return"
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start",
> +  "arguments":{"addr":{"type":"unix",
> +    "data":{"path":"'"$TEST_DIR/nbd"'"}}}}' "return"
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
> +  "arguments":{"device":"n"}}' "return"
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap",
> +  "arguments":{"name":"n", "bitmap":"b"}}' "return"
> +

So far, so good.

> +echo
> +echo "=== Contrast normal status with dirty-bitmap status ==="
> +echo
> +
> +QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
> +IMG="driver=nbd,export=n,server.type=unix,server.path=$TEST_DIR/nbd"
> +$QEMU_IO -r -c 'r -P 0 0 1m' -c 'r -P 0x11 1m 1m' \
> +  -c 'r -P 0x22 2m 2m' --image-opts "$IMG" | _filter_qemu_io

Confirming that we've got 0x11 from [1M, 2M) and 0x22 from [2M, 4M).

> +$QEMU_IMG map --output=json --image-opts \
> +  "$IMG" | _filter_qemu_img_map

Normal allocation map. Ought to show [1M, 4M).

> +$QEMU_IMG map --output=json --image-opts \
> +  "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b" | _filter_qemu_img_map

Hacked bitmap allocation map. Ought to show [2M, 4M).

> +
> +echo
> +echo "=== End NBD server ==="
> +echo
> +
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove",
> +  "arguments":{"name":"n"}}' "return"
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-stop"}' "return"
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"quit"}' "return"
> +
> +# success, all done
> +echo '*** done'
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
> new file mode 100644
> index 00000000000..33021c8e6a1
> --- /dev/null
> +++ b/tests/qemu-iotests/223.out
> @@ -0,0 +1,49 @@
> +QA output created by 223
> +
> +=== Create partially sparse image, then add dirty bitmap ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
> +wrote 2097152/2097152 bytes at offset 1048576
> +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Testing:
> +QMP_VERSION
> +{"return": {}}
> +{"return": {}}
> +{"return": {}}
> +{"return": {}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
> +
> +
> +=== Write part of the file under active bitmap ===
> +
> +wrote 2097152/2097152 bytes at offset 2097152
> +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +=== End dirty bitmap, and start serving image over NBD ===
> +
> +{"return": {}}
> +{"return": {}}
> +{"return": {}}
> +{"return": {}}
> +{"return": {}}
> +{"return": {}}
> +
> +=== Contrast normal status with dirty-bitmap status ===
> +
> +read 1048576/1048576 bytes at offset 0
> +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 1048576/1048576 bytes at offset 1048576
> +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 2097152/2097152 bytes at offset 2097152
> +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false},
> +{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true}]

Looks right.

> +[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true},
> +{ "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]

Also looks right.

> +
> +=== End NBD server ===
> +
> +{"return": {}}
> +{"return": {}}
> +{"return": {}}
> +*** done
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index eea75819d2a..a446476583e 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -220,3 +220,4 @@
>  218 rw auto quick
>  219 rw auto
>  221 rw auto quick
> +223 rw auto quick
> 

It Works!:

Tested-by: John Snow <jsnow@redhat.com>

Tests what it aims to:

Reviewed-by: John Snow <jsnow@redhat.com>


I think the trick will be combining 222 and 223 into one workflow. I
think we might be missing a piece.

Consider this:

- We have some image which has a bitmap 'B' tracking writes since the
last incremental backup was made or fleeced, using the traditional "one
bitmap per backup regimen" strategy.

- We prepare to fleece by creating a new temporary store for fleecing,
and add the target drive as a backing node.

At this point, there's no point-in-time established just yet; we're
still "live" and so is the bitmap.

- We run blockdev-backup sync=none to start achieving PIT semantics for
the fleecing node.

- We start the NBD server, and add the fleecing node export.

- We attempt to link the bitmap to the NBD export, but we can't! It's
still active, and we never froze it for the PIT.



We need a way to freeze this bitmap manually like in sync=incremental,
to associate it with this PIT... or, we need to allow the "merge"
command to operate under the transaction so we can copy a bitmap out at
that PIT, e.g.

- Create bitmap "foo"
- Disable bitmap "foo"
Transaction {
  - blockdev-backup sync=none
  - merge "B" into "foo"
}
- NBD start, export, add bitmap "foo"



I think there's no escaping that we need at least one of the following:

- Merge (or copy) in transactions
- Manual bitmap freeze/thaw commands (transactionable)
- blockdev-fleece job & QMP command that does the following items:
	- Handles the creating of a temporary fleecing node
	- Establishes the blockdev-backup semantics for PIT export
	- Accepts a bitmap and forks it just like blockdev-backup does

In the case of the block job, we don't need to tie it to NBD
necessarily; we can leave it as the creation of the node. This would
allow us to export it over a different transport later.

The "add bitmap to NBD server" mechanism would then change to accept
either a disabled OR frozen bitmap.

Canceling/completing the job manually dictates if we clear the bitmap or
merge in the new changes, just like blockdev-backup.

The job could be implemented as a form of blockdev-backup with most of
the same code, but some new setup/teardown code.

That's a fanciful operation though, and too late for 3.0.

Re: [Qemu-devel] [PATCH v2 2/2] iotests: New test 223 for exporting dirty bitmap over NBD

[Eric Blake]

On 07/02/2018 04:27 PM, John Snow wrote:

>> +{ "execute": "block-dirty-bitmap-add",
>> +  "arguments": {
>> +    "node": "n",
>> +    "name": "b",
> 
> Saving a few precious bytes.

Would "mynode" and "mybitmap" be any friendlier? :)

> 
>> +    "persistent": true
>> +  }
>> +}
>> +{ "execute": "quit" }
>> +EOF
>> +
>> +echo
>> +echo "=== Write part of the file under active bitmap ==="
>> +echo
>> +
>> +$QEMU_IO -c 'w -P 0x22 2M 2M' "$TEST_IMG" | _filter_qemu_io
>> +
> 
> - Write 0x22 to [2M, 4M) recorded by the bitmap.

I debated about playing with smaller writes, since the bitmap tracking 
rounds up to bitmap granularity. But for this test, sticking to aligned 
boundaries seemed easiest (and I think we have other tests of bitmap 
granularities, although I didn't search today).

>> +=== Contrast normal status with dirty-bitmap status ===
>> +
>> +read 1048576/1048576 bytes at offset 0
>> +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +read 1048576/1048576 bytes at offset 1048576
>> +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +read 2097152/2097152 bytes at offset 2097152
>> +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false},
>> +{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true}]
> 
> Looks right.

Note for potential future test: discarding a block should also show up 
as a dirty bitmap change, even though it would disappear from the normal 
block status allocated.

> 
> It Works!:
> 
> Tested-by: John Snow <jsnow@redhat.com>
> 
> Tests what it aims to:
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 

Thanks; I'll queue this through my NBD tree and send a pull request in a 
few hours.

> 
> I think the trick will be combining 222 and 223 into one workflow. I
> think we might be missing a piece.
> 
> Consider this:
> 
> - We have some image which has a bitmap 'B' tracking writes since the
> last incremental backup was made or fleeced, using the traditional "one
> bitmap per backup regimen" strategy.
> 
> - We prepare to fleece by creating a new temporary store for fleecing,
> and add the target drive as a backing node.
> 
> At this point, there's no point-in-time established just yet; we're
> still "live" and so is the bitmap.
> 
> - We run blockdev-backup sync=none to start achieving PIT semantics for
> the fleecing node.
> 
> - We start the NBD server, and add the fleecing node export.
> 
> - We attempt to link the bitmap to the NBD export, but we can't! It's
> still active, and we never froze it for the PIT.

We do have this:

transaction {
  - x-block-dirty-bitmap-disable B1
  - block-dirty-bitmap-add B2
  - blockdev-backup sync=none
}

which starts bitmap B2 from the same point in time that we used the 
disabled bitmap B1.  We can then do what we want with B1 (perhaps 
copying it elsewhere or merging in other bitmaps), taking all the time 
we need before finally exporting a bitmap over NBD, because it was 
frozen at the same PIT, _and_ we are still tracking changes since then 
via B2.

Later, if we change our mind (for example, if creating the NBD server 
failed, or the 3rd party didn't get to read everything they wanted), 
then we want to merge B1 and B2 back into a single live bitmap so that 
the next attempt still has a single bitmap tracking all changes since 
the PIT that B1 was created.  But for that, we can first re-enable B1, 
then use x-block-dirty-bitmap-merge to merge B2 into B1, then 
block-dirty-bitmap-remove B2.  Although that cleanup sequence is not 
transactionable, it doesn't have to be (we temporarily have two bitmaps 
running at once, but the merge of those two bitmaps is going to be the 
same whether done atomically or whether one or the other got a few more 
dirty clusters in the meantime).

So, I think we have the pieces we need, as long as we have a working 
3-way transaction to disable the old, create a new, and start 
blockdev-sync, and nothing else is tied to a point in time.

> 
> 
> 
> We need a way to freeze this bitmap manually like in sync=incremental,
> to associate it with this PIT... or, we need to allow the "merge"
> command to operate under the transaction so we can copy a bitmap out at
> that PIT, e.g.
> 
> - Create bitmap "foo"
> - Disable bitmap "foo"
> Transaction {
>    - blockdev-backup sync=none
>    - merge "B" into "foo"
> }
> - NBD start, export, add bitmap "foo"
> 
> 
> 
> I think there's no escaping that we need at least one of the following:
> 
> - Merge (or copy) in transactions
> - Manual bitmap freeze/thaw commands (transactionable)
> - blockdev-fleece job & QMP command that does the following items:
> 	- Handles the creating of a temporary fleecing node
> 	- Establishes the blockdev-backup semantics for PIT export
> 	- Accepts a bitmap and forks it just like blockdev-backup does

Having one QMP command that does all things in order might be nice, but 
I think we're okay with our existing building blocks.

> 
> In the case of the block job, we don't need to tie it to NBD
> necessarily; we can leave it as the creation of the node. This would
> allow us to export it over a different transport later.
> 
> The "add bitmap to NBD server" mechanism would then change to accept
> either a disabled OR frozen bitmap.

Right now, NBD bitmap export refuses a live bitmap, and accepts a 
disabled bitmap. I'm not sure if it accepts a frozen bitmap.

> 
> Canceling/completing the job manually dictates if we clear the bitmap or
> merge in the new changes, just like blockdev-backup.
> 
> The job could be implemented as a form of blockdev-backup with most of
> the same code, but some new setup/teardown code.
> 
> That's a fanciful operation though, and too late for 3.0.

Yeah, any further polish will have to come in 3.1 (and also dropping the 
x- prefix and getting libvirt to utilize everything); but I think 3.0 
has enough building blocks to at least play with it.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 1/2] nbd/client: Add x-dirty-bitmap to query bitmap from server

[Vladimir Sementsov-Ogievskiy]

02.07.2018 22:14, Eric Blake wrote:
> In order to test that the NBD server is properly advertising
> dirty bitmaps, we need a bare minimum client that can request
> and read the context.  Since feature freeze for 3.0 is imminent,
> this is the smallest workable patch, which replaces the qemu
> block status report with the results of the NBD server's dirty
> bitmap (making it very easy to use 'qemu-img map --output=json'
> to learn where the dirty portions are).  Note that the NBD
> protocol defines a dirty section with the same bit but opposite
> sense that normal "base:allocation" uses to report an allocated
> section; so in qemu-img map output, "data":true corresponds to
> clean, "data":false corresponds to dirty.
>
> A more complete solution that allows dirty bitmaps to be queried
> at the same time as normal block status will be required before
> this addition can lose the x- prefix.  Until then, the fact that
> this replaces normal status with dirty status means actions
> like 'qemu-img convert' will likely misbehave due to treating
> dirty regions of the file as if they are unallocated.
>
> The next patch adds an iotest to exercise this new code.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
>   qapi/block-core.json |  7 ++++++-
>   block/nbd-client.h   |  1 +
>   include/block/nbd.h  |  1 +
>   block/nbd-client.c   |  3 +++
>   block/nbd.c          | 10 ++++++++--
>   nbd/client.c         |  4 ++--
>   6 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 577ce5e9991..90e554ed0ff 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3471,12 +3471,17 @@
>   #
>   # @tls-creds:   TLS credentials ID
>   #
> +# @x-dirty-bitmap: A "qemu:dirty-bitmap:NAME" string to query in place of
> +#                  traditional "base:allocation" block status (see
> +#                  NBD_OPT_LIST_META_CONTEXT in the NBD protocol) (since 3.0)
> +#

"x-dirty-bitmap=qemu:dirty-bitmap:NAME", is a bit strange, looks like it 
should be "x-dirty-bitmap=NAME", and "qemu:dirty-bitmap" added 
automatically. (and you don't check it, so actually this parameter is 
x-meta-context, and user can select any context, for example, 
"base:allocation", so "x-meta-context=qemu:dirty-bitmap:NAME" sounds 
better too). But I'm ok to leave it as is for now, with x- prefix.


>   # Since: 2.9
>   ##
>   { 'struct': 'BlockdevOptionsNbd',
>     'data': { 'server': 'SocketAddress',
>               '*export': 'str',
> -            '*tls-creds': 'str' } }
> +            '*tls-creds': 'str',
> +            '*x-dirty-bitmap': 'str' } }
>
>   ##
>   # @BlockdevOptionsRaw:
> diff --git a/block/nbd-client.h b/block/nbd-client.h
> index 0ece76e5aff..cfc90550b99 100644
> --- a/block/nbd-client.h
> +++ b/block/nbd-client.h
> @@ -45,6 +45,7 @@ int nbd_client_init(BlockDriverState *bs,
>                       const char *export_name,
>                       QCryptoTLSCreds *tlscreds,
>                       const char *hostname,
> +                    const char *x_dirty_bitmap,
>                       Error **errp);
>   void nbd_client_close(BlockDriverState *bs);
>
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index daaeae61bf9..4638c839f51 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -259,6 +259,7 @@ static inline bool nbd_reply_type_is_error(int type)
>   struct NBDExportInfo {
>       /* Set by client before nbd_receive_negotiate() */
>       bool request_sizes;
> +    char *x_dirty_bitmap;
>
>       /* In-out fields, set by client before nbd_receive_negotiate() and
>        * updated by server results during nbd_receive_negotiate() */
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 8d69eaaa32f..9686ecbd5ee 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -970,6 +970,7 @@ int nbd_client_init(BlockDriverState *bs,
>                       const char *export,
>                       QCryptoTLSCreds *tlscreds,
>                       const char *hostname,
> +                    const char *x_dirty_bitmap,
>                       Error **errp)
>   {
>       NBDClientSession *client = nbd_get_client_session(bs);
> @@ -982,9 +983,11 @@ int nbd_client_init(BlockDriverState *bs,
>       client->info.request_sizes = true;
>       client->info.structured_reply = true;
>       client->info.base_allocation = true;
> +    client->info.x_dirty_bitmap = g_strdup(x_dirty_bitmap);
>       ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), export,
>                                   tlscreds, hostname,
>                                   &client->ioc, &client->info, errp);
> +    g_free(client->info.x_dirty_bitmap);

hm, pointer remains invalid. If you want free it here, what is the 
reason to strdup it? At least, it worth to zero out the pointer after 
g_free.. Or, free it not here but in client close.

with pointer zeroed or g_free to client destroy procedure,

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>



-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v2 2/2] iotests: New test 223 for exporting dirty bitmap over NBD

[Vladimir Sementsov-Ogievskiy]

02.07.2018 22:14, Eric Blake wrote:
> Although this test is NOT a full test of image fleecing (as it
> intentionally uses just a single block device directly exported
> over NBD, rather than trying to set up a blockdev-backup job with
> multiple BDS involved), it DOES prove that qemu as a server is
> able to properly expose a dirty bitmap over NBD.
>
> When coupled with image fleecing, it is then possible for a
> third-party client to do an incremental backup by using
> qemu-img map with the x-dirty-bitmap option to learn which parts
> of the file are dirty (perhaps confusingly, they are the portions
> mapped as "data":false - which is part of the reason this is
> still in the x- experimental namespace), along with another
> normal client (perhaps 'qemu-nbd -c' to expose the server over
> /dev/nbd0 and then just use normal I/O on that block device) to
> read the dirty sections.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   tests/qemu-iotests/223     | 138 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/223.out |  49 ++++++++++++++++
>   tests/qemu-iotests/group   |   1 +
>   3 files changed, 188 insertions(+)
>   create mode 100755 tests/qemu-iotests/223
>   create mode 100644 tests/qemu-iotests/223.out
>
> diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
> new file mode 100755
> index 00000000000..b63b7a4f9e1
> --- /dev/null
> +++ b/tests/qemu-iotests/223
> @@ -0,0 +1,138 @@
> +#!/bin/bash

[...]

> +
> +echo
> +echo "=== End NBD server ==="
> +echo
> +
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove",
> +  "arguments":{"name":"n"}}' "return"
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-stop"}' "return"

blockdev-del is not necessary?

with or without:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v2 2/2] iotests: New test 223 for exporting dirty bitmap over NBD

[Vladimir Sementsov-Ogievskiy]

03.07.2018 00:27, John Snow wrote:
>
> On 07/02/2018 03:14 PM, Eric Blake wrote:
>> Although this test is NOT a full test of image fleecing (as it
>> intentionally uses just a single block device directly exported
>> over NBD, rather than trying to set up a blockdev-backup job with
>> multiple BDS involved), it DOES prove that qemu as a server is
>> able to properly expose a dirty bitmap over NBD.
>>
>> When coupled with image fleecing, it is then possible for a
>> third-party client to do an incremental backup by using
>> qemu-img map with the x-dirty-bitmap option to learn which parts
>> of the file are dirty (perhaps confusingly, they are the portions
>> mapped as "data":false - which is part of the reason this is
>> still in the x- experimental namespace), along with another
>> normal client (perhaps 'qemu-nbd -c' to expose the server over
>> /dev/nbd0 and then just use normal I/O on that block device) to
>> read the dirty sections.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>   tests/qemu-iotests/223     | 138 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/223.out |  49 ++++++++++++++++
>>   tests/qemu-iotests/group   |   1 +
>>   3 files changed, 188 insertions(+)
>>   create mode 100755 tests/qemu-iotests/223
>>   create mode 100644 tests/qemu-iotests/223.out
>>
>> diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
>> new file mode 100755
>> index 00000000000..b63b7a4f9e1
>> --- /dev/null
>> +++ b/tests/qemu-iotests/223
>> @@ -0,0 +1,138 @@
>> +#!/bin/bash
>> +#
>> +# Test reading dirty bitmap over NBD
>> +#
>> +# Copyright (C) 2018 Red Hat, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +seq="$(basename $0)"
>> +echo "QA output created by $seq"
>> +
>> +here="$PWD"
>> +status=1 # failure is the default!
>> +
>> +_cleanup()
>> +{
>> +    _cleanup_test_img
>> +    _cleanup_qemu
>> +    rm -f "$TEST_DIR/nbd"
>> +}
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +# get standard environment, filters and checks
>> +. ./common.rc
>> +. ./common.filter
>> +. ./common.qemu
>> +
>> +_supported_fmt qcow2
>> +_supported_proto file # uses NBD as well
>> +_supported_os Linux
>> +
>> +function do_run_qemu()
>> +{
>> +    echo Testing: "$@"
>> +    $QEMU -nographic -qmp stdio -serial none "$@"
>> +    echo
>> +}
>> +
>> +function run_qemu()
>> +{
>> +    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
>> +                          | _filter_qemu | _filter_imgfmt \
>> +                          | _filter_actual_image_size
>> +}
>> +
>> +echo
>> +echo "=== Create partially sparse image, then add dirty bitmap ==="
>> +echo
>> +
>> +_make_test_img 4M
>> +$QEMU_IO -c 'w -P 0x11 1M 2M' "$TEST_IMG" | _filter_qemu_io
> - Write 0x11 from [1M, 3M), not recorded by a bitmap.
>
>> +run_qemu <<EOF
>> +{ "execute": "qmp_capabilities" }
>> +{ "execute": "blockdev-add",
>> +  "arguments": {
>> +    "driver": "$IMGFMT",
>> +    "node-name": "n",
>> +    "file": {
>> +      "driver": "file",
>> +      "filename": "$TEST_IMG"
>> +    }
>> +  }
>> +}
>> +{ "execute": "block-dirty-bitmap-add",
>> +  "arguments": {
>> +    "node": "n",
>> +    "name": "b",
> Saving a few precious bytes.
>
>> +    "persistent": true
>> +  }
>> +}
>> +{ "execute": "quit" }
>> +EOF
>> +
>> +echo
>> +echo "=== Write part of the file under active bitmap ==="
>> +echo
>> +
>> +$QEMU_IO -c 'w -P 0x22 2M 2M' "$TEST_IMG" | _filter_qemu_io
>> +
> - Write 0x22 to [2M, 4M) recorded by the bitmap.
>
>> +echo
>> +echo "=== End dirty bitmap, and start serving image over NBD ==="
>> +echo
>> +
>> +_launch_qemu 2> >(_filter_nbd)
>> +
>> +silent=
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"qmp_capabilities"}' "return"
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"blockdev-add",
>> +  "arguments":{"driver":"qcow2", "node-name":"n",
>> +    "file":{"driver":"file", "filename":"'"$TEST_IMG"'"}}}' "return"
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
>> +  "arguments":{"node":"n", "name":"b"}}' "return"
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start",
>> +  "arguments":{"addr":{"type":"unix",
>> +    "data":{"path":"'"$TEST_DIR/nbd"'"}}}}' "return"
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
>> +  "arguments":{"device":"n"}}' "return"
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap",
>> +  "arguments":{"name":"n", "bitmap":"b"}}' "return"
>> +
> So far, so good.
>
>> +echo
>> +echo "=== Contrast normal status with dirty-bitmap status ==="
>> +echo
>> +
>> +QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
>> +IMG="driver=nbd,export=n,server.type=unix,server.path=$TEST_DIR/nbd"
>> +$QEMU_IO -r -c 'r -P 0 0 1m' -c 'r -P 0x11 1m 1m' \
>> +  -c 'r -P 0x22 2m 2m' --image-opts "$IMG" | _filter_qemu_io
> Confirming that we've got 0x11 from [1M, 2M) and 0x22 from [2M, 4M).
>
>> +$QEMU_IMG map --output=json --image-opts \
>> +  "$IMG" | _filter_qemu_img_map
> Normal allocation map. Ought to show [1M, 4M).
>
>> +$QEMU_IMG map --output=json --image-opts \
>> +  "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b" | _filter_qemu_img_map
> Hacked bitmap allocation map. Ought to show [2M, 4M).
>
>> +
>> +echo
>> +echo "=== End NBD server ==="
>> +echo
>> +
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove",
>> +  "arguments":{"name":"n"}}' "return"
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-stop"}' "return"
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"quit"}' "return"
>> +
>> +# success, all done
>> +echo '*** done'
>> +rm -f $seq.full
>> +status=0
>> diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
>> new file mode 100644
>> index 00000000000..33021c8e6a1
>> --- /dev/null
>> +++ b/tests/qemu-iotests/223.out
>> @@ -0,0 +1,49 @@
>> +QA output created by 223
>> +
>> +=== Create partially sparse image, then add dirty bitmap ===
>> +
>> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
>> +wrote 2097152/2097152 bytes at offset 1048576
>> +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +Testing:
>> +QMP_VERSION
>> +{"return": {}}
>> +{"return": {}}
>> +{"return": {}}
>> +{"return": {}}
>> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
>> +
>> +
>> +=== Write part of the file under active bitmap ===
>> +
>> +wrote 2097152/2097152 bytes at offset 2097152
>> +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +
>> +=== End dirty bitmap, and start serving image over NBD ===
>> +
>> +{"return": {}}
>> +{"return": {}}
>> +{"return": {}}
>> +{"return": {}}
>> +{"return": {}}
>> +{"return": {}}
>> +
>> +=== Contrast normal status with dirty-bitmap status ===
>> +
>> +read 1048576/1048576 bytes at offset 0
>> +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +read 1048576/1048576 bytes at offset 1048576
>> +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +read 2097152/2097152 bytes at offset 2097152
>> +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false},
>> +{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true}]
> Looks right.
>
>> +[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true},
>> +{ "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]
> Also looks right.
>
>> +
>> +=== End NBD server ===
>> +
>> +{"return": {}}
>> +{"return": {}}
>> +{"return": {}}
>> +*** done
>> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
>> index eea75819d2a..a446476583e 100644
>> --- a/tests/qemu-iotests/group
>> +++ b/tests/qemu-iotests/group
>> @@ -220,3 +220,4 @@
>>   218 rw auto quick
>>   219 rw auto
>>   221 rw auto quick
>> +223 rw auto quick
>>
> It Works!:
>
> Tested-by: John Snow <jsnow@redhat.com>
>
> Tests what it aims to:
>
> Reviewed-by: John Snow <jsnow@redhat.com>
>
>
> I think the trick will be combining 222 and 223 into one workflow. I
> think we might be missing a piece.
>
> Consider this:
>
> - We have some image which has a bitmap 'B' tracking writes since the
> last incremental backup was made or fleeced, using the traditional "one
> bitmap per backup regimen" strategy.
>
> - We prepare to fleece by creating a new temporary store for fleecing,
> and add the target drive as a backing node.
>
> At this point, there's no point-in-time established just yet; we're
> still "live" and so is the bitmap.
>
> - We run blockdev-backup sync=none to start achieving PIT semantics for
> the fleecing node.
>
> - We start the NBD server, and add the fleecing node export.
>
> - We attempt to link the bitmap to the NBD export, but we can't! It's
> still active, and we never froze it for the PIT.
>
>
>
> We need a way to freeze this bitmap manually like in sync=incremental,
> to associate it with this PIT... or, we need to allow the "merge"

We've chosen the second way. I didn't sent patch yet, sorry for that. 
Will do it now (too late, yes? :(.

> command to operate under the transaction so we can copy a bitmap out at
> that PIT, e.g.
>
> - Create bitmap "foo"
> - Disable bitmap "foo"

^^ should be under a transaction too

> Transaction {
>    - blockdev-backup sync=none
>    - merge "B" into "foo"
> }
> - NBD start, export, add bitmap "foo"

or, we can disable bitmap B, and create another one, to track changes 
from backup point. and we can merge them in future, if needed.


>
>
> I think there's no escaping that we need at least one of the following:
>
> - Merge (or copy) in transactions
> - Manual bitmap freeze/thaw commands (transactionable)
> - blockdev-fleece job & QMP command that does the following items:
> 	- Handles the creating of a temporary fleecing node
> 	- Establishes the blockdev-backup semantics for PIT export
> 	- Accepts a bitmap and forks it just like blockdev-backup does
>
> In the case of the block job, we don't need to tie it to NBD
> necessarily; we can leave it as the creation of the node. This would
> allow us to export it over a different transport later.
>
> The "add bitmap to NBD server" mechanism would then change to accept
> either a disabled OR frozen bitmap.
>
> Canceling/completing the job manually dictates if we clear the bitmap or
> merge in the new changes, just like blockdev-backup.
>
> The job could be implemented as a form of blockdev-backup with most of
> the same code, but some new setup/teardown code.
>
> That's a fanciful operation though, and too late for 3.0.


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v2 1/2] nbd/client: Add x-dirty-bitmap to query bitmap from server

[Eric Blake]

On 07/03/2018 04:46 AM, Vladimir Sementsov-Ogievskiy wrote:

>>   #
>> +# @x-dirty-bitmap: A "qemu:dirty-bitmap:NAME" string to query in 
>> place of
>> +#                  traditional "base:allocation" block status (see
>> +#                  NBD_OPT_LIST_META_CONTEXT in the NBD protocol) 
>> (since 3.0)
>> +#
> 
> "x-dirty-bitmap=qemu:dirty-bitmap:NAME", is a bit strange, looks like it 
> should be "x-dirty-bitmap=NAME", and "qemu:dirty-bitmap" added 
> automatically. (and you don't check it, so actually this parameter is 
> x-meta-context, and user can select any context, for example, 
> "base:allocation", so "x-meta-context=qemu:dirty-bitmap:NAME" sounds 
> better too). But I'm ok to leave it as is for now, with x- prefix.

Good point on 'x-meta-context' being slightly nicer; but bikeshedding an 
experimental name doesn't really impact the release.

For 3.1, I'd love to have:

qemu-img map --output=json --image-opts driver=nbd,...,bitmap=FOO

automatically connect to qemu:dirty-bitmap:FOO in addition to block 
status, resulting in output for iotest 223 that resembles:

[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false},
{ "start": 1048576, "length": 1048576, "depth": 0, "zero": false, 
"data": true},
{ "start": 2097152, "length": 2097152, "depth": 0, "zero": false, 
"data": true, "dirty": true}]

(that is, an optional 'dirty' member added to the JSON to identify the 
dirty portions, in addition to everything else already identified).  I 
also want to enhance that test to show that discarding clusters works 
during dirty tracking (that is, a "data":false, "dirty":true entry 
should be demonstrated).

Getting to that point will mean adding a new BDRV_BLOCK_DIRTY bit to the 
output of bdrv_block_status() (even if only the NBD and passthrough 
drivers set it), as well as teaching qemu-img map to optionally honor 
that bit when present.  It also means the NBD client code will be 
subscribing to status from two meta contexts at once, with the second 
being exactly a qemu:dirty-bitmap (rather than a free-for-all string 
that x-dirty-bitmap currently gives us).

It's too late to get these improvements into 3.0, but should be a good 
path forward in the next few months.


>> @@ -982,9 +983,11 @@ int nbd_client_init(BlockDriverState *bs,
>>       client->info.request_sizes = true;
>>       client->info.structured_reply = true;
>>       client->info.base_allocation = true;
>> +    client->info.x_dirty_bitmap = g_strdup(x_dirty_bitmap);
>>       ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), export,
>>                                   tlscreds, hostname,
>>                                   &client->ioc, &client->info, errp);
>> +    g_free(client->info.x_dirty_bitmap);
> 
> hm, pointer remains invalid. If you want free it here, what is the 
> reason to strdup it? At least, it worth to zero out the pointer after 
> g_free.. Or, free it not here but in client close.

The g_strdup() is necessary since client.x_dirty_bitmap is 'const char 
*'.  Nothing used the pointer after it is freed, although I could agree 
that assigning it to NULL to make that point obvious wouldn't hurt; 
however, the pull request was already taken as-is, so we can just live 
with it until the 3.1 improvements.

> 
> with pointer zeroed or g_free to client destroy procedure,
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 2/2] iotests: New test 223 for exporting dirty bitmap over NBD

[Eric Blake]

On 07/03/2018 05:09 AM, Vladimir Sementsov-Ogievskiy wrote:
> 02.07.2018 22:14, Eric Blake wrote:
>> Although this test is NOT a full test of image fleecing (as it
>> intentionally uses just a single block device directly exported
>> over NBD, rather than trying to set up a blockdev-backup job with
>> multiple BDS involved), it DOES prove that qemu as a server is
>> able to properly expose a dirty bitmap over NBD.
>>
>> When coupled with image fleecing, it is then possible for a
>> third-party client to do an incremental backup by using
>> qemu-img map with the x-dirty-bitmap option to learn which parts
>> of the file are dirty (perhaps confusingly, they are the portions
>> mapped as "data":false - which is part of the reason this is
>> still in the x- experimental namespace), along with another
>> normal client (perhaps 'qemu-nbd -c' to expose the server over
>> /dev/nbd0 and then just use normal I/O on that block device) to
>> read the dirty sections.
>>

>> +
>> +echo
>> +echo "=== End NBD server ==="
>> +echo
>> +
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove",
>> +  "arguments":{"name":"n"}}' "return"
>> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-stop"}' "return"
> 
> blockdev-del is not necessary?

I guess it's symmetric in that since we hotplugged the disk, we would 
also make sure hotunplug works after everything else has quit using it. 
But even the nbd-server-stop is not strictly necessary, since quitting 
qemu should have the same effect.  At this point, I'm not too worried 
about changing the test.

> 
> with or without:
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Thanks for the review; the pull request went through without your 
notation being appended, but it never hurts to have additional review 
(and we still have time even after 3.0 soft freeze to fix any important 
bugs in what went in).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org