Re: [Libguestfs] [PATCH] file: Better support for read-only files

On Tue, Mar 2, 2021 at 4:45 PM Eric Blake <eblake(a)redhat.com&gt; 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?