A couple of notes on this:

Backing files are opened as read-only nodes by default, so you cannot insert copy-on-read nodes into an implicitly opened chain anyway.  If you add every node with blockdev-add, however, it becomes possible.  Also, you can add COR on top of everything, which makes it part of hte backing chain.

The next question would be which functions those are.  The main one that comes to my mind is bdrv_find_backing_image() for block-commit and block-stream.  block-commit disallows committing through explicitly added filter nodes, and maybe block-stream should, too, but it doesn’t.  So the main case here is a chain which has a COR filter on top and you want to commit something down the chain.

Non-active commit is something that can be made to work.  Active commit, however, is a bit trickier.  Something to note here: Our current distinction between active and non-active commit is broken, I believe.  It should not be based on whether the top node is a root node, but whether the top node is R/W, regardless of whether it is a root node.

Anyway, when you have a COR node as your root node and want to commit its filtered node somewhere down the backing chain, that will not work.  You cannot commit the COR node itself (for one thing, because we disallow committing through explicitly added filter nodes), and you cannot specify the filtered node, actually.  bdrv_find_backing_image() looks for *backing images* and this is based on bs->backing_file (which is completely wrong, but that’s a different matter[1]).  Now the COR node obviously does not have a bs->backing_file, so you cannot specify the node it filters.

But let’s assume you could specify it.  Oops, then block-commit would believe it’s a non-active commit because the node you specified is not a root node.  So that completely breaks then (maybe even silently, just corrupting your data).

So this is one case where I am actually inclined to believe it is best that COR breaks backing chains for now, because filters and backing chains just may not go too well together for now and there may be many things to fix.

(For fun I’ve attached a shell script that creates three qcow2 images in /tmp and generates some QMP output that breaks commit because it should be an active commit, but it is a non-active commit.  Use it like so:
$ sh broken-commit.sh | x86_64-softmmu/qemu-system-x86_64 -qmp stdio
[...]
=== Read works (before commit has completed) ===

read 16777216/16777216 bytes at offset 0
[...]
=== Read fails (after commit has completed) ===

Pattern verification failed at offset 0, 16777216 bytes
[...])


So what I am trying to say is that this is probably not as simple as replacing some bs->backing instances by a function bdrv_filtered_cow_bs().  Allowing filters in backing chains may lead to all kinds of funny stuff.


[1] It’s wrong because it assumes that bs->backing_file contains the image header’s backing file string.  Which it does not.  But it probably should.

Created attachment 1459802 [details]
Shell script generating QMP output that breaks block-commit

This script invokes block-commit, which is performed as a non-active commit.  However, the node being committed may be written to.  Data written during committing may or may not be written to the base, and thus may or may not disappear when the block job completes.

More to note, before I forget: In fact, we’d even need an active commit when any images in the backing chain of top may be written to (so top may change its data even without being written to).

However, this eventually boils down to how broken writable backing files are, because even active commit would choke on such a case.  Dirty bitmaps only track writes to the BDS, but not writes to children (we’ve had this discussion before).  When creating a dirty bitmap, we probably need to ensure that the current node blocks the WRITE permission on all of its children so it can only change by immediate writes to it.
(And we need to ensure the same when doing a non-active commit.)

Now you could say that all of this is a different kettle of fish, but it really isn’t.  The issue it all comes down to is that our filtering system is broken and we don’t quite know, how.

There are nodes that filter other nodes R/W (actual filters).  There are nodes that filter other nodes with COW (backing chains).  There are nodes that filter other nodes both R/W and COW (active mirror).  Now we mix it all and suddenly everything breaks down, which honestly probably should not be surprising.

So in fact I believe that it may be for the best that the COR driver just does not work with backing chains for now.  Because I don’t think we quite know what breaks when we do allow COR in backing chains.

(And as you can see from the commit example, things may very well break silently and just corrupt your data.)

Max

