[Qemu-devel] [PATCH v2 0/2] block: formalize and test fleecing

[Qemu-devel] [PATCH v2 0/2] block: formalize and test fleecing

[John Snow]

Formalize the fleecing workflow in patch one,
test that it works in patch two.

John Snow (2):
  block: allow blockdev-backup from any source
  iotests: add 222 to test basic fleecing

 blockdev.c                 |   2 +-
 tests/qemu-iotests/222     | 149 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/222.out |  60 ++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 4 files changed, 211 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/222
 create mode 100644 tests/qemu-iotests/222.out

-- 
2.14.4

[Qemu-devel] [PATCH v2 1/2] block: allow blockdev-backup from any source

[John Snow]

In the case of image fleecing, the node we choose as the source
for a blockdev-backup is going to be both a root node AND the
backing node for the exported image. It does not qualify as a root
image in this case.

Loosen the restriction.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 58d7570932..526f8b60be 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3517,7 +3517,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
         backup->compress = false;
     }
 
-    bs = qmp_get_root_bs(backup->device, errp);
+    bs = bdrv_lookup_bs(backup->device, backup->device, errp);
     if (!bs) {
         return NULL;
     }
-- 
2.14.4

[Qemu-devel] [PATCH v2 2/2] iotests: add 222 to test basic fleecing

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/222     | 149 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/222.out |  60 ++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 210 insertions(+)
 create mode 100644 tests/qemu-iotests/222
 create mode 100644 tests/qemu-iotests/222.out

