[v2,2/5] block: Generic file creation fallback

If a protocol driver does not support image creation, we can see whether
maybe the file exists already.  If so, just truncating it will be
sufficient.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 147 insertions(+), 12 deletions(-)

On Wed, 2020-01-22 at 17:45 +0100, Max Reitz wrote:
> If a protocol driver does not support image creation, we can see whether
> maybe the file exists already.  If so, just truncating it will be
> sufficient.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 147 insertions(+), 12 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 99ce26d64d..e167eca04b 100644
> --- a/block.c
> +++ b/block.c
> @@ -532,20 +532,139 @@ out:
>      return ret;
>  }
>  
> -int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
> +/**
> + * Helper function for bdrv_create_file_fallback(): Resize @blk to at
> + * least the given @minimum_size.
> + *
> + * On success, return @blk's actual length.
> + * Otherwise, return -errno.
> + */
> +static int64_t create_file_fallback_truncate(BlockBackend *blk,
> +                                             int64_t minimum_size, Error **errp)
>  {
> -    BlockDriver *drv;
> +    Error *local_err = NULL;
> +    int64_t size;
> +    int ret;
> +
> +    ret = blk_truncate(blk, minimum_size, false, PREALLOC_MODE_OFF, &local_err);
> +    if (ret < 0 && ret != -ENOTSUP) {
> +        error_propagate(errp, local_err);
> +        return ret;
> +    }
> +

...


> +    size = blk_getlength(blk);
> +    if (size < 0) {
> +        error_free(local_err);
> +        error_setg_errno(errp, -size,
> +                         "Failed to inquire the new image file's length");
> +        return size;
> +    }
> +
> +    if (size < minimum_size) {
> +        /* Need to grow the image, but we failed to do that */
> +        error_propagate(errp, local_err);
> +        return -ENOTSUP;
> +    }

Very minor nitpick:

The above code basically handles case when truncate is not supported,
by trying to see file size is large enough anyway.
If truncate succeed this is a bit redundant, but doesn't hurt to be honest.
If truncate is not supported, it also works, but I am thinking that
maybe its is better to create a generic truncate failback instead
that will 'work' when asked for same size resize or smaller that existing size + exact=false.

Currently when the above failback doesn't work (meaning that we indeed have to enlarge the file)
we get this error message because truncate is not supported.

[root@fedora31vm ~/qemu]# qemu-img create -f raw nvme://0000:03:00.0 10000M
Formatting 'nvme://0000:03:00.0', fmt=raw size=10485760000
qemu-img: nvme://0000:03:00.0: Image format driver does not support resize

If we had generic truncate failback, it could maybe be smarter about this and say something like
'Can increase the size of the image'

But if you feel like that is not important, I don't have any issue to keep this as is,
since the code does work.


> +    error_free(local_err);
> +    local_err = NULL;
> +
> +    return size;
> +}
> +
> +/**
> + * Helper function for bdrv_create_file_fallback(): Zero the first
> + * sector to remove any potentially pre-existing image header.
> + */
> +static int create_file_fallback_zero_first_sector(BlockBackend *blk,
> +                                                  int64_t current_size,
> +                                                  Error **errp)
> +{
> +    int64_t bytes_to_clear;
> +    int ret;
> +
> +    bytes_to_clear = MIN(current_size, BDRV_SECTOR_SIZE);
> +    if (bytes_to_clear) {
> +        ret = blk_pwrite_zeroes(blk, 0, bytes_to_clear, BDRV_REQ_MAY_UNMAP);
> +        if (ret < 0) {
> +            error_setg_errno(errp, -ret,
> +                             "Failed to clear the new image's first sector");
> +            return ret;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int bdrv_create_file_fallback(const char *filename, BlockDriver *drv,
> +                                     QemuOpts *opts, Error **errp)
> +{
> +    BlockBackend *blk;
> +    QDict *options = qdict_new();
> +    int64_t size = 0;
> +    char *buf = NULL;
> +    PreallocMode prealloc;
>      Error *local_err = NULL;
>      int ret;
>  
> +    size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> +    prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
> +                               PREALLOC_MODE_OFF, &local_err);
> +    g_free(buf);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return -EINVAL;
> +    }
> +
> +    if (prealloc != PREALLOC_MODE_OFF) {
> +        error_setg(errp, "Unsupported preallocation mode '%s'",
> +                   PreallocMode_str(prealloc));
> +        return -ENOTSUP;
> +    }
> +
> +    qdict_put_str(options, "driver", drv->format_name);
> +
> +    blk = blk_new_open(filename, NULL, options,
> +                       BDRV_O_RDWR | BDRV_O_RESIZE, errp);
> +    if (!blk) {
> +        error_prepend(errp, "Protocol driver '%s' does not support image "
> +                      "creation, and opening the image failed: ",
> +                      drv->format_name);
> +        return -EINVAL;
> +    }
> +
> +    size = create_file_fallback_truncate(blk, size, errp);
> +    if (size < 0) {
> +        ret = size;
> +        goto out;
> +    }
> +
> +    ret = create_file_fallback_zero_first_sector(blk, size, errp);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    ret = 0;
> +out:
> +    blk_unref(blk);
> +    return ret;
> +}

Looks all right, very good code.

> +
> +int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
> +{
> +    BlockDriver *drv;
> +
>      drv = bdrv_find_protocol(filename, true, errp);
>      if (drv == NULL) {
>          return -ENOENT;
>      }
>  
> -    ret = bdrv_create(drv, filename, opts, &local_err);
> -    error_propagate(errp, local_err);
> -    return ret;
> +    if (drv->bdrv_co_create_opts) {
> +        return bdrv_create(drv, filename, opts, errp);
> +    } else {
> +        return bdrv_create_file_fallback(filename, drv, opts, errp);
> +    }
>  }
>  
>  /**
> @@ -1422,6 +1541,24 @@ QemuOptsList bdrv_runtime_opts = {
>      },
>  };
>  
> +static QemuOptsList fallback_create_opts = {
> +    .name = "fallback-create-opts",
> +    .head = QTAILQ_HEAD_INITIALIZER(fallback_create_opts.head),
> +    .desc = {
> +        {
> +            .name = BLOCK_OPT_SIZE,
> +            .type = QEMU_OPT_SIZE,
> +            .help = "Virtual disk size"
> +        },
> +        {
> +            .name = BLOCK_OPT_PREALLOC,
> +            .type = QEMU_OPT_STRING,
> +            .help = "Preallocation mode (allowed values: off)"
> +        },
> +        { /* end of list */ }
> +    }
> +};
> +
>  /*
>   * Common part for opening disk images and files
>   *
> @@ -5749,15 +5886,13 @@ void bdrv_img_create(const char *filename, const char *fmt,
>          return;
>      }
>  
> -    if (!proto_drv->create_opts) {
> -        error_setg(errp, "Protocol driver '%s' does not support image creation",
> -                   proto_drv->format_name);
> -        return;
> -    }
> -
>      /* Create parameter list */
>      create_opts = qemu_opts_append(create_opts, drv->create_opts);
> -    create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
> +    if (proto_drv->create_opts) {
> +        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
> +    } else {
> +        create_opts = qemu_opts_append(create_opts, &fallback_create_opts);
> +    }
>  
>      opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);

I also tested the code a bit on the qemu virtual nvme drive.
Thanks for the patch series,

Best regards,
	Maxim Levitsky