diff mbox series

block/blkio: Tolerate device size changes

Message ID 20221029122031.975273-1-afaria@redhat.com
State New
Headers show
Series block/blkio: Tolerate device size changes | expand

Commit Message

Alberto Faria Oct. 29, 2022, 12:20 p.m. UTC
Some libblkio drivers may be able to work with regular files (e.g.,
io_uring) or otherwise resizable devices. Conservatively set
BlockDriver::has_variable_length to true to ensure bdrv_nb_sectors()
always gives up-to-date results.

Also implement BlockDriver::bdrv_co_truncate for the case where no
preallocation is needed and the device already has a size compatible
with what was requested.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---

This is based on Stefan's block tree:
https://gitlab.com/stefanha/qemu/-/commits/block

 block/blkio.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Stefan Hajnoczi Oct. 31, 2022, 6:33 p.m. UTC | #1
On Sat, Oct 29, 2022 at 01:20:31PM +0100, Alberto Faria wrote:
> Some libblkio drivers may be able to work with regular files (e.g.,
> io_uring) or otherwise resizable devices. Conservatively set
> BlockDriver::has_variable_length to true to ensure bdrv_nb_sectors()
> always gives up-to-date results.
> 
> Also implement BlockDriver::bdrv_co_truncate for the case where no
> preallocation is needed and the device already has a size compatible
> with what was requested.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
> 
> This is based on Stefan's block tree:
> https://gitlab.com/stefanha/qemu/-/commits/block
> 
>  block/blkio.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

Thanks, applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block

Stefan
Kevin Wolf Nov. 2, 2022, 11:56 a.m. UTC | #2
Am 29.10.2022 um 14:20 hat Alberto Faria geschrieben:
> Some libblkio drivers may be able to work with regular files (e.g.,
> io_uring) or otherwise resizable devices. Conservatively set
> BlockDriver::has_variable_length to true to ensure bdrv_nb_sectors()
> always gives up-to-date results.
> 
> Also implement BlockDriver::bdrv_co_truncate for the case where no
> preallocation is needed and the device already has a size compatible
> with what was requested.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
> 
> This is based on Stefan's block tree:
> https://gitlab.com/stefanha/qemu/-/commits/block
> 
>  block/blkio.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/block/blkio.c b/block/blkio.c
> index 82f26eedd2..190454cdbe 100644
> --- a/block/blkio.c
> +++ b/block/blkio.c
> @@ -845,6 +845,31 @@ static int64_t blkio_getlength(BlockDriverState *bs)
>      return capacity;
>  }
>  
> +static int coroutine_fn blkio_truncate(BlockDriverState *bs, int64_t offset,
> +                                       bool exact, PreallocMode prealloc,
> +                                       BdrvRequestFlags flags, Error **errp)
> +{
> +    int64_t current_length;
> +
> +    if (prealloc != PREALLOC_MODE_OFF) {
> +        error_setg(errp, "Unsupported preallocation mode '%s'",
> +                   PreallocMode_str(prealloc));
> +        return -ENOTSUP;
> +    }
> +
> +    current_length = blkio_getlength(bs);
> +
> +    if (offset > current_length) {
> +        error_setg(errp, "Cannot grow device");
> +        return -EINVAL;
> +    } else if (exact && offset != current_length) {
> +        error_setg(errp, "Cannot resize device");
> +        return -ENOTSUP;
> +    }
> +
> +    return 0;
> +}
> +
>  static int blkio_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
>  {
>      return 0;
> @@ -960,10 +985,12 @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp)
>      { \
>          .format_name             = name, \
>          .protocol_name           = name, \
> +        .has_variable_length     = true, \

I don't think this is a good idea, it will read the capacity from
libblkio before every single request, which can't result in good
performance.

We're generally only using .has_variable_length for removable media on
the backend (i.e. for host_cdrom and filters that can potentially sit on
top of it, but will used the cached value on the protocol level again
for other protocol drivers).

If you resize a host_device, you need to use QMP 'block_resize'
explicitly. I think it should be the same for libblkio.

Kevin
diff mbox series

Patch

diff --git a/block/blkio.c b/block/blkio.c
index 82f26eedd2..190454cdbe 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -845,6 +845,31 @@  static int64_t blkio_getlength(BlockDriverState *bs)
     return capacity;
 }
 
+static int coroutine_fn blkio_truncate(BlockDriverState *bs, int64_t offset,
+                                       bool exact, PreallocMode prealloc,
+                                       BdrvRequestFlags flags, Error **errp)
+{
+    int64_t current_length;
+
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_str(prealloc));
+        return -ENOTSUP;
+    }
+
+    current_length = blkio_getlength(bs);
+
+    if (offset > current_length) {
+        error_setg(errp, "Cannot grow device");
+        return -EINVAL;
+    } else if (exact && offset != current_length) {
+        error_setg(errp, "Cannot resize device");
+        return -ENOTSUP;
+    }
+
+    return 0;
+}
+
 static int blkio_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
     return 0;
@@ -960,10 +985,12 @@  static void blkio_refresh_limits(BlockDriverState *bs, Error **errp)
     { \
         .format_name             = name, \
         .protocol_name           = name, \
+        .has_variable_length     = true, \
         .instance_size           = sizeof(BDRVBlkioState), \
         .bdrv_file_open          = blkio_file_open, \
         .bdrv_close              = blkio_close, \
         .bdrv_getlength          = blkio_getlength, \
+        .bdrv_co_truncate        = blkio_truncate, \
         .bdrv_get_info           = blkio_get_info, \
         .bdrv_attach_aio_context = blkio_attach_aio_context, \
         .bdrv_detach_aio_context = blkio_detach_aio_context, \