Eric Blake
2020-Feb-10 21:43 UTC
[Libguestfs] [nbdkit PATCH 05/10] plugins: Wire up file-based plugin support for NBD_INFO_INIT_STATE
The NBD protocol is adding an extension to let servers advertise initialization state to the client: whether the image contains holes, and whether it is known to read as all zeroes. For file-based plugins, we are already probing lseek(SEEK_HOLE) to learn if extents are supported; a slight tweak to remember if that result is EOF tells us if we are sparse, and a similar lseek(SEEK_DATA) returning ENXIO tells us if the entire file is a hole, with at most two lseek calls during open (rather than one lseek for each of .can_extents, .init_sparse, and .init_zero). The split plugin is similar to file. For partitioning and linuxdisk, we know we are exposing a sparse image. For other file-based plugins, like ext2, we do not yet expose .extents so it is not worth trying to expose .init_sparse or .init_zero. A successful test depends on whether the current file system creates sparse files, but it was easy enough to ensure the test skips rather than fails when that is not the case. Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/file/file.c | 82 ++++++++++++++++++++++++----- plugins/linuxdisk/linuxdisk.c | 13 ++++- plugins/partitioning/partitioning.c | 13 ++++- plugins/split/split.c | 59 +++++++++++++++++++-- tests/Makefile.am | 3 +- tests/test-file-init.sh | 69 ++++++++++++++++++++++++ 6 files changed, 219 insertions(+), 20 deletions(-) create mode 100755 tests/test-file-init.sh diff --git a/plugins/file/file.c b/plugins/file/file.c index 8c2ea07..ca7e2d9 100644 --- a/plugins/file/file.c +++ b/plugins/file/file.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -154,6 +154,9 @@ struct handle { bool can_zero_range; bool can_fallocate; bool can_zeroout; + bool can_extents; + bool init_sparse; + bool init_zero; }; /* Create the per-connection handle. */ @@ -214,6 +217,52 @@ file_open (int readonly) h->can_fallocate = true; h->can_zeroout = h->is_block_device; + h->can_extents = false; + h->init_sparse = false; + h->init_zero = false; +#ifdef SEEK_HOLE + if (!h->is_block_device) { + off_t r; + + /* A simple test to see whether SEEK_DATA/SEEK_HOLE are likely to work on + * the current filesystem, and to see if the image is sparse or zero. + */ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock); + r = lseek (h->fd, 0, SEEK_DATA); + if (r == -1) { + if (errno == ENXIO) { + nbdkit_debug ("extents enabled, entire image is hole"); + h->can_extents = true; + h->init_sparse = true; + h->init_zero = true; + } else { + nbdkit_debug ("extents disabled: lseek(SEEK_DATA): %m"); + } + } + else { + h->can_extents = true; + if (r > 0) { + nbdkit_debug ("extents enabled, image includes hole before data"); + h->init_sparse = true; + } + else { + r = lseek (h->fd, 0, SEEK_HOLE); + if (r == -1) { + nbdkit_debug ("extents disabled: lseek(SEEK_HOLE): %m"); + h->can_extents = false; + } + else if (r == statbuf.st_size) { + nbdkit_debug ("extents enabled, image currently all data"); + } + else { + nbdkit_debug ("extents enabled, image includes data before hole"); + h->init_sparse = true; + } + } + } + } +#endif + return h; } @@ -540,18 +589,8 @@ static int file_can_extents (void *handle) { struct handle *h = handle; - off_t r; - /* A simple test to see whether SEEK_HOLE etc is likely to work on - * the current filesystem. - */ - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock); - r = lseek (h->fd, 0, SEEK_HOLE); - if (r == -1) { - nbdkit_debug ("extents disabled: lseek: SEEK_HOLE: %m"); - return 0; - } - return 1; + return h->can_extents; } static int @@ -622,6 +661,23 @@ file_extents (void *handle, uint32_t count, uint64_t offset, ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock); return do_extents (handle, count, offset, flags, extents); } + +/* Initial state. */ +static int +file_init_sparse (void *handle) +{ + struct handle *h = handle; + + return h->init_sparse; +} + +static int +file_init_zero (void *handle) +{ + struct handle *h = handle; + + return h->init_zero; +} #endif /* SEEK_HOLE */ #if HAVE_POSIX_FADVISE @@ -668,6 +724,8 @@ static struct nbdkit_plugin plugin = { #ifdef SEEK_HOLE .can_extents = file_can_extents, .extents = file_extents, + .init_sparse = file_init_sparse, + .init_zero = file_init_zero, #endif #if HAVE_POSIX_FADVISE .cache = file_cache, diff --git a/plugins/linuxdisk/linuxdisk.c b/plugins/linuxdisk/linuxdisk.c index 99dbc99..0907302 100644 --- a/plugins/linuxdisk/linuxdisk.c +++ b/plugins/linuxdisk/linuxdisk.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2019 Red Hat Inc. + * Copyright (C) 2019-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -167,6 +167,16 @@ linuxdisk_can_cache (void *handle) return NBDKIT_CACHE_EMULATE; } +/* Initial state. */ +static int +linuxdisk_init_sparse (void *handle) +{ + /* region_zero regions mean we are sparse, even if we don't yet + * support .extents + */ + return 1; +} + /* Read data from the virtual disk. */ static int linuxdisk_pread (void *handle, void *buf, uint32_t count, uint64_t offset, @@ -230,6 +240,7 @@ static struct nbdkit_plugin plugin = { .get_size = linuxdisk_get_size, .can_multi_conn = linuxdisk_can_multi_conn, .can_cache = linuxdisk_can_cache, + .init_sparse = linuxdisk_init_sparse, .pread = linuxdisk_pread, .errno_is_preserved = 1, }; diff --git a/plugins/partitioning/partitioning.c b/plugins/partitioning/partitioning.c index 6e426b9..0b42b2e 100644 --- a/plugins/partitioning/partitioning.c +++ b/plugins/partitioning/partitioning.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018-2019 Red Hat Inc. + * Copyright (C) 2018-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -307,6 +307,16 @@ partitioning_can_cache (void *handle) return NBDKIT_CACHE_EMULATE; } +/* Initial state. */ +static int +partitioning_init_sparse (void *handle) +{ + /* region_zero regions mean we are sparse, even if we don't yet + * support .extents + */ + return 1; +} + /* Read data. */ static int partitioning_pread (void *handle, void *buf, uint32_t count, uint64_t offset) @@ -437,6 +447,7 @@ static struct nbdkit_plugin plugin = { .get_size = partitioning_get_size, .can_multi_conn = partitioning_can_multi_conn, .can_cache = partitioning_can_cache, + .init_sparse = partitioning_init_sparse, .pread = partitioning_pread, .pwrite = partitioning_pwrite, .flush = partitioning_flush, diff --git a/plugins/split/split.c b/plugins/split/split.c index 70fd4d4..ebb5f5a 100644 --- a/plugins/split/split.c +++ b/plugins/split/split.c @@ -97,6 +97,8 @@ split_config (const char *key, const char *value) struct handle { struct file *files; uint64_t size; /* Total concatenated size. */ + bool init_sparse; + bool init_zero; }; struct file { @@ -147,6 +149,8 @@ split_open (int readonly) } offset = 0; + h->init_sparse = false; /* set later as needed */ + h->init_zero = true; /* cleared later as needed */ for (i = 0; i < nr_files; ++i) { h->files[i].offset = offset; @@ -161,17 +165,44 @@ split_open (int readonly) i, filenames[i], h->files[i].offset, h->files[i].size); #ifdef SEEK_HOLE - /* Test if this file supports extents. */ + /* Test if this file supports extents, and for initial state. */ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock); r = lseek (h->files[i].fd, 0, SEEK_DATA); - if (r == -1 && errno != ENXIO) { - nbdkit_debug ("disabling extents: lseek on %s: %m", filenames[i]); - h->files[i].can_extents = false; + if (r == -1) { + if (errno == ENXIO) { + /* Entire file is hole */ + h->files[i].can_extents = true; + h->init_sparse = true; + } + else { + nbdkit_debug ("disabling extents: lseek(SEEK_DATA) on %s: %m", + filenames[i]); + h->files[i].can_extents = false; + h->init_zero = false; + } } - else + else { h->files[i].can_extents = true; + h->init_zero = false; + if (r > 0) + /* File includes hole before data */ + h->init_sparse = true; + else { + r = lseek (h->files[i].fd, 0, SEEK_HOLE); + if (r == -1) { + nbdkit_debug ("disabling extents: lseek(SEEK_HOLE) on %s: %m", + filenames[i]); + h->files[i].can_extents = false; + } + else if (r < statbuf.st_size) { + /* File includes data before hole */ + h->init_sparse = true; + } + } + } #else h->files[i].can_extents = false; + h->init_zero = false; #endif } h->size = offset; @@ -227,6 +258,22 @@ split_can_cache (void *handle) #endif } +static int +split_init_sparse (void *handle) +{ + struct handle *h = handle; + + return h->init_sparse; +} + +static int +split_init_zero (void *handle) +{ + struct handle *h = handle; + + return h->init_zero; +} + /* Helper function to map the offset to the correct file. */ static int compare_offset (const void *offsetp, const void *filep) @@ -450,6 +497,8 @@ static struct nbdkit_plugin plugin = { .close = split_close, .get_size = split_get_size, .can_cache = split_can_cache, + .init_sparse = split_init_sparse, + .init_zero = split_init_zero, .pread = split_pread, .pwrite = split_pwrite, #if HAVE_POSIX_FADVISE diff --git a/tests/Makefile.am b/tests/Makefile.am index 4e036a3..a8ff601 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -113,6 +113,7 @@ EXTRA_DIST = \ test-export-name.sh \ test-extentlist.sh \ test-file-extents.sh \ + test-file-init.sh \ test-floppy.sh \ test-foreground.sh \ test-fua.sh \ @@ -554,7 +555,7 @@ test_file_block_SOURCES = test-file-block.c test.h test_file_block_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS) test_file_block_LDADD = libtest.la $(LIBGUESTFS_LIBS) -TESTS += test-file-extents.sh +TESTS += test-file-extents.sh test-file-init.sh # floppy plugin test. TESTS += test-floppy.sh diff --git a/tests/test-file-init.sh b/tests/test-file-init.sh new file mode 100755 index 0000000..c66ac65 --- /dev/null +++ b/tests/test-file-init.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2020 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +# Test the initial state of the file plugin. + +source ./functions.sh +set -e + +requires nbdsh -c 'exit (not hasattr (h, "get_init_flags"))' +requires truncate --help +requires stat --help + +sock=`mktemp -u` +files="file-init.pid $sock file-init.out" +rm -f $files +cleanup_fn rm -f $files + +# See if the current file system creates sparse files +truncate --size=1M file-init.out +if test "$(stat -c %b file-init.out)" != 0; then + echo "$0: unable to create sparse file, skipping this test" + exit 77 +fi + +# Run nbdkit with the file plugin. +start_nbdkit -P file-init.pid -U $sock file file-init.out + +# The image should start as sparse/zero, before we write to it +nbdsh --connect "nbd+unix://?socket=$sock" \ + -c 'assert (h.get_init_flags () == nbd.INIT_SPARSE | nbd.INIT_ZERO)' \ + -c 'h.pwrite ("hello world".encode (), 0)' + +# The image is still sparse, but no longer zero, before we fill it +nbdsh --connect "nbd+unix://?socket=$sock" \ + -c 'assert (h.get_init_flags () == nbd.INIT_SPARSE)' \ + -c 'h.pwrite (("1" * (1024*1024)).encode (), 0)' + +# The image is neither sparse nor zero +nbdsh --connect "nbd+unix://?socket=$sock" \ + -c 'assert (h.get_init_flags () == 0)' -- 2.24.1
Richard W.M. Jones
2020-Feb-11 10:52 UTC
Re: [Libguestfs] [nbdkit PATCH 05/10] plugins: Wire up file-based plugin support for NBD_INFO_INIT_STATE
On Mon, Feb 10, 2020 at 03:43:58PM -0600, Eric Blake wrote:> @@ -214,6 +217,52 @@ file_open (int readonly) > h->can_fallocate = true; > h->can_zeroout = h->is_block_device; > > + h->can_extents = false; > + h->init_sparse = false; > + h->init_zero = false; > +#ifdef SEEK_HOLE > + if (!h->is_block_device) { > + off_t r; > + > + /* A simple test to see whether SEEK_DATA/SEEK_HOLE are likely to work on > + * the current filesystem, and to see if the image is sparse or zero. > + */ > + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock); > + r = lseek (h->fd, 0, SEEK_DATA); > + if (r == -1) { > + if (errno == ENXIO) { > + nbdkit_debug ("extents enabled, entire image is hole"); > + h->can_extents = true; > + h->init_sparse = true; > + h->init_zero = true; > + } else { > + nbdkit_debug ("extents disabled: lseek(SEEK_DATA): %m"); > + } > + } > + else { > + h->can_extents = true; > + if (r > 0) { > + nbdkit_debug ("extents enabled, image includes hole before data"); > + h->init_sparse = true; > + } > + else { > + r = lseek (h->fd, 0, SEEK_HOLE); > + if (r == -1) { > + nbdkit_debug ("extents disabled: lseek(SEEK_HOLE): %m"); > + h->can_extents = false; > + } > + else if (r == statbuf.st_size) { > + nbdkit_debug ("extents enabled, image currently all data"); > + } > + else { > + nbdkit_debug ("extents enabled, image includes data before hole"); > + h->init_sparse = true; > + } > + } > + } > + }In the non-block case, can't we stat(2) the file and look at statbuf.st_blocks?> +requires nbdsh -c 'exit (not hasattr (h, "get_init_flags"))'Not wrong, but it's easier to do it like this: github.com/libguestfs/nbdkit/blob/5e4745641bb4676f607fdb3f8750dbf6e9516877/tests/test-vsock.sh#L48 Rich. -- Richard Jones, Virtualization Group, Red Hat people.redhat.com/~rjones Read my programming and virtualization blog: rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. libguestfs.org/virt-v2v
Eric Blake
2020-Feb-11 14:24 UTC
Re: [Libguestfs] [nbdkit PATCH 05/10] plugins: Wire up file-based plugin support for NBD_INFO_INIT_STATE
On 2/11/20 4:52 AM, Richard W.M. Jones wrote:> On Mon, Feb 10, 2020 at 03:43:58PM -0600, Eric Blake wrote: >> @@ -214,6 +217,52 @@ file_open (int readonly) >> h->can_fallocate = true; >> h->can_zeroout = h->is_block_device; >> >> + h->can_extents = false; >> + h->init_sparse = false; >> + h->init_zero = false; >> +#ifdef SEEK_HOLE>> + else if (r == statbuf.st_size) { >> + nbdkit_debug ("extents enabled, image currently all data"); >> + } >> + else { >> + nbdkit_debug ("extents enabled, image includes data before hole"); >> + h->init_sparse = true; >> + } >> + } >> + } >> + } > > In the non-block case, can't we stat(2) the file and look at statbuf.st_blocks?True. In fact, we already did do fstat(), so the information is already there. Note that st_blocks == 0 proves a file is sparse. But POSIX says st_block is not necessarily using the same units as st_blksize, so checking st_block * st_blksize < st_size is NOT necessarily evidence that a file is sparse. What's worse, there are some file systems where first- and second-level indirect blocks are used for large files, and where those indirects are counted towards st_block (but NOT towards st_size), so it is possible to have a sparse file where st_block * st_blksize > st_size but the file is still sparse. In short, proving that a file has holes is easy with lseek(SEEK_HOLE), but only a best-guess heuristic with stat() data. That said, you're right that it is at least worth adding the heuristics in the !SEEK_HOLE case.> >> +requires nbdsh -c 'exit (not hasattr (h, "get_init_flags"))' > > Not wrong, but it's easier to do it like this: > > github.com/libguestfs/nbdkit/blob/5e4745641bb4676f607fdb3f8750dbf6e9516877/tests/test-vsock.sh#L48Thanks, that is cleaner. I will update it (along with the updates required by renaming it to two "is_init_*" functions) (affects several patches in this series). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Possibly Parallel Threads
- [nbdkit PATCH 05/10] plugins: Wire up file-based plugin support for NBD_INFO_INIT_STATE
- [nbdkit PATCH 04/10] plugins: Wire up in-memory plugin support for NBD_INFO_INIT_STATE
- [nbdkit PATCH 03/10] filters: Wire up filter support for NBD_INFO_INIT_STATE
- Re: Cross-project NBD extension proposal: NBD_INFO_INIT_STATE
- [nbdkit PATCH 10/10] plugins: Wire up nbd plugin support for NBD_INFO_INIT_STATE