(In reply to Max Reitz from comment #1)
> [1] It’s wrong because it assumes that bs->backing_file contains the image
> header’s backing file string.  Which it does not.  But it probably should.

Turns out this is extremely important because when you actually write a test to test this, you’ll see that you actually cannot select a backing image behind a filter node simply because the filter node distorts the filename.

Max

So what we need first is BDS.backing_file to reflect the image header and nothing else, because if it deals with anything else it becomes a mess.
But that is not enough, because you can override backing files and then bdrv_find_backing_image() needs to ignore BDS.backing_file and needs to compare the given filename to the actual backing filename.
(But we do want it to use the image header value if the user has not overridden it, not least because otherwise we basically force the user to check out what qemu has come up due to bdrv_refresh_filename(), which is not quite optimal.)

So that basically means I need a way of telling whether a backing file has been overridden to use the correct reference in bdrv_find_backing_image(), so yeah, I need the filename series.

Max

Also, it should be noted that throttle of course breaks backing chains exactly like COR does, probably even worse, because you can use throttle on read-only nodes and therefore actually inside of backing chains.

Max

After tinkering more, it turns out that bdrv_find_backing_image() just doesn’t work with filters, basically whatever you do.

It doesn’t work with a filter on the top because the image below is not a backing image, so you cannot specify it.

It doesn’t work with filters below because they make using filenames effectively useless (you can query them with some command and specify them, but that’s just not OK).

So I’m back to my original question.  What is broken that isn’t broken by something else just by virtue of using filters?  query-block’s auto-descend through the backing chain maybe?

In my honest opinion, after having worked about two weeks on this (with absolutely no end in sight), the real fix is to add node name support to block-commit and to fix the query infrastructure.

Max

(In reply to Max Reitz from comment #7)
> In my honest opinion, after having worked about two weeks on this (with
> absolutely no end in sight), the real fix is to add node name support to
> block-commit and to fix the query infrastructure.

Node name support in block-commit doesn't contribute anything here because the commit job still needs a backing subchain, not just two random nodes.

$ ./qemu-img create -f qcow2 /tmp/backing.raw 64M
Formatting '/tmp/backing.raw', fmt=qcow2 size=67108864 cluster_size=65536 lazy_refcounts=off refcount_bits=16

$ ./qemu-img create -f qcow2 -b /tmp/backing.qcow2 /tmp/overlay.qcow2
Formatting '/tmp/overlay.qcow2', fmt=qcow2 size=67108864 backing_file=/tmp/backing.qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16

$ x86_64-softmmu/qemu-system-x86_64 -blockdev file,filename=/tmp/backing.raw,node-name=backing -blockdev copy-on-read,file=backing,node-name=cor -blockdev file,filename=/tmp/overlay.qcow2,node-name=overlay-file -blockdev qcow2,file=overlay-file,backing=cor,node-name=overlay -qmp stdio

{"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3}, "package": "v3.0.0-628-ga06176f961"}, "capabilities": []}}
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"block-commit","arguments":{"device":"overlay","top-node":"overlay","base-node":"backing","job-id":"job0"}}
{"error": {"class": "GenericError", "desc": "'backing' is not in this backing file chain"}}

(In reply to Kevin Wolf from comment #8)
> (In reply to Max Reitz from comment #7)
> > In my honest opinion, after having worked about two weeks on this (with
> > absolutely no end in sight), the real fix is to add node name support to
> > block-commit and to fix the query infrastructure.
> 
> Node name support in block-commit doesn't contribute anything here because
> the commit job still needs a backing subchain, not just two random nodes.

Of course it contributes something.  As I asked:

(In reply to Max Reitz from comment #7)
> So I’m back to my original question.  What is broken that isn’t broken by
> something else just by virtue of using filters?

Now we have node-name support for block-commit, that’s good.  (And your series wasn‘t even on the list when I wrote that comment.)  But before, you just couldn’t issue a block-commit even with my now-upstream series on top, because filenames weren’t enough.

So, yes, now there is a good example.  But I didn’t have that a month ago.


Anyway, the result with my series is this:

$ x86_64-softmmu/qemu-system-x86_64 -blockdev file,filename=/tmp/backing.qcow2,node-name=backing -blockdev copy-on-read,file=backing,node-name=cor -blockdev file,filename=/tmp/overlay.qcow2,node-name=overlay-file -blockdev qcow2,file=overlay-file,backing=cor,node-name=overlay -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3}, "package": "v3.0.0-668-ge4d18b7759"}, "capabilities": []}}
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"block-commit","arguments":{"device":"overlay","top-node":"overlay","base-node":"backing","job-id":"job0"}}
{"error": {"class": "GenericError", "desc": "Cannot commit through explicit filter nodes"}}