diff --git a/tests/qemu-iotests/222 b/tests/qemu-iotests/222
new file mode 100644
index 0000000000..4a1bf1fba3
--- /dev/null
+++ b/tests/qemu-iotests/222
@@ -0,0 +1,149 @@
+#!/usr/bin/env python
+#
+# This test covers the basic fleecing workflow, which provides a
+# point-in-time snapshot of a node that can be queried over NBD.
+#
+# Copyright (C) 2018 Red Hat, Inc.
+# John helped, too.
+#
+# 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/>.
+#
+# Creator/Owner: John Snow <jsnow@redhat.com>
+
+import iotests
+from iotests import log, qemu_img, qemu_io, qemu_io_silent
+
+iotests.verify_platform(['linux'])
+
+patterns = [("0x5d", "0",         "64k"),
+            ("0xd5", "1M",        "64k"),
+            ("0xdc", "32M",       "64k"),
+            ("0xcd", "0x3ff0000", "64k")]  # 64M - 64K
+
+overwrite = [("0xab", "0",         "64k"), # Full overwrite
+             ("0xad", "0x00f8000", "64k"), # Partial-left (1M-32K)
+             ("0x1d", "0x2008000", "64k"), # Partial-right (32M+32K)
+             ("0xea", "0x3fe0000", "64k")] # Adjacent-left (64M - 128K)
+
+remainder = [("0xd5", "0x108000",  "32K"), # Right-end of partial-left [1]
+             ("0xdc", "32M",       "32k"), # Left-end of partial-right [2]
+             ("0xcd", "0x3ff0000", "64k")] # patterns[3]
+
+with iotests.FilePath('base.img') as base_img_path, \
+     iotests.FilePath('fleece.img') as fleece_img_path, \
+     iotests.FilePath('nbd.sock') as nbd_sock_path, \
+     iotests.VM() as vm:
+
+    log('--- Setting up images ---')
+    log('')
+
+    assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0
+    assert qemu_img('create', '-f', "qcow2", fleece_img_path, '64M') == 0
+
+    for p in patterns:
+        qemu_io('-c', 'write -P%s %s %s' % p, base_img_path)
+
+    log('Done')
+
+    log('')
+    log('--- Launching VM ---')
+    log('')
+
+    vm.add_drive(base_img_path)
+    vm.launch()
+    log('Done')
+
+    log('')
+    log('--- Setting up Fleecing Graph ---')
+    log('')
+
+    srcNode = "drive0"
+    tgtNode = "fleeceNode"
+
+    # create tgtNode backed by srcNode
+    log(vm.qmp("blockdev-add", **{
+        "driver": "qcow2",
+        "node-name": tgtNode,
+        "file": {
+            "driver": "file",
+            "filename": fleece_img_path,
+        },
+        "backing": srcNode,
+    }))
+
+    # Establish COW from source to fleecing node
+    log(vm.qmp("blockdev-backup",
+               device=srcNode,
+               target=tgtNode,
+               sync="none"))
+
+    log('')
+    log('--- Setting up NBD Export ---')
+    log('')
+
+    nbd_uri = 'nbd+unix:///%s?socket=%s' % (tgtNode, nbd_sock_path)
+    log(vm.qmp("nbd-server-start",
+               **{"addr": { "type": "unix",
+                            "data": { "path": nbd_sock_path } } }))
+
+    log(vm.qmp("nbd-server-add", device=tgtNode))
+
+    log('')
+    log('--- Sanity Check ---')
+    log('')
+
+    for p in patterns:
+        cmd = "read -P%s %s %s" % p
+        log(cmd)
+        assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
+
+    log('')
+    log('--- Testing COW ---')
+    log('')
+
+    for p in overwrite:
+        cmd = "write -P%s %s %s" % p
+        log(cmd)
+        log(vm.hmp_qemu_io(srcNode, cmd))
+
+    log('')
+    log('--- Verifying Data ---')
+    log('')
+
+    for p in patterns:
+        cmd = "read -P%s %s %s" % p
+        log(cmd)
+        assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
+
+    log('')
+    log('--- Cleanup ---')
+    log('')
+
+    #log(vm.hmp_qemu_io(srcNode, 'flush'))
+    log(vm.qmp('block-job-cancel', device=srcNode))
+    log(vm.qmp('nbd-server-stop'))
+    log(vm.qmp('blockdev-del', node_name=tgtNode))
+    vm.shutdown()
+
+    log('')
+    log('--- Confirming writes ---')
+    log('')
+
+    for p in (overwrite + remainder):
+        cmd = "read -P%s %s %s" % p
+        log(cmd)
+        assert qemu_io_silent(base_img_path, '-c', cmd) == 0
+
+    log('')
+    log('Done')
diff --git a/tests/qemu-iotests/222.out b/tests/qemu-iotests/222.out
new file mode 100644
index 0000000000..533fb6bc61
--- /dev/null
+++ b/tests/qemu-iotests/222.out
@@ -0,0 +1,60 @@
+--- Setting up images ---
+
+Done
+
+--- Launching VM ---
+
+Done
+
+--- Setting up Fleecing Graph ---
+
+{u'return': {}}
+{u'return': {}}
+
+--- Setting up NBD Export ---
+
+{u'return': {}}
+{u'return': {}}
+
+--- Sanity Check ---
+
+read -P0x5d 0 64k
+read -P0xd5 1M 64k
+read -P0xdc 32M 64k
+read -P0xcd 0x3ff0000 64k
+
+--- Testing COW ---
+
+write -P0xab 0 64k
+{u'return': u''}
+write -P0xad 0x00f8000 64k
+{u'return': u''}
+write -P0x1d 0x2008000 64k
+{u'return': u''}
+write -P0xea 0x3fe0000 64k
+{u'return': u''}
+
+--- Verifying Data ---
+
+read -P0x5d 0 64k
+read -P0xd5 1M 64k
+read -P0xdc 32M 64k
+read -P0xcd 0x3ff0000 64k
+
+--- Cleanup ---
+
+{u'return': {}}
+{u'return': {}}
+{u'return': {}}
+
+--- Confirming writes ---
+
+read -P0xab 0 64k
+read -P0xad 0x00f8000 64k
+read -P0x1d 0x2008000 64k
+read -P0xea 0x3fe0000 64k
+read -P0xd5 0x108000 32K
+read -P0xdc 32M 32k
+read -P0xcd 0x3ff0000 64k
+
+Done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index eea75819d2..8019a9f721 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
+222 rw auto quick
-- 
2.14.4

Re: [Qemu-devel] [PATCH v2 1/2] block: allow blockdev-backup from any source

[Eric Blake]

