Re: [Qemu-devel] [PATCH v5 RESEND 3/4] raw-posix: Add full image preallocation option

[Fam Zheng <famz@redhat.com>]

On Tue, 02/11 15:07, Hu Tao wrote:
> This patch adds a new option preallocation for raw format, and implements
> full preallocation.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  block/raw-posix.c | 43 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 01fb41a..1961b74 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1229,11 +1229,22 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
>      int fd;
>      int result = 0;
>      int64_t total_size = 0;
> +    PreallocMode prealloc = PREALLOC_MODE_OFF;
>  
>      /* Read out options */
>      while (options && options->name) {
>          if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
>              total_size = options->value.n & BDRV_SECTOR_MASK;
> +        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> +            if (!options->value.s || !strcmp(options->value.s, "off")) {
> +                prealloc = PREALLOC_MODE_OFF;
> +            } else if (!strcmp(options->value.s, "full")) {
> +                prealloc = PREALLOC_MODE_FULL;
> +            } else {
> +                error_setg(errp, "Invalid preallocation mode: '%s'",
> +                           options->value.s);
> +                return -EINVAL;
> +            }
>          }
>          options++;
>      }
> @@ -1243,16 +1254,27 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
>      if (fd < 0) {
>          result = -errno;
>          error_setg_errno(errp, -result, "Could not create file");
> -    } else {
> -        if (ftruncate(fd, total_size) != 0) {
> -            result = -errno;
> -            error_setg_errno(errp, -result, "Could not resize file");
> -        }
> -        if (qemu_close(fd) != 0) {
> -            result = -errno;
> -            error_setg_errno(errp, -result, "Could not close the new file");
> +        goto out;
> +    }
> +    if (ftruncate(fd, total_size) != 0) {
> +        result = -errno;
> +        error_setg_errno(errp, -result, "Could not resize file");
> +        goto out_close;
> +    }
> +    if (prealloc == PREALLOC_MODE_FULL) {
> +        /* posix_fallocate() doesn't set errno. */
> +        result = -posix_fallocate(fd, 0, total_size);
> +        if (result != 0) {
> +            error_setg_errno(errp, -result,
> +                             "Could not preallocate data for the new file");
>          }
>      }
> +out_close:
> +    if (qemu_close(fd) != 0) {
> +        result = -errno;
> +        error_setg_errno(errp, -result, "Could not close the new file");

If errp is already set because ftruncate or posix_ftruncate failed, and
qemu_close() fails too, the call to error_setg_errno() will abort on failing
the assertion of (*errp == NULL).

You could either embed the two failures in one error message, or pick the
urgent one and drop the other.

Thanks,
Fam

> +    }
> +out:
>      return result;
>  }
>  
> @@ -1403,6 +1425,11 @@ static QEMUOptionParameter raw_create_options[] = {
>          .type = OPT_SIZE,
>          .help = "Virtual disk size"
>      },
> +    {
> +        .name = BLOCK_OPT_PREALLOC,
> +        .type = OPT_STRING,
> +        .help = "Preallocation mode (allowed values: off, full)"
> +    },
>      { NULL }
>  };
>  
> -- 
> 1.8.0
>