Re: [PATCH v2 04/20] fuse: Allow growable exports

[Kevin Wolf]

Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> These will behave more like normal files in that writes beyond the EOF
> will automatically grow the export size.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qapi/block-export.json |  6 +++++-
>  block/export/fuse.c    | 12 +++++++++++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index cb5bd54cbf..cb26daa98b 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json
> @@ -183,10 +183,14 @@
>  # @mountpoint: Path on which to export the block device via FUSE.
>  #              This must point to an existing regular file.
>  #
> +# @growable: Whether writes beyond the EOF should grow the block node
> +#            accordingly. (default: false)
> +#
>  # Since: 5.2
>  ##
>  { 'struct': 'BlockExportOptionsFuse',
> -  'data': { 'mountpoint': 'str' },
> +  'data': { 'mountpoint': 'str',
> +            '*growable': 'bool' },
>    'if': 'defined(CONFIG_FUSE)' }
>  
>  ##
> diff --git a/block/export/fuse.c b/block/export/fuse.c
> index 8fc667231d..f3a84579ba 100644
> --- a/block/export/fuse.c
> +++ b/block/export/fuse.c
> @@ -45,6 +45,7 @@ typedef struct FuseExport {
>  
>      char *mountpoint;
>      bool writable;
> +    bool growable;
>  } FuseExport;
>  
>  static GHashTable *exports;
> @@ -101,6 +102,7 @@ static int fuse_export_create(BlockExport *blk_exp,
>  
>      exp->mountpoint = g_strdup(args->mountpoint);
>      exp->writable = blk_exp_args->writable;
> +    exp->growable = args->growable;
>  
>      ret = setup_fuse_export(exp, args->mountpoint, errp);
>      if (ret < 0) {
> @@ -436,7 +438,15 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, 
> const char *buf,
>  
>      size = MIN(size, BDRV_REQUEST_MAX_BYTES);
>      if (offset + size > length) {
> -        size = length - offset;
> +        if (exp->growable) {
> +            ret = fuse_do_truncate(exp, offset + size, PREALLOC_MODE_OFF);

Do we need BDRV_REQ_ZERO_WRITE for blk_truncate() in this case?

Actually, since we export a regular file, it would probably be good for
the setattr case, too.

If someone actually uses this to sequentially write past the end of the
file, it will be quite inefficient because fuse_do_truncate() temporarily
acquires locks for each write request. It might be a good idea to
acquire BLK_PERM_RESIZE from the start for growable exports (like image
formats do for bs->file).

> +            if (ret < 0) {
> +                fuse_reply_err(req, -ret);
> +                return;
> +            }
> +        } else {
> +            size = length - offset;
> +        }
>      }

Kevin