[Libguestfs] [nbdkit PATCH] swab: Implement .block_size callback
Laszlo Ersek
lersek at redhat.com
Wed Feb 23 08:23:12 UTC 2022
On 02/22/22 15:41, Eric Blake wrote:
> On Tue, Feb 22, 2022 at 12:49:13PM +0100, Laszlo Ersek wrote:
>> On 02/21/22 23:00, Eric Blake wrote:
>>> We were previously enforcing minimum block size with EINVAL for
>>> too-small requests. Advertise this to the client.
>>> ---
>>> filters/swab/nbdkit-swab-filter.pod | 6 ++++++
>>> filters/swab/swab.c | 24 +++++++++++++++++++++++-
>>> 2 files changed, 29 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/filters/swab/nbdkit-swab-filter.pod b/filters/swab/nbdkit-swab-filter.pod
>>> index f8500150..030a0852 100644
>>> --- a/filters/swab/nbdkit-swab-filter.pod
>>> +++ b/filters/swab/nbdkit-swab-filter.pod
>>> @@ -34,6 +34,11 @@ the last few bytes, combine this filter with
>>> L<nbdkit-truncate-filter(1)>; fortunately, sector-based disk images
>>> are already suitably sized.
>>>
>>> +Note that this filter fails operations that are not aligned to the
>>> +swab-bits boundaries; if you need byte-level access, apply the
>>> +L<nbdkit-blocksize-filter(1)> before this one, to get
>>> +read-modify-write access to individual bytes.
>>> +
>>> =head1 PARAMETERS
>>
>> I understand that the alignment of requests is enforced, but what
>> happens if the client sends a request (correctly aligned) that is 17
>> bytes in size, for example?
>>
>> ... Aha, so is_aligned() doesn't only check "offset", it also checks
>> "count". That wasn't clear to me from the addition to
>> "filters/swab/nbdkit-swab-filter.pod". I suggest spelling that out more
>> explicitly.
>
> I went with:
>
> -Note that this filter fails operations that are not aligned to the
> -swab-bits boundaries; if you need byte-level access, apply the
> -L<nbdkit-blocksize-filter(1)> before this one, to get
> -read-modify-write access to individual bytes.
> +Note that this filter fails operations where the offset or count are
> +not aligned to the swab-bits boundaries; if you need byte-level
> +access, apply the L<nbdkit-blocksize-filter(1)> before this one, to
> +get read-modify-write access to individual bytes.
>
>>> +/* Block size constraints. */
>>> +static int
>>> +swab_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 = bits/8;
>>> + *preferred = 512;
>>> + *maximum = 0xffffffff;
>>> + }
>>> + else {
>>> + *minimum = MAX (*minimum, bits/8);
>>> + }
>>
>> Given that the count too must be a whole multiple of the swap-block size
>> (correctly so), what if the underlying plugin specifies a minimum block
>> size of 17?
>
> Not possible ;) Minimum block size must be a power of 2 between 1 and
> 64k; the plugin layer enforces this. Since swab-bits alignments are
> 1, 2, 4, or 8 (also a power of 2), the MAX() operation is sufficient
> without needing ROUND_UP.
>
>>
>> I think that will take effect here, and then this filter will specify
>> such a minimum block size (17) that it will, in turn, reject
>> unconditionally. That kind of defeats the purpose of exposing a "minimum
>> block size".
>>
>> Wouldn't it be better if, on the "else" branch, we rounded up "*minimum"?
>>
>> *minimum = ROUND_UP (*minimum, bits/8);
>>
>
> Now in as amended as commit b9f8ef83
>
Thanks!
More information about the Libguestfs
mailing list