Skip to content
  • Fiona Ebner's avatar
    99dd4a15
    block/io: clear BDRV_BLOCK_RECURSE flag after recursing in bdrv_co_block_status · 99dd4a15
    Fiona Ebner authored
    
    
    Using fleecing backup like in [0] on a qcow2 image (with metadata
    preallocation) can lead to the following assertion failure:
    
    > bdrv_co_do_block_status: Assertion `!(ret & BDRV_BLOCK_ZERO)' failed.
    
    In the reproducer [0], it happens because the BDRV_BLOCK_RECURSE flag
    will be set by the qcow2 driver, so the caller will recursively check
    the file child. Then the BDRV_BLOCK_ZERO set too. Later up the call
    chain, in bdrv_co_do_block_status() for the snapshot-access driver,
    the assertion failure will happen, because both flags are set.
    
    To fix it, clear the recurse flag after the recursive check was done.
    
    In detail:
    
    > #0  qcow2_co_block_status
    
    Returns 0x45 = BDRV_BLOCK_RECURSE | BDRV_BLOCK_DATA |
    BDRV_BLOCK_OFFSET_VALID.
    
    > #1  bdrv_co_do_block_status
    
    Because of the data flag, bdrv_co_do_block_status() will now also set
    BDRV_BLOCK_ALLOCATED. Because of the recurse flag,
    bdrv_co_do_block_status() for the bdrv_file child will be called,
    which returns 0x16 = BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_OFFSET_VALID |
    BDRV_BLOCK_ZERO. Now the return value inherits the zero flag.
    
    Returns 0x57 = BDRV_BLOCK_RECURSE | BDRV_BLOCK_DATA |
    BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_ZERO.
    
    > #2  bdrv_co_common_block_status_above
    > #3  bdrv_co_block_status_above
    > #4  bdrv_co_block_status
    > #5  cbw_co_snapshot_block_status
    > #6  bdrv_co_snapshot_block_status
    > #7  snapshot_access_co_block_status
    > #8  bdrv_co_do_block_status
    
    Return value is propagated all the way up to here, where the assertion
    failure happens, because BDRV_BLOCK_RECURSE and BDRV_BLOCK_ZERO are
    both set.
    
    > #9  bdrv_co_common_block_status_above
    > #10 bdrv_co_block_status_above
    > #11 block_copy_block_status
    > #12 block_copy_dirty_clusters
    > #13 block_copy_common
    > #14 block_copy_async_co_entry
    > #15 coroutine_trampoline
    
    [0]:
    
    > #!/bin/bash
    > rm /tmp/disk.qcow2
    > ./qemu-img create /tmp/disk.qcow2 -o preallocation=metadata -f qcow2 1G
    > ./qemu-img create /tmp/fleecing.qcow2 -f qcow2 1G
    > ./qemu-img create /tmp/backup.qcow2 -f qcow2 1G
    > ./qemu-system-x86_64 --qmp stdio \
    > --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \
    > --blockdev qcow2,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.qcow2 \
    > --blockdev qcow2,node-name=node2,file.driver=file,file.filename=/tmp/backup.qcow2 \
    > <<EOF
    > {"execute": "qmp_capabilities"}
    > {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } }
    > {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } }
    > {"execute": "blockdev-backup", "arguments": { "device": "snap0", "target": "node1", "sync": "full", "job-id": "backup0" } }
    > EOF
    
    Signed-off-by: default avatarFiona Ebner <f.ebner@proxmox.com>
    Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
    Message-id: 20240116154839.401030-1-f.ebner@proxmox.com
    Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
    (cherry picked from commit 8a9be7992426c8920d4178e7dca59306a18c7a3a)
    Signed-off-by: default avatarMichael Tokarev <mjt@tls.msk.ru>
    99dd4a15
    block/io: clear BDRV_BLOCK_RECURSE flag after recursing in bdrv_co_block_status
    Fiona Ebner authored
    
    
    Using fleecing backup like in [0] on a qcow2 image (with metadata
    preallocation) can lead to the following assertion failure:
    
    > bdrv_co_do_block_status: Assertion `!(ret & BDRV_BLOCK_ZERO)' failed.
    
    In the reproducer [0], it happens because the BDRV_BLOCK_RECURSE flag
    will be set by the qcow2 driver, so the caller will recursively check
    the file child. Then the BDRV_BLOCK_ZERO set too. Later up the call
    chain, in bdrv_co_do_block_status() for the snapshot-access driver,
    the assertion failure will happen, because both flags are set.
    
    To fix it, clear the recurse flag after the recursive check was done.
    
    In detail:
    
    > #0  qcow2_co_block_status
    
    Returns 0x45 = BDRV_BLOCK_RECURSE | BDRV_BLOCK_DATA |
    BDRV_BLOCK_OFFSET_VALID.
    
    > #1  bdrv_co_do_block_status
    
    Because of the data flag, bdrv_co_do_block_status() will now also set
    BDRV_BLOCK_ALLOCATED. Because of the recurse flag,
    bdrv_co_do_block_status() for the bdrv_file child will be called,
    which returns 0x16 = BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_OFFSET_VALID |
    BDRV_BLOCK_ZERO. Now the return value inherits the zero flag.
    
    Returns 0x57 = BDRV_BLOCK_RECURSE | BDRV_BLOCK_DATA |
    BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_ZERO.
    
    > #2  bdrv_co_common_block_status_above
    > #3  bdrv_co_block_status_above
    > #4  bdrv_co_block_status
    > #5  cbw_co_snapshot_block_status
    > #6  bdrv_co_snapshot_block_status
    > #7  snapshot_access_co_block_status
    > #8  bdrv_co_do_block_status
    
    Return value is propagated all the way up to here, where the assertion
    failure happens, because BDRV_BLOCK_RECURSE and BDRV_BLOCK_ZERO are
    both set.
    
    > #9  bdrv_co_common_block_status_above
    > #10 bdrv_co_block_status_above
    > #11 block_copy_block_status
    > #12 block_copy_dirty_clusters
    > #13 block_copy_common
    > #14 block_copy_async_co_entry
    > #15 coroutine_trampoline
    
    [0]:
    
    > #!/bin/bash
    > rm /tmp/disk.qcow2
    > ./qemu-img create /tmp/disk.qcow2 -o preallocation=metadata -f qcow2 1G
    > ./qemu-img create /tmp/fleecing.qcow2 -f qcow2 1G
    > ./qemu-img create /tmp/backup.qcow2 -f qcow2 1G
    > ./qemu-system-x86_64 --qmp stdio \
    > --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \
    > --blockdev qcow2,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.qcow2 \
    > --blockdev qcow2,node-name=node2,file.driver=file,file.filename=/tmp/backup.qcow2 \
    > <<EOF
    > {"execute": "qmp_capabilities"}
    > {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } }
    > {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } }
    > {"execute": "blockdev-backup", "arguments": { "device": "snap0", "target": "node1", "sync": "full", "job-id": "backup0" } }
    > EOF
    
    Signed-off-by: default avatarFiona Ebner <f.ebner@proxmox.com>
    Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
    Message-id: 20240116154839.401030-1-f.ebner@proxmox.com
    Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
    (cherry picked from commit 8a9be7992426c8920d4178e7dca59306a18c7a3a)
    Signed-off-by: default avatarMichael Tokarev <mjt@tls.msk.ru>
Loading