[nbdkit PATCH 0/2] Avoid oddities with files unaligned to granularity
When using a filter that rounds up to alignment boundaries, the tail bytes of the plugin are difficult to access correctly. Rather than duplicating lots of code already in the truncate filter, it's easier to just make the other filters default to rounding down and add doc links on how to round up instead. Eric Blake (2): blocksize: Lift restriction against 0-size file cache, cow: Round size down filters/blocksize/nbdkit-blocksize-filter.pod | 15 +++++++++--- filters/cache/nbdkit-cache-filter.pod | 9 +++++++- filters/cow/nbdkit-cow-filter.pod | 8 +++++++ filters/truncate/nbdkit-truncate-filter.pod | 9 ++++++++ filters/blocksize/blocksize.c | 23 ++++--------------- filters/cache/cache.c | 8 +++++-- filters/cow/cow.c | 6 ++++- 7 files changed, 53 insertions(+), 25 deletions(-) -- 2.20.1
When the blocksize filter was first written, nbdkit claimed that 0-size files were not supported. Later, we added the zero plugin and ensured that a 0-size file works, so the restrictions against rounding down to zero size are no longer necessary. The next patch will document how to access the trailing bytes after all. While at it, utilize the ROUND_DOWN() macro rather than open-coding things, as that macro also post-dated the filter's initial implementation. Signed-off-by: Eric Blake <eblake@redhat.com> --- filters/blocksize/nbdkit-blocksize-filter.pod | 3 +-- filters/blocksize/blocksize.c | 23 ++++--------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/filters/blocksize/nbdkit-blocksize-filter.pod b/filters/blocksize/nbdkit-blocksize-filter.pod index 0abed2f..3b79e28 100644 --- a/filters/blocksize/nbdkit-blocksize-filter.pod +++ b/filters/blocksize/nbdkit-blocksize-filter.pod @@ -38,8 +38,7 @@ read requests to alignment boundaries, performs read-modify-write cycles for any unaligned head or tail of a write or zero request, and silently ignores any unaligned head or tail of a trim request. The filter also truncates the plugin size down to an aligned value (as it -cannot safely operate on the unaligned tail); it is an error if this -would result in a size of 0. +cannot safely operate on the unaligned tail). This parameter understands the suffix 'k' for 1024. diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c index 47bea96..ba5d9e7 100644 --- a/filters/blocksize/blocksize.c +++ b/filters/blocksize/blocksize.c @@ -138,30 +138,18 @@ blocksize_config_complete (nbdkit_next_config_complete *next, void *nxdata) "maxdata=<SIZE> Maximum size for read/write (default 64M).\n" \ "maxlen=<SIZE> Maximum size for trim/zero (default 4G-minblock)." -static int -blocksize_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, - void *handle) -{ - /* Early call to get_size to ensure it doesn't truncate to 0. */ - int64_t size = next_ops->get_size (nxdata); - - if (size == -1) - return -1; - if (size < minblock) { - nbdkit_error ("disk is too small for minblock size %u", minblock); - return -1; - } - /* TODO: cache per-connection FUA mode? */ - return 0; -} +/* TODO: Should we have a .prepare to cache per-connection FUA mode? */ +/* Round size down to avoid issues at end of file. */ static int64_t blocksize_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) { int64_t size = next_ops->get_size (nxdata); - return size == -1 ? size : size & ~(minblock - 1); + if (size == -1) + return -1; + return ROUND_DOWN (size, minblock); } static int @@ -387,7 +375,6 @@ static struct nbdkit_filter filter = { .config = blocksize_config, .config_complete = blocksize_config_complete, .config_help = blocksize_config_help, - .prepare = blocksize_prepare, .get_size = blocksize_get_size, .can_multi_conn = blocksize_can_multi_conn, .pread = blocksize_pread, -- 2.20.1
The blocksize filter already rounds sizes down, to avoid having to special-case a partial action at the end of an unaligned file. But our current behavior for cache and cow is not similarly careful, and can end up causing errors by passing a too-large count to the plugin. The bulk of our plugins are somewhat safe, in that they either fail the pread (for example, the file plugin gets a short read due to EOF) or populate the tail of the buffer with other data (which the client will never see, because we at least clamp user requests), so that the cache is only populated with untainted data. But it's hard to argue whether there might be another plugin out there that might abort because we violated our documented constraints, or which might leave the tail of the buffer unpopulated on a short read while still reporting success. Thus, I cannot rule out that our cache file might contain uninitialized data from the heap, or even leak it back out to the file system through the plugin or the NBD client. The quickest fix is to just always round down, and then do a better job of documenting how the user can round up as needed. Signed-off-by: Eric Blake <eblake@redhat.com> --- filters/blocksize/nbdkit-blocksize-filter.pod | 14 ++++++++++++-- filters/cache/nbdkit-cache-filter.pod | 9 ++++++++- filters/cow/nbdkit-cow-filter.pod | 8 ++++++++ filters/truncate/nbdkit-truncate-filter.pod | 9 +++++++++ filters/cache/cache.c | 8 ++++++-- filters/cow/cow.c | 6 +++++- 6 files changed, 48 insertions(+), 6 deletions(-) diff --git a/filters/blocksize/nbdkit-blocksize-filter.pod b/filters/blocksize/nbdkit-blocksize-filter.pod index 3b79e28..c684f7d 100644 --- a/filters/blocksize/nbdkit-blocksize-filter.pod +++ b/filters/blocksize/nbdkit-blocksize-filter.pod @@ -38,7 +38,9 @@ read requests to alignment boundaries, performs read-modify-write cycles for any unaligned head or tail of a write or zero request, and silently ignores any unaligned head or tail of a trim request. The filter also truncates the plugin size down to an aligned value (as it -cannot safely operate on the unaligned tail). +cannot safely operate on the unaligned tail); if you need access to +the last few bytes, you can combine the use of +L<nbdkit-truncate-filter(1)> to round the size up instead. This parameter understands the suffix 'k' for 1024. @@ -82,12 +84,20 @@ nbd plugin: nbdkit --filter=blocksize nbd maxdata=32M socket=/path/to/socket +Serve a file as if it were a block device that insists on 4k +alignment, while still allowing access to any unaligned bytes at the +end of the file: + + nbdkit --filter=blocksize --filter=truncate file /path/to/file \ + mindata=4k round-up=4k + =head1 SEE ALSO L<nbdkit(1)>, L<nbdkit-nbd-plugin(1)>, L<nbdkit-vddk-plugin(1)>, -L<nbdkit-filter(3)>. +L<nbdkit-filter(3)>, +L<nbdkit-truncate-filter(1)>. =head1 AUTHORS diff --git a/filters/cache/nbdkit-cache-filter.pod b/filters/cache/nbdkit-cache-filter.pod index 431a720..5993831 100644 --- a/filters/cache/nbdkit-cache-filter.pod +++ b/filters/cache/nbdkit-cache-filter.pod @@ -25,6 +25,12 @@ does not have effective caching, or (with C<cache=unsafe>) to defeat flush requests from the client (which is unsafe and can cause data loss, as the name suggests). +Note that the use of this filter rounds the image size down to a +multiple of the caching granularity (the larger of 4096 or the +C<f_bsize> field of L<fstatvfs(3)>), to ease the implementation. If +you need to round the image size up instead to access the last few +bytes, combine this filter with L<nbdkit-truncate-filter(1)>. + =head1 PARAMETERS =over 4 @@ -136,6 +142,7 @@ environment variable before starting nbdkit. L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-readahead-filter(1)>, +L<nbdkit-truncate-filter(1)>, L<nbdkit-filter(3)>, L<qemu-img(1)>. @@ -147,4 +154,4 @@ Richard W.M. Jones =head1 COPYRIGHT -Copyright (C) 2018 Red Hat Inc. +Copyright (C) 2018-2019 Red Hat Inc. diff --git a/filters/cow/nbdkit-cow-filter.pod b/filters/cow/nbdkit-cow-filter.pod index f88d8ee..448f48c 100644 --- a/filters/cow/nbdkit-cow-filter.pod +++ b/filters/cow/nbdkit-cow-filter.pod @@ -31,6 +31,13 @@ that away. It also allows us to create diffs, see below. The plugin is opened read-only (as if the I<-r> flag was passed), but you should B<not> pass the I<-r> flag to nbdkit. +=item * + +Use of this filter rounds the image size down to a multiple of the +caching granularity (4096), to ease the implementation. If you need to +round the image size up instead to access the last few bytes, combine +this filter with L<nbdkit-truncate-filter(1)>. + =back Limitations of the filter include: @@ -108,6 +115,7 @@ C<TMPDIR> environment variable before starting nbdkit. L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-xz-plugin(1)>, +L<nbdkit-truncate-filter(1)>, L<nbdkit-filter(3)>, L<qemu-img(1)>. diff --git a/filters/truncate/nbdkit-truncate-filter.pod b/filters/truncate/nbdkit-truncate-filter.pod index 9eba6cd..92645c0 100644 --- a/filters/truncate/nbdkit-truncate-filter.pod +++ b/filters/truncate/nbdkit-truncate-filter.pod @@ -59,6 +59,12 @@ Round the size up to the next multiple of C<N> bytes. If the size of the underlying plugin is already a multiple of C<N> bytes, this has no effect. +This option is useful when combined with other filters (such as +L<nbdkit-blocksize-filter(1)>, L<nbdkit-cache-filter(1)>, or +L<nbdkit-cow-filter(1)>) that normally round down due to a larger +granularity, in order to access the last few bytes of a file that +would otherwise be rendered inaccessible. + This parameter is optional. =item B<round-down=>N @@ -76,6 +82,9 @@ This parameter is optional. L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, +L<nbdkit-blocksize-filter(1)>, +L<nbdkit-cache-filter(1)>, +L<nbdkit-cow-filter(1)>, L<nbdkit-offset-filter(1)>, L<nbdkit-partition-filter(1)>. diff --git a/filters/cache/cache.c b/filters/cache/cache.c index 360458f..e215cac 100644 --- a/filters/cache/cache.c +++ b/filters/cache/cache.c @@ -60,6 +60,7 @@ #include "reclaim.h" #include "isaligned.h" #include "minmax.h" +#include "rounding.h" #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL @@ -188,10 +189,12 @@ cache_config_complete (nbdkit_next_config_complete *next, void *nxdata) return next (nxdata); } -/* Get the file size and ensure the cache is the correct size. */ +/* Get the file size; round it down to cache granularity before + * setting cache size. + */ static int64_t cache_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, - void *handle) + void *handle) { int64_t size; int r; @@ -201,6 +204,7 @@ cache_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, return -1; nbdkit_debug ("cache: underlying file size: %" PRIi64, size); + size = ROUND_DOWN (size, blksize); ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_set_size (size); diff --git a/filters/cow/cow.c b/filters/cow/cow.c index 1ce5893..aa1348b 100644 --- a/filters/cow/cow.c +++ b/filters/cow/cow.c @@ -49,6 +49,7 @@ #include "blk.h" #include "isaligned.h" #include "minmax.h" +#include "rounding.h" #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL @@ -80,7 +81,9 @@ cow_open (nbdkit_next_open *next, void *nxdata, int readonly) return NBDKIT_HANDLE_NOT_NEEDED; } -/* Get the file size and ensure the overlay is the correct size. */ +/* Get the file size; round it down to overlay granularity before + * setting overlay size. + */ static int64_t cow_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) @@ -93,6 +96,7 @@ cow_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, return -1; nbdkit_debug ("cow: underlying file size: %" PRIi64, size); + size = ROUND_DOWN (size, BLKSIZE); ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_set_size (size); -- 2.20.1
ACK series. 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
participants (2)
-
Eric Blake -
Richard W.M. Jones