[Qemu-block] Use of PreallocMode in block drivers (was: [Qemu-devel] [PATCH] block/rbd: add preallocation support)

[Markus Armbruster]

Cc: Peter for a libvirt perspective.

Stefano Garzarella <address@hidden> writes:

> This patch adds the support of preallocation (off/full) for the RBD
> block driver.
> If available, we use rbd_writesame() to quickly fill the image when
> full preallocation is required.
>
> Signed-off-by: Stefano Garzarella <address@hidden>
> ---
>  block/rbd.c          | 149 ++++++++++++++++++++++++++++++++++++++-----
>  qapi/block-core.json |   4 +-
>  2 files changed, 136 insertions(+), 17 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index 0c549c9935..29dd1bb040 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -13,6 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  
> +#include "qemu/units.h"
>  #include <rbd/librbd.h>
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
> @@ -331,6 +332,110 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)
>      }
>  }
>  
> +static int qemu_rbd_do_truncate(rbd_image_t image, int64_t offset,
> +                                PreallocMode prealloc, Error **errp)
> +{
> +    uint64_t current_length;
> +    char *buf = NULL;
> +    int ret;
> +
> +    ret = rbd_get_size(image, &current_length);
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "Failed to get file length");
> +        goto out;
> +    }
> +
> +    if (current_length > offset && prealloc != PREALLOC_MODE_OFF) {
> +        error_setg(errp, "Cannot use preallocation for shrinking files");
> +        ret = -ENOTSUP;
> +        goto out;
> +    }
> +
> +    switch (prealloc) {
> +    case PREALLOC_MODE_FULL: {
[...]
> +    case PREALLOC_MODE_OFF:
[...]
> +    default:
> +        error_setg(errp, "Unsupported preallocation mode: %s",
> +                   PreallocMode_str(prealloc));
> +        ret = -ENOTSUP;
> +        goto out;
> +    }

Other block drivers also accept only some values of PreallocMode.  Okay.

I wonder whether management applications need to know which values are
supported.

Let me review support in drivers:

* file (file-win32.c)
* iscsi
* nfs
* qed
* ssh

  - Reject all but PREALLOC_MODE_OFF

* copy-on-read
* luks (crypto.c)
* raw

  - Pass through only

* file host_cdrom host_device (file-posix.c)

  - Reject all but PREALLOC_MODE_OFF when shrinking and for non-regular
    files
  - Reject PREALLOC_MODE_FALLOC unless CONFIG_POSIX_FALLOCATE
  - Reject PREALLOC_MODE_METADATA

* gluster

  - Reject all but PREALLOC_MODE_OFF when shrinking
  - Reject PREALLOC_MODE_FALLOC unless CONFIG_GLUSTERFS_FALLOCATE
  - Reject PREALLOC_MODE_FULL unless CONFIG_GLUSTERFS_ZEROFILL
  - Reject PREALLOC_MODE_METADATA

* qcow2

  - Reject all but PREALLOC_MODE_OFF when shrinking and with a backing
    file
  
* rbd with this patch

  - Reject all but PREALLOC_MODE_OFF when shrinking
  - Reject PREALLOC_MODE_METADATA and PREALLOC_MODE_FALLOC

* sheepdog

  - Reject PREALLOC_MODE_METADATA and PREALLOC_MODE_FALLOC
  - Doesn't support shrinking

* vdi

  - Reject PREALLOC_MODE_FALLOC and PREALLOC_MODE_FULL
  - Doesn't support shrinking

* blkdebug
* blklogwrites
* blkverify
* bochs
* cloop
* dmg
* ftp
* ftps
* http
* https
* luks
* nbd
* null-aio
* null-co
* nvme
* parallels
* qcow
* quorum
* replication
* throttle
* vhdx
* vmdk
* vpc
* vvfat
* vxhs

  - These appear not to use PreallocMode: they don't implement
    .bdrv_co_truncate(), and either don't implement .bdrv_co_create() or
    implement it without a prealloc parameter.

Looks good to me.

> +
> +    ret = 0;
> +
> +out:
> +    g_free(buf);
> +    return ret;
> +}
> +
>  static QemuOptsList runtime_opts = {
>      .name = "rbd",
>      .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
[...]
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 7ccbfff9d0..db25a4065b 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4277,13 +4277,15 @@
>  #                   point to a snapshot.
>  # @size             Size of the virtual disk in bytes
>  # @cluster-size     RBD object size
> +# @preallocation    Preallocation mode (allowed values: off, full)
>  #
>  # Since: 2.12
>  ##
>  { 'struct': 'BlockdevCreateOptionsRbd',
>    'data': { 'location':         'BlockdevOptionsRbd',
>              'size':             'size',
> -            '*cluster-size' :   'size' } }
> +            '*cluster-size' :   'size',
> +            '*preallocation':   'PreallocMode' } }
>  
>  ##
>  # @BlockdevVmdkSubformat:

The non-support of values 'metadata' and 'falloc' is not visible in
introspection, only in documentation.  No reason to block this patch, as
the other block drivers have the same introspection weakness (only
sheepdog and vdi bother to document).

Should we address the introspection weakness?  Only if there's a use for
the information, I think.

Should we improve documentation for the other block drivers?