[Libguestfs] [PATCH] file: Better support for read-only files
Nir Soffer
nsoffer at redhat.com
Tue Mar 2 17:21:06 UTC 2021
On Tue, Mar 2, 2021 at 4:45 PM Eric Blake <eblake at redhat.com> wrote:
>
> Previously, attempts to use nbdkit without -r in order to visit a
> read-only image failed to open the image (in fact, a single read-only
> file in an otherwise usable directory makes "nbdkit file dir=. --run
> 'nbdinfo --list'" fail to print anything in libnbd 1.6). But a saner
> approach is to try a fallback to a readonly fd if a read-write fd
> fails.
So this is mainly a convenience if the user forgot -r, right?
> The windows code compiles, but I wasn't able to test it as thoroughly
> as the Unix code.
> ---
> plugins/file/file.c | 30 ++++++++++++++++--
> plugins/file/winfile.c | 21 +++++++++++--
> tests/Makefile.am | 12 +++++--
> tests/test-file-readonly.sh | 63 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 118 insertions(+), 8 deletions(-)
> create mode 100755 tests/test-file-readonly.sh
>
> diff --git a/plugins/file/file.c b/plugins/file/file.c
> index 1651079a..a9ecceb6 100644
> --- a/plugins/file/file.c
> +++ b/plugins/file/file.c
> @@ -1,5 +1,5 @@
> /* nbdkit
> - * Copyright (C) 2013-2020 Red Hat Inc.
> + * Copyright (C) 2013-2021 Red Hat Inc.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions are
> @@ -304,6 +304,7 @@ struct handle {
> int fd;
> bool is_block_device;
> int sector_size;
> + bool can_write;
> bool can_punch_hole;
> bool can_zero_range;
> bool can_fallocate;
> @@ -343,12 +344,25 @@ file_open (int readonly)
> }
>
> flags = O_CLOEXEC|O_NOCTTY;
> - if (readonly)
> + if (readonly) {
> flags |= O_RDONLY;
> - else
> + h->can_write = false;
> + }
> + else {
> flags |= O_RDWR;
> + h->can_write = true;
> + }
>
> h->fd = openat (dfd, file, flags);
> + if (h->fd == -1 && !readonly) {
> + int saved_errno = errno;
> + flags = (flags & ~O_ACCMODE) | O_RDONLY;
> + h->fd = openat (dfd, file, flags);
> + if (h->fd == -1)
> + errno = saved_errno;
> + else
> + h->can_write = false;
> + }
> if (h->fd == -1) {
> nbdkit_error ("open: %s: %m", file);
This will always report the error that failed open when using O_RDWR.
But if open(O_RDWR) fail with EPERM, and open(O_RDONLY) fails
with another error, the other error will be hidden.
Maybe it is better to log the first error so we don't need to save errno
when trying to open in read only mode?
> if (dfd != -1)
> @@ -464,6 +478,15 @@ file_get_size (void *handle)
> }
> }
>
> +/* Check if file is read-only. */
> +static int
> +file_can_write (void *handle)
> +{
> + struct handle *h = handle;
> +
> + return h->can_write;
> +}
> +
> /* Allow multiple parallel connections from a single client. */
> static int
> file_can_multi_conn (void *handle)
> @@ -880,6 +903,7 @@ static struct nbdkit_plugin plugin = {
> .open = file_open,
> .close = file_close,
> .get_size = file_get_size,
> + .can_write = file_can_write,
> .can_multi_conn = file_can_multi_conn,
> .can_trim = file_can_trim,
> .can_fua = file_can_fua,
> diff --git a/plugins/file/winfile.c b/plugins/file/winfile.c
> index 6d1a6643..ec186255 100644
> --- a/plugins/file/winfile.c
> +++ b/plugins/file/winfile.c
> @@ -1,5 +1,5 @@
> /* nbdkit
> - * Copyright (C) 2013-2020 Red Hat Inc.
> + * Copyright (C) 2013-2021 Red Hat Inc.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions are
> @@ -106,6 +106,7 @@ winfile_dump_plugin (void)
> struct handle {
> HANDLE fh;
> int64_t size;
> + bool is_readonly;
> bool is_volume;
> bool is_sparse;
> };
> @@ -126,6 +127,12 @@ winfile_open (int readonly)
>
> fh = CreateFile (filename, flags, FILE_SHARE_READ|FILE_SHARE_WRITE,
> NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (fh == INVALID_HANDLE_VALUE && !readonly) {
> + flags &= ~GENERIC_WRITE;
> + readonly = true;
> + fh = CreateFile (filename, flags, FILE_SHARE_READ|FILE_SHARE_WRITE,
> + NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + }
> if (fh == INVALID_HANDLE_VALUE) {
> nbdkit_error ("%s: error %lu", filename, GetLastError ());
> return NULL;
> @@ -179,15 +186,24 @@ winfile_open (int readonly)
> }
> h->fh = fh;
> h->size = size.QuadPart;
> + h->is_readonly = readonly;
> h->is_volume = is_volume;
> h->is_sparse = is_sparse;
> - nbdkit_debug ("%s: size=%" PRIi64 " is_volume=%s is_sparse=%s",
> + nbdkit_debug ("%s: size=%" PRIi64 " readonly=%s is_volume=%s is_sparse=%s",
> filename, h->size,
> + readonly ? "true" : "false",
> is_volume ? "true" : "false",
> is_sparse ? "true" : "false");
> return h;
> }
>
> +static int
> +wintfile_can_write (void *handle)
> +{
> + struct handle *h = handle;
> + return !h->is_readonly;
> +}
> +
> static int
> winfile_can_trim (void *handle)
> {
> @@ -425,6 +441,7 @@ static struct nbdkit_plugin plugin = {
> .dump_plugin = winfile_dump_plugin,
>
> .open = winfile_open,
> + .can_write = winfile_can_write,
> .can_trim = winfile_can_trim,
> .can_zero = winfile_can_zero,
> .can_extents = winfile_can_extents,
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 70898f20..e8c9c535 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -1,5 +1,5 @@
> # nbdkit
> -# Copyright (C) 2013-2020 Red Hat Inc.
> +# Copyright (C) 2013-2021 Red Hat Inc.
> #
> # Redistribution and use in source and binary forms, with or without
> # modification, are permitted provided that the following conditions are
> @@ -673,8 +673,14 @@ EXTRA_DIST += \
> $(NULL)
>
> # file plugin test.
> -TESTS += test-file.sh
> -EXTRA_DIST += test-file.sh
> +TESTS += \
> + test-file.sh \
> + test-file-readonly.sh \
> + $(NULL)
> +EXTRA_DIST += \
> + test-file.sh \
> + test-file-readonly.sh \
> + $(NULL)
> LIBGUESTFS_TESTS += test-file-block
>
> test_file_block_SOURCES = test-file-block.c test.h
> diff --git a/tests/test-file-readonly.sh b/tests/test-file-readonly.sh
> new file mode 100755
> index 00000000..cde2bd65
> --- /dev/null
> +++ b/tests/test-file-readonly.sh
> @@ -0,0 +1,63 @@
> +#!/usr/bin/env bash
> +# nbdkit
> +# Copyright (C) 2013-2021 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 file plugin on readonly files.
> +
> +source ./functions.sh
> +set -e
> +set -x
> +
> +requires_plugin file
> +requires_nbdsh_uri
> +requires truncate --version
> +
> +sock=$(mktemp -u /tmp/nbdkit-test-sock.XXXXXX)
> +files="file-readonly.pid file-readonly.img $sock"
> +rm -f $files
> +cleanup_fn rm -f $files
> +
> +truncate -s 16384 file-readonly.img
> +chmod a-w file-readonly.img
> +start_nbdkit -P file-readonly.pid -U $sock file file-readonly.img
> +
> +# Try to test all the major functions supported by both
> +# the Unix and Windows versions of the file plugin.
> +nbdsh -u "nbd+unix://?socket=$sock" -c '
> +assert h.is_read_only()
> +assert not h.can_zero()
> +
> +buf = h.pread(8192, 0)
> +assert buf == b"\0" * 8192
> +
> +if h.can_flush():
> + h.flush()
> +'
> --
> 2.30.1
This can make it easier to use, but on the other hand it complicates
the code and error handling when a user could use --readdonly, so
I'm not sure about this.
Nir
More information about the Libguestfs
mailing list