[00/14] block: blockdev-del force=false

Message

Vladimir Sementsov-Ogievskiy Feb. 7, 2022, 4:37 p.m. UTC

Hi all!

Currently close-path in block layer can't fail. Neither bdrv_close() nor
.bdrv_close() handlers have return value. So, when something fails (for
example cached metadata flush) the only thing we can is to print an
error to log.

This behavior is OK on VM stop: we want to stop the running process and
if it can't save metadata, we can do nothing about it. We only rely
on disk format safety: dirty bit or something like this, which will
force some checks on next VM start.

Still there are cases when we _can_ reasonably handle failures on close:

Consider backup: if something failed when we close target qcow2 image,
correct thing to do is to reject the resulting image and consider the
whole backup failed. Still, current behavior is to ignore such failures
which probably leads to creating inconsistent backup images.

Another thing (that actually brings me to creating this series) is dirty
bitmap manipulation: assume we start Qemu (or qemu-storage-daemon) to do
some manipulations with bitmaps and prepare some new qcow2 image with
these bitmaps. Native way to store bitmaps is just to close qcow2 node,
so we either call blockdev-del or just terminate the process. But in
either case there is no way to get the result: is everything stored
correctly or something failed?

So, welcome the new series, that provide a new interface
blockdev-del(force=false) to either get a clean error or be sure that
everything (bitmaps, L2 caches, refcounts) correctly stored and flushed.

Intended usage:
 - blockdev-create / blockdev-add to add a qcow2 node
 - do some operations with it (for example backup job
   or bitmap manipulations)
 - blockdev-del(force=false)
    - If it fails
      - report the error to user
      - repeat the command with force=true (or just
        terminate Qemu process if no more operations to do),
      - _remove_ the prepared qcow2 image from FS
    - If it succeeds
      - Great, now we are sure that qcow2 image prepared as we want and
        correctly closed/saved/flushed. As well, node is removed and
        Qemu will not touch it anymore for sure. So, report success!

The series could be found here:
  git: https://src.openvz.org/scm/~vsementsov/qemu.git  tag  up-block-safe-remove-v1
  browse: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=refs%2Ftags%2Fup-block-safe-remove-v1

The series is based on my "[PATCH 00/14] block: cleanup backing and file handling"
Based-on: <[e-mail]>

Vladimir Sementsov-Ogievskiy (14):
  block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child
  block: drop bdrv_detach_child()
  block: bdrv_refresh_perms(): allow external tran
  block: add bdrv_try_set_aio_context_tran transaction action
  block: merge bdrv_delete and bdrv_close
  block: bdrv_delete(): drop unnecessary zeroing
  block: implemet bdrv_try_unref()
  qapi/block-core: add 'force' argument to blockdev-del
  qcow2: qcow2_inactivate(): use qcow2_flush_caches()
  qcow2: qcow2_inactivate(): don't call qcow2_mark_clean() when RO
  qcow2: refactor qcow2_inactivate
  qcow2: implement .bdrv_close_safe
  block/file-posix: implement .bdrv_close_safe
  iotests: add test for blockdev-del(force=false)

 qapi/block-core.json                          |   6 +-
 block/qcow2.h                                 |   6 +-
 include/block/block.h                         |   3 +
 include/block/block_int.h                     |   2 +
 block.c                                       | 438 +++++++++++++-----
 block/file-posix.c                            |  38 +-
 block/monitor/block-hmp-cmds.c                |   2 +-
 block/qcow2-bitmap.c                          |   2 +-
 block/qcow2-refcount.c                        |  22 +-
 block/qcow2.c                                 | 152 ++++--
 blockdev.c                                    |  17 +-
 hw/block/xen-block.c                          |   2 +-
 tests/qemu-iotests/026.out                    | 126 ++---
 tests/qemu-iotests/071.out                    |   3 +-
 tests/qemu-iotests/080.out                    |   4 +-
 .../tests/blockdev-del-close-failure          |  54 +++
 .../tests/blockdev-del-close-failure.out      |   4 +
 17 files changed, 611 insertions(+), 270 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/blockdev-del-close-failure
 create mode 100644 tests/qemu-iotests/tests/blockdev-del-close-failure.out