Eric Blake
2022-Feb-21 22:00 UTC
[Libguestfs] [nbdkit PATCH] swab: Implement .block_size callback
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
=over 4
@@ -90,6 +95,7 @@ L<nbdkit(1)>,
L<nbdkit-file-plugin(1)>,
L<nbdkit-pattern-plugin(1)>,
L<nbdkit-filter(3)>,
+L<nbdkit-blocksize-filter(1)>.
L<nbdkit-truncate-filter(1)>.
=head1 AUTHORS
diff --git a/filters/swab/swab.c b/filters/swab/swab.c
index 68776eee..6e8dc981 100644
--- a/filters/swab/swab.c
+++ b/filters/swab/swab.c
@@ -44,6 +44,7 @@
#include "byte-swapping.h"
#include "isaligned.h"
#include "cleanup.h"
+#include "minmax.h"
#include "rounding.h"
/* Can only be 8 (filter disabled), 16, 32 or 64. */
@@ -85,8 +86,28 @@ swab_get_size (nbdkit_next *next,
return ROUND_DOWN (size, bits/8);
}
+/* 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);
+ }
+
+ return 0;
+}
+
/* The request must be aligned.
- * XXX We could lift this restriction with more work.
+ * If you want finer alignment, use the blocksize filter.
*/
static bool
is_aligned (uint32_t count, uint64_t offset, int *err)
@@ -220,6 +241,7 @@ static struct nbdkit_filter filter = {
.config = swab_config,
.config_help = swab_config_help,
.get_size = swab_get_size,
+ .block_size = swab_block_size,
.pread = swab_pread,
.pwrite = swab_pwrite,
.trim = swab_trim,
--
2.35.1
Richard W.M. Jones
2022-Feb-21 22:05 UTC
[Libguestfs] [nbdkit PATCH] swab: Implement .block_size callback
On Mon, Feb 21, 2022 at 04:00:11PM -0600, 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 > > =over 4 > @@ -90,6 +95,7 @@ L<nbdkit(1)>, > L<nbdkit-file-plugin(1)>, > L<nbdkit-pattern-plugin(1)>, > L<nbdkit-filter(3)>, > +L<nbdkit-blocksize-filter(1)>. > L<nbdkit-truncate-filter(1)>. > > =head1 AUTHORS > diff --git a/filters/swab/swab.c b/filters/swab/swab.c > index 68776eee..6e8dc981 100644 > --- a/filters/swab/swab.c > +++ b/filters/swab/swab.c > @@ -44,6 +44,7 @@ > #include "byte-swapping.h" > #include "isaligned.h" > #include "cleanup.h" > +#include "minmax.h" > #include "rounding.h" > > /* Can only be 8 (filter disabled), 16, 32 or 64. */ > @@ -85,8 +86,28 @@ swab_get_size (nbdkit_next *next, > return ROUND_DOWN (size, bits/8); > } > > +/* 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); > + } > + > + return 0; > +} > + > /* The request must be aligned. > - * XXX We could lift this restriction with more work. > + * If you want finer alignment, use the blocksize filter. > */ > static bool > is_aligned (uint32_t count, uint64_t offset, int *err) > @@ -220,6 +241,7 @@ static struct nbdkit_filter filter = { > .config = swab_config, > .config_help = swab_config_help, > .get_size = swab_get_size, > + .block_size = swab_block_size, > .pread = swab_pread, > .pwrite = swab_pwrite, > .trim = swab_trim,Looks sensible, ACK Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Laszlo Ersek
2022-Feb-22 11:49 UTC
[Libguestfs] [nbdkit PATCH] swab: Implement .block_size callback
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 PARAMETERSI 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.> > =over 4 > @@ -90,6 +95,7 @@ L<nbdkit(1)>, > L<nbdkit-file-plugin(1)>, > L<nbdkit-pattern-plugin(1)>, > L<nbdkit-filter(3)>, > +L<nbdkit-blocksize-filter(1)>. > L<nbdkit-truncate-filter(1)>. > > =head1 AUTHORS > diff --git a/filters/swab/swab.c b/filters/swab/swab.c > index 68776eee..6e8dc981 100644 > --- a/filters/swab/swab.c > +++ b/filters/swab/swab.c > @@ -44,6 +44,7 @@ > #include "byte-swapping.h" > #include "isaligned.h" > #include "cleanup.h" > +#include "minmax.h" > #include "rounding.h" > > /* Can only be 8 (filter disabled), 16, 32 or 64. */ > @@ -85,8 +86,28 @@ swab_get_size (nbdkit_next *next, > return ROUND_DOWN (size, bits/8); > } > > +/* 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? 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); Thanks, Laszlo> + > + return 0; > +} > + > /* The request must be aligned. > - * XXX We could lift this restriction with more work. > + * If you want finer alignment, use the blocksize filter. > */ > static bool > is_aligned (uint32_t count, uint64_t offset, int *err) > @@ -220,6 +241,7 @@ static struct nbdkit_filter filter = { > .config = swab_config, > .config_help = swab_config_help, > .get_size = swab_get_size, > + .block_size = swab_block_size, > .pread = swab_pread, > .pwrite = swab_pwrite, > .trim = swab_trim, >