On 06/28/2018 01:00 PM, John Snow wrote:
> In the case of image fleecing, the node we choose as the source
> for a blockdev-backup is going to be both a root node AND the
> backing node for the exported image. It does not qualify as a root
> image in this case.
> 
> Loosen the restriction.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   blockdev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

In v1, you mentioned that this used to work but then regressed, 
pinpointing that detail in the commit message might be nice, but not 
essential (since we didn't test it until now).  So,

Reviewed-by: Eric Blake <eblake@redhat.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: add 222 to test basic fleecing

[Eric Blake]

On 06/28/2018 01:00 PM, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/qemu-iotests/222     | 149 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/222.out |  60 ++++++++++++++++++
>   tests/qemu-iotests/group   |   1 +
>   3 files changed, 210 insertions(+)
>   create mode 100644 tests/qemu-iotests/222
>   create mode 100644 tests/qemu-iotests/222.out
> 

> +
> +    log('')
> +    log('--- Cleanup ---')
> +    log('')
> +
> +    #log(vm.hmp_qemu_io(srcNode, 'flush'))
> +    log(vm.qmp('block-job-cancel', device=srcNode))

Is the commented line still worthwhile to keep?

> +    log(vm.qmp('nbd-server-stop'))
> +    log(vm.qmp('blockdev-del', node_name=tgtNode))
> +    vm.shutdown()
> +
> +    log('')
> +    log('--- Confirming writes ---')
> +    log('')
> +
> +    for p in (overwrite + remainder):
> +        cmd = "read -P%s %s %s" % p
> +        log(cmd)
> +        assert qemu_io_silent(base_img_path, '-c', cmd) == 0
> +

Thanks for the additions compared to v1 (including the previously 
missing .out file ;)
Reviewed-by: Eric Blake <eblake@redhat.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: add 222 to test basic fleecing

[Eric Blake]

On 06/28/2018 01:00 PM, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/qemu-iotests/222     | 149 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/222.out |  60 ++++++++++++++++++
>   tests/qemu-iotests/group   |   1 +
>   3 files changed, 210 insertions(+)
>   create mode 100644 tests/qemu-iotests/222
>   create mode 100644 tests/qemu-iotests/222.out
> 

> +iotests.verify_platform(['linux'])
> +
> +patterns = [("0x5d", "0",         "64k"),
> +            ("0xd5", "1M",        "64k"),
> +            ("0xdc", "32M",       "64k"),
> +            ("0xcd", "0x3ff0000", "64k")]  # 64M - 64K
> +
> +overwrite = [("0xab", "0",         "64k"), # Full overwrite
> +             ("0xad", "0x00f8000", "64k"), # Partial-left (1M-32K)
> +             ("0x1d", "0x2008000", "64k"), # Partial-right (32M+32K)
> +             ("0xea", "0x3fe0000", "64k")] # Adjacent-left (64M - 128K)
> +
> +remainder = [("0xd5", "0x108000",  "32K"), # Right-end of partial-left [1]
> +             ("0xdc", "32M",       "32k"), # Left-end of partial-right [2]
> +             ("0xcd", "0x3ff0000", "64k")] # patterns[3]

What if you also add:

zeroes =  [("0x00", "0x108000",  "32K"), # Right-end of partial-left [1]
            ("0x00", "32M",       "32k"), # Left-end of partial-right [2]
            ("0x00", "0x3ff0000", "64k")] # patterns[3]

then...

> +    log('')
> +    log('--- Sanity Check ---')
> +    log('')
> +
> +    for p in patterns:

for p in (patterns + zeroes)

> +        cmd = "read -P%s %s %s" % p
> +        log(cmd)
> +        assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
> +
> +    log('')
> +    log('--- Testing COW ---')
> +    log('')
> +
> +    for p in overwrite:
> +        cmd = "write -P%s %s %s" % p
> +        log(cmd)
> +        log(vm.hmp_qemu_io(srcNode, cmd))
> +
> +    log('')
> +    log('--- Verifying Data ---')
> +    log('')
> +
> +    for p in patterns:

for p in (patterns + zeroes)

> +        cmd = "read -P%s %s %s" % p
> +        log(cmd)
> +        assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
> +
> +    log('')
> +    log('--- Cleanup ---')
> +    log('')
> +
> +    #log(vm.hmp_qemu_io(srcNode, 'flush'))
> +    log(vm.qmp('block-job-cancel', device=srcNode))
> +    log(vm.qmp('nbd-server-stop'))
> +    log(vm.qmp('blockdev-del', node_name=tgtNode))
> +    vm.shutdown()
> +
> +    log('')
> +    log('--- Confirming writes ---')
> +    log('')
> +
> +    for p in (overwrite + remainder):

so that reads of both the fleeced point in time and of the final image 
cover the same set of byte ranges.

-- 
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: add 222 to test basic fleecing

[John Snow]

On 06/28/2018 02:17 PM, Eric Blake wrote:
> On 06/28/2018 01:00 PM, John Snow wrote:
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/qemu-iotests/222     | 149
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/222.out |  60 ++++++++++++++++++
>>   tests/qemu-iotests/group   |   1 +
>>   3 files changed, 210 insertions(+)
>>   create mode 100644 tests/qemu-iotests/222
>>   create mode 100644 tests/qemu-iotests/222.out
>>
> 
>> +iotests.verify_platform(['linux'])
>> +
>> +patterns = [("0x5d", "0",         "64k"),
>> +            ("0xd5", "1M",        "64k"),
>> +            ("0xdc", "32M",       "64k"),
>> +            ("0xcd", "0x3ff0000", "64k")]  # 64M - 64K
>> +
>> +overwrite = [("0xab", "0",         "64k"), # Full overwrite
>> +             ("0xad", "0x00f8000", "64k"), # Partial-left (1M-32K)
>> +             ("0x1d", "0x2008000", "64k"), # Partial-right (32M+32K)
>> +             ("0xea", "0x3fe0000", "64k")] # Adjacent-left (64M - 128K)
>> +
>> +remainder = [("0xd5", "0x108000",  "32K"), # Right-end of
>> partial-left [1]
>> +             ("0xdc", "32M",       "32k"), # Left-end of
>> partial-right [2]
>> +             ("0xcd", "0x3ff0000", "64k")] # patterns[3]
> 
> What if you also add:
> 
> zeroes =  [("0x00", "0x108000",  "32K"), # Right-end of partial-left [1]
>            ("0x00", "32M",       "32k"), # Left-end of partial-right [2]
>            ("0x00", "0x3ff0000", "64k")] # patterns[3]
> 
> then...
> 
>> +    log('')
>> +    log('--- Sanity Check ---')
>> +    log('')
>> +
>> +    for p in patterns:
> 
> for p in (patterns + zeroes)
> 
>> +        cmd = "read -P%s %s %s" % p
>> +        log(cmd)
>> +        assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri)
>> == 0
>> +
>> +    log('')
>> +    log('--- Testing COW ---')
>> +    log('')
>> +
>> +    for p in overwrite:
>> +        cmd = "write -P%s %s %s" % p
>> +        log(cmd)
>> +        log(vm.hmp_qemu_io(srcNode, cmd))
>> +
>> +    log('')
>> +    log('--- Verifying Data ---')
>> +    log('')
>> +
>> +    for p in patterns:
> 
> for p in (patterns + zeroes)
> 
>> +        cmd = "read -P%s %s %s" % p
>> +        log(cmd)
>> +        assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri)
>> == 0
>> +
>> +    log('')
>> +    log('--- Cleanup ---')
>> +    log('')
>> +
>> +    #log(vm.hmp_qemu_io(srcNode, 'flush'))
>> +    log(vm.qmp('block-job-cancel', device=srcNode))
>> +    log(vm.qmp('nbd-server-stop'))
>> +    log(vm.qmp('blockdev-del', node_name=tgtNode))
>> +    vm.shutdown()
>> +
>> +    log('')
>> +    log('--- Confirming writes ---')
>> +    log('')
>> +
>> +    for p in (overwrite + remainder):
> 
> so that reads of both the fleeced point in time and of the final image
> cover the same set of byte ranges.
> 