Because I didn't want to be rash.  What do you think should happen with the COR node in the case of commit?  Or rather, what should happen with a generic filter node?

commit and mirror just read from the overlay, so the filter gets applied, which seems correct, I guess?  And since filters cannot change data, it isn't like writing the data to the base would then change its appearance in the overlay.

But then commit would need to remove the filter.  Is that what we want?  Imagine you have a throttle filter in there, is removing what the user wants?

Or with COR, you could have this configuration:

http -> file A -> COR -> file B

And then you commit file B into file A, so you get:

http -> file A

That's weird.


Now the question is what we want to do.  Either we allow this and just remove the filter drivers, explicitly documenting it.  Users have to make sure to move filter drivers before they finalize the job.  (Or along with that finalization, in a transaction.)

Or we don't allow it.  Then users would need to move the filters before invoking the job.  May be functionally the same, but I suppose it's a bit stupid.


So I suppose we do want to remove the filter nodes, we just need to document it.  (My measure is: It doesn't seem to hard to explain ("Everything in (base, top] gets committed into base, so naturally everything disappears from there"), so it sounds like a good idea.)

Max

Yes, I think removing is the right thing to do. We require the management tool to know the graph structure when it wants to use any operation on a node level. If it wanted to keep the filter, it shouldn't commit into "file A", but into "COR".

Of course, if we want to translate the existing copy-on-read=on option into a filter at some point (or throttling, or anything else), we'd have to care about compatibility with management tools that don't know about nodes. Then a filename would refer to the base file, but we would actually have to select the topmost filter above that base node as the target for the commit job.

Maybe deprecating the old options would be the easier solution. ;-)

I’m setting the ITR to 8.3.0 because the current upstream series will not go into qemu 4.2.0, and it would be very difficult to backport it.

Max

This bug breaks block-commit in the following libvirt configuration:

    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' copy_on_read='on'/>
      <source file='/tmp/copy4.qcow2'/>
      <target dev='vda' bus='virtio'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
    </disk>

When a 'copy-on-read' driver is on top of a backing chain block-commit is not possible as QEMU reports:

{"class": "GenericError", "desc": "'libvirt-4-format' is not in this backing file chain"}}

libvirt would start qemu with the following (snipped) commandline:

/home/pipo/git/qemu.git/x86_64-softmmu/qemu-system-x86_64 \

[...]

-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/systemrescuecd-x86-4.9.5.iso","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"raw","file":"libvirt-2-storage"}' \

-device ide-cd,bus=ide.0,unit=0,drive=libvirt-2-format,id=ide0-0-0,bootindex=1 \

-blockdev '{"driver":"file","filename":"/tmp/copy0.qcow2","node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-6-format","read-only":true,"driver":"qcow2","file":"libvirt-6-storage","backing":null}' \
-blockdev '{"driver":"file","filename":"/tmp/copy1.qcow2","node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-5-format","read-only":true,"driver":"qcow2","file":"libvirt-5-storage","backing":"libvirt-6-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/copy2.qcow2","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"qcow2","file":"libvirt-4-storage","backing":"libvirt-5-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/copy3.qcow2","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-3-format","read-only":true,"driver":"qcow2","file":"libvirt-3-storage","backing":"libvirt-4-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/copy4.qcow2","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2","file":"libvirt-1-storage","backing":"libvirt-3-format"}' \
-blockdev '{"driver":"copy-on-read","node-name":"libvirt-CoR-vda","file":"libvirt-1-format"}' \

-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0xa,drive=libvirt-CoR-vda,id=virtio-disk0 \

[...]

