Libguestfs - Feb 2022 - [PATCH nbdkit] cache, cow: Export block size constraints

On 02/20/22 21:49, Richard W.M. Jones wrote:
> Because these filters perform a read-modify-write cycle for requests
> which are smaller than the block size of the filter, we can adjust or
> set the preferred block size to the block size of the filter or the
> preferred block size of the underlying plugin, whichever is larger.
> 
> We're careful not to set a preferred block size which is larger than
> the maximum block size.
> 
> After this change:
> 
> $ ./nbdkit --filter=cow floppy /usr/share/doc/nbdkit-devel \
>                                cow-block-size=128k \
>                                --run 'nbdinfo $uri'
> protocol: newstyle-fixed without TLS
> export="":
> 	export-size: 1458176 (1424K)
> 	content: DOS/MBR boot sector; partition 1 : ID=0xc, start-CHS
(0x3ff,254,63), end-CHS (0x3ff,254,63), startsector 2048, 800 sectors
> 	uri: nbd://localhost:10809/
> 	contexts:
> 		base:allocation
> 	is_rotational: false
> 	is_read_only: false
> 	can_cache: true
> 	can_df: true
> 	can_fast_zero: false
> 	can_flush: true
> 	can_fua: true
> 	can_multi_conn: true
> 	can_trim: true
> 	can_zero: false
> 	block_size_minimum: 1
> 	block_size_preferred: 131072      <--- note
> 	block_size_maximum: 4294967295
> ---
>  filters/cache/cache.c | 21 +++++++++++++++++++++
>  filters/cow/cow.c     | 21 +++++++++++++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/filters/cache/cache.c b/filters/cache/cache.c
> index c912c5fb..f0ee42f1 100644
> --- a/filters/cache/cache.c
> +++ b/filters/cache/cache.c
> @@ -260,6 +260,26 @@ cache_get_size (nbdkit_next *next,
>    return size;
>  }
>  
> +/* Block size constraints. */
> +static int
> +cache_block_size (nbdkit_next *next, void *handle,
> +                  uint32_t *minimum, uint32_t *preferred, uint32_t
*maximum)
> +{
> +  if (next->block_size (next, minimum, preferred, maximum) == -1)
> +    return -1;
> +
> +  if (*minimum == 0) {         /* No constraints set by the plugin. */
> +    *minimum = 1;
> +    *preferred = blksize;
> +    *maximum = 0xffffffff;
> +  }
> +  else if (*maximum >= blksize) {
Do we need braces here?
> +    *preferred = MAX (*preferred, blksize);
> +  }
> +
> +  return 0;
> +}
> +
>  /* Force an early call to cache_get_size because we have to set the
>   * backing file size and bitmap size before any other read or write
>   * calls.
> @@ -716,6 +736,7 @@ static struct nbdkit_filter filter = {
>    .get_ready         = cache_get_ready,
>    .prepare           = cache_prepare,
>    .get_size          = cache_get_size,
> +  .block_size        = cache_block_size,
>    .can_cache         = cache_can_cache,
>    .can_fast_zero     = cache_can_fast_zero,
>    .can_flush         = cache_can_flush,
> diff --git a/filters/cow/cow.c b/filters/cow/cow.c
> index e111c60f..cdc5b44f 100644
> --- a/filters/cow/cow.c
> +++ b/filters/cow/cow.c
> @@ -182,6 +182,26 @@ cow_get_size (nbdkit_next *next,
>    return size;
>  }
>  
> +/* Block size constraints. */
> +static int
> +cow_block_size (nbdkit_next *next, void *handle,
> +                uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
> +{
> +  if (next->block_size (next, minimum, preferred, maximum) == -1)
> +    return -1;
> +
> +  if (*minimum == 0) {         /* No constraints set by the plugin. */
> +    *minimum = 1;
> +    *preferred = blksize;
> +    *maximum = 0xffffffff;
> +  }
> +  else if (*maximum >= blksize) {
Same here.

Makes sense to me otherwise.

Acked-by: Laszlo Ersek <lersek at redhat.com>

Thanks
Laszlo
> +    *preferred = MAX (*preferred, blksize);
> +  }
> +
> +  return 0;
> +}
> +
>  /* Force an early call to cow_get_size because we have to set the
>   * backing file size and bitmap size before any other read or write
>   * calls.
> @@ -762,6 +782,7 @@ static struct nbdkit_filter filter = {
>    .get_ready         = cow_get_ready,
>    .prepare           = cow_prepare,
>    .get_size          = cow_get_size,
> +  .block_size        = cow_block_size,
>    .can_write         = cow_can_write,
>    .can_flush         = cow_can_flush,
>    .can_trim          = cow_can_trim,
>

On Mon, Feb 21, 2022 at 10:22:04AM +0100, Laszlo Ersek
wrote:
> > +/* Block size constraints. */
> > +static int
> > +cache_block_size (nbdkit_next *next, void *handle,
> > +                  uint32_t *minimum, uint32_t *preferred, uint32_t
*maximum)
> > +{
> > +  if (next->block_size (next, minimum, preferred, maximum) == -1)
> > +    return -1;
> > +
> > +  if (*minimum == 0) {         /* No constraints set by the plugin.
*/
> > +    *minimum = 1;
> > +    *preferred = blksize;
> > +    *maximum = 0xffffffff;
> > +  }
> > +  else if (*maximum >= blksize) {
> 
> Do we need braces here?
> 
> > +    *preferred = MAX (*preferred, blksize);
> > +  }
I don't think we need them, but it might be clearer with them.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html