Sure, why not.

--js

Re: [Qemu-devel] [PATCH v2 2/2] iotests: add 222 to test basic fleecing

[John Snow]

On 06/28/2018 02:10 PM, Eric Blake wrote:
> On 06/28/2018 01:00 PM, John Snow wrote:
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/qemu-iotests/222     | 149
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/222.out |  60 ++++++++++++++++++
>>   tests/qemu-iotests/group   |   1 +
>>   3 files changed, 210 insertions(+)
>>   create mode 100644 tests/qemu-iotests/222
>>   create mode 100644 tests/qemu-iotests/222.out
>>
> 
>> +
>> +    log('')
>> +    log('--- Cleanup ---')
>> +    log('')
>> +
>> +    #log(vm.hmp_qemu_io(srcNode, 'flush'))
>> +    log(vm.qmp('block-job-cancel', device=srcNode))
> 
> Is the commented line still worthwhile to keep?

NUTS.

> 
>> +    log(vm.qmp('nbd-server-stop'))
>> +    log(vm.qmp('blockdev-del', node_name=tgtNode))
>> +    vm.shutdown()
>> +
>> +    log('')
>> +    log('--- Confirming writes ---')
>> +    log('')
>> +
>> +    for p in (overwrite + remainder):
>> +        cmd = "read -P%s %s %s" % p
>> +        log(cmd)
>> +        assert qemu_io_silent(base_img_path, '-c', cmd) == 0
>> +
> 
> Thanks for the additions compared to v1 (including the previously
> missing .out file ;)
> Reviewed-by: Eric Blake <eblake@redhat.com>
>