$ virsh qemu-monitor-command --pretty upstream-bj '{"execute":"x-debug-query-block-graph"}'
{
  "return": {
    "edges": [
      {
        "name": "file",
        "parent": 4,
        "shared-perm": [
          "graph-mod",
          "resize",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "write-unchanged",
          "write",
          "consistent-read"
        ],
        "child": 15
      },
      {
        "name": "file",
        "parent": 15,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "resize",
          "write-unchanged",
          "write",
          "consistent-read"
        ],
        "child": 14
      },
      {
        "name": "backing",
        "parent": 15,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 13
      },
      {
        "name": "file",
        "parent": 13,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 12
      },
      {
        "name": "backing",
        "parent": 13,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 11
      },
      {
        "name": "file",
        "parent": 11,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 10
      },
      {
        "name": "backing",
        "parent": 11,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 9
      },
      {
        "name": "file",
        "parent": 9,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 8
      },
      {
        "name": "backing",
        "parent": 9,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 7
      },
      {
        "name": "file",
        "parent": 7,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 6
      },
      {
        "name": "file",
        "parent": 2,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 5
      },
      {
        "name": "root",
        "parent": 3,
        "shared-perm": [
          "graph-mod",
          "resize",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "write",
          "consistent-read"
        ],
        "child": 4
      },
      {
        "name": "root",
        "parent": 1,
        "shared-perm": [
          "graph-mod",
          "write-unchanged",
          "consistent-read"
        ],
        "perm": [
          "consistent-read"
        ],
        "child": 2
      }
    ],
    "nodes": [
      {
        "name": "libvirt-CoR-vda",
        "type": "block-driver",
        "id": 4
      },
      {
        "name": "libvirt-1-format",
        "type": "block-driver",
        "id": 15
      },
      {
        "name": "libvirt-1-storage",
        "type": "block-driver",
        "id": 14
      },
      {
        "name": "libvirt-3-format",
        "type": "block-driver",
        "id": 13
      },
      {
        "name": "libvirt-3-storage",
        "type": "block-driver",
        "id": 12
      },
      {
        "name": "libvirt-4-format",
        "type": "block-driver",
        "id": 11
      },
      {
        "name": "libvirt-4-storage",
        "type": "block-driver",
        "id": 10
      },
      {
        "name": "libvirt-5-format",
        "type": "block-driver",
        "id": 9
      },
      {
        "name": "libvirt-5-storage",
        "type": "block-driver",
        "id": 8
      },
      {
        "name": "libvirt-6-format",
        "type": "block-driver",
        "id": 7
      },
      {
        "name": "libvirt-6-storage",
        "type": "block-driver",
        "id": 6
      },
      {
        "name": "libvirt-2-format",
        "type": "block-driver",
        "id": 2
      },
      {
        "name": "libvirt-2-storage",
        "type": "block-driver",
        "id": 5
      },
      {
        "name": "/machine/peripheral/virtio-disk0/virtio-backend",
        "type": "block-backend",
        "id": 3
      },
      {
        "name": "ide0-0-0",
        "type": "block-backend",
        "id": 1
      }
    ]
  },
  "id": "libvirt-367"
}

Upstream libvirt currently does:
2020-01-23 15:09:45.621+0000: 2898636: info : qemuMonitorIOWrite:450 : QEMU_MONITOR_IO_WRITE: mon=0x7fffe00492f0 buf={"execute":"block-commit","arguments":{"device":"libvirt-1-format","job-id":"commit-vda-libvirt-4-format","top-node":"libvirt-4-format","base-node":"libvirt-5-format","backing-file":"/tmp/copy1.qcow2","auto-finalize":true,"auto-dismiss":false},"id":"libvirt-368"}
 len=265 ret=265 errno=0
2020-01-23 15:09:45.623+0000: 2898636: debug : qemuMonitorJSONIOProcessLine:220 : Line [{"id": "libvirt-368", "error": {"class": "GenericError", "desc": "Need a root block node"}}]
2020-01-23 15:09:45.623+0000: 2898636: info : qemuMonitorJSONIOProcessLine:239 : QEMU_MONITOR_RECV_REPLY: mon=0x7fffe00492f0 reply={"id": "libvirt-368", "error": {"class": "GenericError", "desc": "Need a root block node"}}

The above is a libvirt bug which I'm trying to fix. With the fix the following command is attempted:

