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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2762 bytes --]

On 15.10.20 12:41, Kevin Wolf wrote:
> 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?

Ah, yes.  (Sorry, code is a bit old and I forgot when I revised it...)

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

Yes.

> 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).

Oh, yes, that makes sense.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]