Re: [Qemu-devel] [PATCH v2 1/2] block: allow blockdev-backup from any source

[John Snow]

On 06/28/2018 02:05 PM, Eric Blake wrote:
> On 06/28/2018 01:00 PM, John Snow wrote:
>> In the case of image fleecing, the node we choose as the source
>> for a blockdev-backup is going to be both a root node AND the
>> backing node for the exported image. It does not qualify as a root
>> image in this case.
>>
>> Loosen the restriction.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   blockdev.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> In v1, you mentioned that this used to work but then regressed,
> pinpointing that detail in the commit message might be nice, but not
> essential (since we didn't test it until now).  So,
> 

Really not sure when it regressed; I consider it unimportant as nothing
uses it presently: no docs, no tests, nothing in libvirt.

My guess, though, is that it worked prior to
cef34eebf3d0f252a3b3e9a2a459b6c3ecc56f68.

In 2.8, possibly?

Kashyap might know, I think he's experimented with this sometime in that
timezone.

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

Re: [Qemu-devel] [PATCH v2 1/2] block: allow blockdev-backup from any source

[Kashyap Chamarthy]

On Thu, Jun 28, 2018 at 05:19:37PM -0400, John Snow wrote:
> 
> 
> On 06/28/2018 02:05 PM, Eric Blake wrote:
> > On 06/28/2018 01:00 PM, John Snow wrote:
> >> In the case of image fleecing, the node we choose as the source
> >> for a blockdev-backup is going to be both a root node AND the
> >> backing node for the exported image. It does not qualify as a root
> >> image in this case.
> >>
> >> Loosen the restriction.
> >>
> >> Signed-off-by: John Snow <jsnow@redhat.com>
> >> ---
> >>   blockdev.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > In v1, you mentioned that this used to work but then regressed,
> > pinpointing that detail in the commit message might be nice, but not
> > essential (since we didn't test it until now).  So,
> > 
> 
> Really not sure when it regressed; I consider it unimportant as nothing
> uses it presently: no docs, no tests, nothing in libvirt.
> 
> My guess, though, is that it worked prior to
> cef34eebf3d0f252a3b3e9a2a459b6c3ecc56f68.

So the above commit adds the ability to assign node names for
`blockdev-backup`, which wasn't possible during 2.8 timeframe.

> In 2.8, possibly?
> 
> Kashyap might know, I think he's experimented with this sometime in that
> timezone.

I don't recall doing the "allow blockdev-backup from any source" test
with 2.8.  (But I recall gradually moving from figuring out the source
block device via wading through the output of `query-named-block-nodes`
to using the 'node-name'.)

Looking at my test notes with QEMU 2.8, this was my
workflow (see how `blockdev-backup` still doesn't use 'node-name's):

(QEMU) blockdev-add driver=qcow2 node-name=node2 file={"driver":"file","filename":"/export/target.qcow2"} backing={"driver":"qcow2","file":{"driver":"file","filename":"/export/base.qcow2"}}
(QEMU) query-named-block-nodes # To dig through the source block device "#blockYYY"
(QEMU) blockdev-backup device=#block197 target=node1 sync=none
(QEMU) nbd-server-start addr={"type":"unix","data":{"path":"./nbd-sock"}}
(QEMU) nbd-server-add device=node1
(QEMU) nbd-server-stop

Later on I moved (thanks to the Git commit you mentioned earlier) to the
much cleaner approach of using 'node-name' (the below syntax is
from my test notes with QEMU 2.12):

(QEMU) blockdev-add driver=qcow2 node-name=node-E file={"driver":"file","filename":"e.qcow2"}
(QEMU) blockdev-backup device=node-B target=node-E sync=full job-id=job0
[...]

-- 
/kashyap

Re: [Qemu-devel] [PATCH v2 1/2] block: allow blockdev-backup from any source

[John Snow]

On 07/02/2018 03:59 AM, Kashyap Chamarthy wrote:
> On Thu, Jun 28, 2018 at 05:19:37PM -0400, John Snow wrote:
>>
>>
>> On 06/28/2018 02:05 PM, Eric Blake wrote:
>>> On 06/28/2018 01:00 PM, John Snow wrote:
>>>> In the case of image fleecing, the node we choose as the source
>>>> for a blockdev-backup is going to be both a root node AND the
>>>> backing node for the exported image. It does not qualify as a root
>>>> image in this case.
>>>>
>>>> Loosen the restriction.
>>>>
>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>> ---
>>>>   blockdev.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> In v1, you mentioned that this used to work but then regressed,
>>> pinpointing that detail in the commit message might be nice, but not
>>> essential (since we didn't test it until now).  So,
>>>
>>
>> Really not sure when it regressed; I consider it unimportant as nothing
>> uses it presently: no docs, no tests, nothing in libvirt.
>>
>> My guess, though, is that it worked prior to
>> cef34eebf3d0f252a3b3e9a2a459b6c3ecc56f68.
> 
> So the above commit adds the ability to assign node names for
> `blockdev-backup`, which wasn't possible during 2.8 timeframe.
> 
>> In 2.8, possibly?
>>
>> Kashyap might know, I think he's experimented with this sometime in that
>> timezone.
> 
> I don't recall doing the "allow blockdev-backup from any source" test
> with 2.8.  (But I recall gradually moving from figuring out the source
> block device via wading through the output of `query-named-block-nodes`
> to using the 'node-name'.)
> 
> Looking at my test notes with QEMU 2.8, this was my
> workflow (see how `blockdev-backup` still doesn't use 'node-name's):
> 
> (QEMU) blockdev-add driver=qcow2 node-name=node2 file={"driver":"file","filename":"/export/target.qcow2"} backing={"driver":"qcow2","file":{"driver":"file","filename":"/export/base.qcow2"}}
> (QEMU) query-named-block-nodes # To dig through the source block device "#blockYYY"
> (QEMU) blockdev-backup device=#block197 target=node1 sync=none
> (QEMU) nbd-server-start addr={"type":"unix","data":{"path":"./nbd-sock"}}
> (QEMU) nbd-server-add device=node1
> (QEMU) nbd-server-stop
> 
> Later on I moved (thanks to the Git commit you mentioned earlier) to the
> much cleaner approach of using 'node-name' (the below syntax is
> from my test notes with QEMU 2.12):
> 
> (QEMU) blockdev-add driver=qcow2 node-name=node-E file={"driver":"file","filename":"e.qcow2"}
> (QEMU) blockdev-backup device=node-B target=node-E sync=full job-id=job0
> [...]
> 

Without this patch, "allow blockdev-backup from any source", the
fleecing workflow prohibits the blockdev-backup command. We're wondering
when that regressed.

If you managed to get fleecing working without this patch here, then it
was working at that time.

--js