2020-01-23 15:11:44.464+0000: 2912632: info : qemuMonitorIOWrite:450 : QEMU_MONITOR_IO_WRITE: mon=0x7fff941687c0 buf={"execute":"block-commit","arguments":{"device":"libvirt-CoR-vda","job-id":"commit-vda-libvirt-4-format","top-node":"libvirt-4-format","base-node":"libvirt-5-format","backing-file":"/tmp/copy1.qcow2","auto-finalize":true,"auto-dismiss":false},"id":"libvirt-12"}
 len=263 ret=263 errno=0
2020-01-23 15:11:44.464+0000: 2912632: debug : qemuMonitorJSONIOProcessLine:220 : Line [{"id": "libvirt-12", "error": {"class": "GenericError", "desc": "'libvirt-4-format' is not in this backing file chain"}}]
2020-01-23 15:11:44.464+0000: 2912632: info : qemuMonitorJSONIOProcessLine:239 : QEMU_MONITOR_RECV_REPLY: mon=0x7fff941687c0 reply={"id": "libvirt-12", "error": {"class": "GenericError", "desc": "'libvirt-4-format' is not in this backing file chain"}}


As a side note. block-stream does not report the "Need a root block node" error and only works correctly if the 'device' parameter doesn't point to the copy-on-read driver. Is this expected?

(In reply to Peter Krempa from comment #16)
> As a side note. block-stream does not report the "Need a root block node"
> error and only works correctly if the 'device' parameter doesn't point to
> the copy-on-read driver. Is this expected?

block-stream’s documentation doesn’t say that @device needs to be a root node.  This was changed in qemu commit 554b6147650.  So I’d say it’s entirely expected that there’s no error.

As for “it only works correctly if @device isn’t a COR filter”, that’s a bug.  Namely this one here.  (I’d like to put a smiley here, but there’s really not too much to smile about when it comes to this BZ.)

Max

Thank you for the explanation! As of the block-stream operation and the clarification on what the fields mean, the current behaviour of libvirt makes the most sense. I've already sent patches which fix the arguments for block-commit and blockdev-mirror and set the bug tracking the issue to depend on this.

QEMU has been recently split into sub-components and as a one-time operation to avoid breakage of tools, we are setting the QEMU sub-component of this BZ to "General". Please review and change the sub-component if necessary the next time you review this BZ. Thanks

The following is a reproducer (creates a simple backing chain with a copy-on-read filter on top, then tries an active commit):

$ qemu-img create -f qcow2 base.qcow2 64M
Formatting 'base.qcow2', fmt=qcow2 cluster_size=65536 compression_type=zlib size=67108864 lazy_refcounts=off refcount_bits=16

$ qemu-img create -f qcow2 -b base.qcow2 -F qcow2 top.qcow2 64M
Formatting 'top.qcow2', fmt=qcow2 cluster_size=65536 compression_type=zlib size=67108864 backing_file=base.qcow2 backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16

$ qemu-system-x86_64 \
    -nodefaults \
    -qmp stdio \
    -blockdev qcow2,node-name=base,file.driver=file,file.filename=base.qcow2 \
    -blockdev qcow2,node-name=top,file.driver=file,file.filename=top.qcow2,backing=base \
    -blockdev copy-on-read,node-name=cor,file=top \
    -device virtio-blk,drive=cor

QMP traffic on 5.2.0 (desired behavior):
(We commit everything down to the "base" node, and then we can delete the "cor" and "top" nodes because the virtio-blk device will be attached to "base" after the commit.)

{"QMP": {"version": {"qemu": {"micro": 90, "minor": 1, "major": 5}, "package": "v5.2.0-rc0-130-g43afbbd9fe"}, "capabilities": ["oob"]}}

{'execute':'qmp_capabilities'}
{"return": {}}

{'execute':'block-commit','arguments':{'job-id':'commit','device':'cor','base-node':'base'}}
{"timestamp": {"seconds": 1605005341, "microseconds": 114782}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "commit"}}
{"timestamp": {"seconds": 1605005341, "microseconds": 114888}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "commit"}}
{"return": {}}

{"timestamp": {"seconds": 1605005341, "microseconds": 115561}, "event": "JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "commit"}}
{"timestamp": {"seconds": 1605005341, "microseconds": 115589}, "event": "BLOCK_JOB_READY", "data": {"device": "commit", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}}
{"return": {}}

{'execute':'block-job-complete','arguments':{'device':'commit'}}
{"timestamp": {"seconds": 1605005341, "microseconds": 653379}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "commit"}}
{"timestamp": {"seconds": 1605005341, "microseconds": 653481}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "commit"}}
{"timestamp": {"seconds": 1605005341, "microseconds": 653731}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "commit", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}}
{"timestamp": {"seconds": 1605005341, "microseconds": 653815}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "commit"}}
{"timestamp": {"seconds": 1605005341, "microseconds": 653911}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "commit"}}

{'execute':'blockdev-del','arguments':{'node-name':'cor'}}
{"return": {}}
{'execute':'blockdev-del','arguments':{'node-name':'top'}}
{"return": {}}


QMP traffic on 5.1.0 (broken):
(Can't even start the commit, because the backing chain is broken by the COR filter, as described by comment 0.)

{"QMP": {"version": {"qemu": {"micro": 0, "minor": 1, "major": 5}, "package": "qemu-5.1.0-5.fc33"}, "capabilities": ["oob"]}}

{'execute':'qmp_capabilities'}
{"return": {}}

{'execute':'block-commit','arguments':{'job-id':'commit','device':'cor','base-node':'base'}}
{"error": {"class": "GenericError", "desc": "'base' is not in this backing file chain"}}

Hi Max,

Thanks for your guidance! Tested with qemu-kvm-5.2.0-1.module+el8.4.0+9091+650b220a.x86_64,
it works normally.

# qemu-img create -f qcow2 base.qcow2 64M
Formatting 'base.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=67108864 lazy_refcounts=off refcount_bits=16

# qemu-img create -f qcow2 -b base.qcow2 -F qcow2 top.qcow2 64M
Formatting 'top.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=67108864 backing_file=base.qcow2 backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16

qemu command line:
/usr/libexec/qemu-kvm \
-nodefaults \
-qmp stdio \
-blockdev qcow2,node-name=base,file.driver=file,file.filename=base.qcow2 \
-blockdev qcow2,node-name=top,file.driver=file,file.filename=top.qcow2,backing=base \
-blockdev copy-on-read,node-name=cor,file=top \
-device virtio-blk,drive=cor \
-qmp tcp:127.0.0.1:4444,server,nowait \

# telnet localhost 4444
Escape character is '^]'.
{"QMP": {"version": {"qemu": {"micro": 0, "minor": 2, "major": 5}, "package": "qemu-kvm-5.2.0-1.module+el8.4.0+9091+650b220a"}, "capabilities": ["oob"]}}

{'execute':'qmp_capabilities'}
{"return": {}}

{'execute':'block-commit','arguments':{'job-id':'commit','device':'cor','base-node':'base'}}
{"timestamp": {"seconds": 1607908835, "microseconds": 839549}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "commit"}}
{"timestamp": {"seconds": 1607908835, "microseconds": 839839}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "commit"}}
{"return": {}}
{"timestamp": {"seconds": 1607908835, "microseconds": 847915}, "event": "JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "commit"}}
{"timestamp": {"seconds": 1607908835, "microseconds": 848052}, "event": "BLOCK_JOB_READY", "data": {"device": "commit", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}}
{"return": {}}

{'execute':'block-job-complete','arguments':{'device':'commit'}}
{"timestamp": {"seconds": 1607908863, "microseconds": 923489}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "commit"}}
{"timestamp": {"seconds": 1607908863, "microseconds": 923617}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "commit"}}
{"timestamp": {"seconds": 1607908863, "microseconds": 923837}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "commit", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}}
{"timestamp": {"seconds": 1607908863, "microseconds": 923938}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "commit"}}
{"timestamp": {"seconds": 1607908863, "microseconds": 924036}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "commit"}}

{'execute':'blockdev-del','arguments':{'node-name':'cor'}}
{"return": {}}

{'execute':'blockdev-del','arguments':{'node-name':'top'}}
{"return": {}}

According to Comment 23, set status to VERIFIED.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (virt:av bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2021:2098