rpms/nbdkit
6
0
Fork 0
Code Issues Pull Requests Releases Activity

Log filename, offset and count in nbdkit-file-plugin error messages

Browse Source 
resolves: RHEL-95364
This commit is contained in:
Richard W.M. Jones 2025-06-05 10:24:35 +01:00
parent 8430edda07
commit 06e4722074
4 changed files with 545 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

263
0024-file-Save-the-filename-or-equivalent-in-the-file-han.patch Normal file
View File

@ -0,0 +1,263 @@
From 0f2fc18c423dc3f5aef5a2d38c918905b4fbc6ef Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 4 Jun 2025 21:00:47 +0100
Subject: [PATCH] file: Save the filename or equivalent in the file handle
This will allow us to improve error messages.
To make this change easier by reducing duplication, I also implemented
a common error path in the file_open function.
(cherry picked from commit 83ad77d28d598e4b85809e7225e754e64314f802)
---
plugins/file/file.c | 114 +++++++++++++++++++++++++-------------------
1 file changed, 65 insertions(+), 49 deletions(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index dc100bb3..4dec4bd9 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -500,6 +500,7 @@ file_list_exports (int readonly, int default_only,
/* The per-connection handle. */
struct handle {
int fd;
+ char *name; /* Filename or equivalent. */
struct stat statbuf;
bool is_block_device;
int sector_size;
@@ -557,7 +558,6 @@ static void *
file_open (int readonly)
{
struct handle *h;
- const char *file;
h = malloc (sizeof *h);
if (h == NULL) {
@@ -566,36 +566,44 @@ file_open (int readonly)
}
h->can_write = !readonly;
h->fd = -1;
+ h->name = NULL;
switch (mode) {
case mode_filename:
- file = filename;
- if (open_file_by_name (h, readonly, -1, file) == -1) {
- free (h);
- return NULL;
+ h->name = strdup (filename);
+ if (h->name == NULL) {
+ nbdkit_error ("strdup: %m");
+ goto err;
}
+ if (open_file_by_name (h, readonly, -1, h->name) == -1)
+ goto err;
break;
case mode_directory: {
int dfd;
+ const char *export_name = nbdkit_export_name ();
- file = nbdkit_export_name ();
- if (strchr (file, '/')) {
- nbdkit_error ("exportname cannot contain /");
- free (h);
+ if (export_name == NULL)
+ goto err;
+
+ h->name = strdup (export_name);
+ if (h->name == NULL) {
+ nbdkit_error ("strdup: %m");
+ goto err;
+ }
+ if (strchr (h->name, '/')) {
+ nbdkit_error ("%s: exportname cannot contain /", h->name);
errno = EINVAL;
- return NULL;
+ goto err;
}
dfd = open (directory, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
if (dfd == -1) {
nbdkit_error ("open %s: %m", directory);
- free (h);
- return NULL;
+ goto err;
}
- if (open_file_by_name (h, readonly, dfd, file) == -1) {
- free (h);
+ if (open_file_by_name (h, readonly, dfd, h->name) == -1) {
close (dfd);
- return NULL;
+ goto err;
}
close (dfd);
break;
@@ -604,14 +612,14 @@ file_open (int readonly)
case mode_fd: {
int r;
- /* This is needed for error messages. */
- file = "<file descriptor>";
-
+ if (asprintf (&h->name, "fd=%d", filedesc) == -1) {
+ nbdkit_error ("asprintf: %m");
+ goto err;
+ }
h->fd = dup (filedesc);
if (h->fd == -1) {
- nbdkit_error ("dup fd=%d: %m", filedesc);
- free (h);
- return NULL;
+ nbdkit_error ("dup: %s: %m", h->name);
+ goto err;
}
/* If the file descriptor is readonly then we should not advertise
@@ -619,10 +627,8 @@ file_open (int readonly)
*/
r = fcntl (h->fd, F_GETFL);
if (r == -1) {
- nbdkit_error ("fcntl: F_GETFL: %m");
- close (h->fd);
- free (h);
- return NULL;
+ nbdkit_error ("fcntl: %s: F_GETFL: %m", h->name);
+ goto err;
}
r &= O_ACCMODE;
if (r == O_RDONLY)
@@ -636,25 +642,30 @@ file_open (int readonly)
case mode_dirfd: {
int dfd;
+ const char *export_name = nbdkit_export_name ();
- file = nbdkit_export_name ();
- if (strchr (file, '/')) {
- nbdkit_error ("exportname cannot contain /");
- free (h);
+ if (export_name == NULL)
+ goto err;
+
+ h->name = strdup (export_name);
+ if (h->name == NULL) {
+ nbdkit_error ("strdup: %m");
+ goto err;
+ }
+ if (strchr (h->name, '/')) {
+ nbdkit_error ("%s: exportname cannot contain /", h->name);
errno = EINVAL;
- return NULL;
+ goto err;
}
/* We don't fork, so no need to worry about FD_CLOEXEC on the directory */
dfd = dup (filedesc);
if (dfd == -1) {
- nbdkit_error ("dup dirfd=%d: %m", filedesc);
- free (h);
- return NULL;
+ nbdkit_error ("dup: dirfd=%d: %m", filedesc);
+ goto err;
}
- if (open_file_by_name (h, readonly, dfd, file) == -1) {
- free (h);
+ if (open_file_by_name (h, readonly, dfd, h->name) == -1) {
close (dfd);
- return NULL;
+ goto err;
}
close (dfd);
break;
@@ -665,12 +676,11 @@ file_open (int readonly)
}
assert (h->fd >= 0);
+ assert (h->name != NULL);
if (fstat (h->fd, &h->statbuf) == -1) {
- nbdkit_error ("fstat: %s: %m", file);
- close (h->fd);
- free (h);
- return NULL;
+ nbdkit_error ("fstat: %s: %m", h->name);
+ goto err;
}
if (fadvise_mode != -1) {
@@ -678,7 +688,7 @@ file_open (int readonly)
#ifdef HAVE_POSIX_FADVISE
int r = posix_fadvise (h->fd, 0, 0, fadvise_mode);
if (r == -1)
- nbdkit_debug ("posix_fadvise: %s: %m (ignored)", file);
+ nbdkit_debug ("posix_fadvise: %s: %m (ignored)", h->name);
#else
nbdkit_debug ("fadvise is not supported");
#endif
@@ -689,17 +699,15 @@ file_open (int readonly)
else if (S_ISREG (h->statbuf.st_mode))
h->is_block_device = false;
else {
- nbdkit_error ("file is not regular or block device: %s", file);
- close (h->fd);
- free (h);
- return NULL;
+ nbdkit_error ("file is not regular or block device: %s", h->name);
+ goto err;
}
h->sector_size = 4096; /* Start with safe guess */
#ifdef BLKSSZGET
if (h->is_block_device) {
if (ioctl (h->fd, BLKSSZGET, &h->sector_size) == -1)
- nbdkit_debug ("cannot get sector size: %s: %m", file);
+ nbdkit_debug ("cannot get sector size: %s: %m", h->name);
}
#endif
@@ -707,7 +715,7 @@ file_open (int readonly)
#ifdef BLKROTATIONAL
if (h->is_block_device) {
if (ioctl (h->fd, BLKROTATIONAL, &h->rotational) == -1)
- nbdkit_debug ("cannot get rotational property: %s: %m", file);
+ nbdkit_debug ("cannot get rotational property: %s: %m", h->name);
}
#endif
@@ -717,10 +725,10 @@ file_open (int readonly)
unsigned int minimum_io_size = 0, optimal_io_size = 0;
if (ioctl (h->fd, BLKIOMIN, &minimum_io_size) == -1)
- nbdkit_debug ("cannot get BLKIOMIN: %s: %m", file);
+ nbdkit_debug ("cannot get BLKIOMIN: %s: %m", h->name);
if (ioctl (h->fd, BLKIOOPT, &optimal_io_size) == -1)
- nbdkit_debug ("cannot get BLKIOOPT: %s: %m", file);
+ nbdkit_debug ("cannot get BLKIOOPT: %s: %m", h->name);
else if (optimal_io_size == 0)
/* All devices in the Linux kernel except for MD report optimal
* as 0. In that case guess a good value.
@@ -754,6 +762,13 @@ file_open (int readonly)
h->can_blkdiscard = h->is_block_device;
return h;
+
+ err:
+ if (h->fd >= 0)
+ close (h->fd);
+ free (h->name);
+ free (h);
+ return NULL;
}
/* Free up the per-connection handle. */
@@ -766,6 +781,7 @@ file_close (void *handle)
remove_fd_from_window (h->fd);
#endif
close (h->fd);
+ free (h->name);
free (h);
}
--
2.47.1

143
0025-file-Add-the-filename-or-equivalent-to-error-message.patch Normal file
View File

@ -0,0 +1,143 @@
From 53f5fa4e1f560475dadbefc1f00e0f3d2ca84ca0 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 4 Jun 2025 21:03:58 +0100
Subject: [PATCH] file: Add the filename (or equivalent) to error messages
It helps to have this information when there's an error. It
particularly helps in two cases: (1) You don't have access to the
nbdkit command line. (2) The filename is derived from an export name
passed by the client.
(cherry picked from commit 7273bd829fc0d8e492786f8908d8ddc3d5399e06)
---
plugins/file/file.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 4dec4bd9..8018b294 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -797,7 +797,7 @@ file_get_size (void *handle)
r = device_size (h->fd, &h->statbuf);
if (r == -1) {
- nbdkit_error ("device_size: %m");
+ nbdkit_error ("device_size: %s: %m", h->name);
return -1;
}
return r;
@@ -883,7 +883,7 @@ file_flush (void *handle, uint32_t flags)
struct handle *h = handle;
if (fdatasync (h->fd) == -1) {
- nbdkit_error ("fdatasync: %m");
+ nbdkit_error ("fdatasync: %s: %m", h->name);
return -1;
}
@@ -904,11 +904,11 @@ file_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
ssize_t r = pread (h->fd, buf, count, offset);
if (r == -1) {
- nbdkit_error ("pread: %m");
+ nbdkit_error ("pread: %s: %m", h->name);
return -1;
}
if (r == 0) {
- nbdkit_error ("pread: unexpected end of file");
+ nbdkit_error ("pread: %s: unexpected end of file", h->name);
return -1;
}
buf += r;
@@ -940,7 +940,7 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
ssize_t r = pwrite (h->fd, buf, count, offset);
if (r == -1) {
- nbdkit_error ("pwrite: %m");
+ nbdkit_error ("pwrite: %s: %m", h->name);
return -1;
}
buf += r;
@@ -1022,7 +1022,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1058,7 +1058,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1066,7 +1066,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
else {
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1088,7 +1088,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1116,14 +1116,14 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
h->can_fallocate = false;
} else {
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1147,7 +1147,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (errno != ENOTTY) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1185,7 +1185,7 @@ file_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
/* Trim is advisory; we don't care if it fails for anything other
* than EIO or EPERM. */
if (errno == EPERM || errno == EIO) {
- nbdkit_error ("fallocate: %m");
+ nbdkit_error ("fallocate: %s: %m", h->name);
return -1;
}
@@ -1306,7 +1306,7 @@ file_cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
r = posix_fadvise (h->fd, offset, count, POSIX_FADV_WILLNEED);
if (r) {
errno = r;
- nbdkit_error ("posix_fadvise: %m");
+ nbdkit_error ("posix_fadvise: %s: %m", h->name);
return -1;
}
return 0;
--
2.47.1

132
0026-file-Add-offset-count-to-error-messages.patch Normal file
View File

@ -0,0 +1,132 @@
From 817bacccc1f44a060a4250ff9e6a27086be09d8b Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 4 Jun 2025 21:15:26 +0100
Subject: [PATCH] file: Add offset/count to error messages
Although the same information is available in debug logs, if verbose
messages (-v) are not used then the information is lost. As it might
be useful occasionally, ensure it is captured in errors.
(cherry picked from commit 253574354252f4a77e2df0769b721e76f1777651)
---
plugins/file/file.c | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 8018b294..e62e2437 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -904,7 +904,8 @@ file_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
ssize_t r = pread (h->fd, buf, count, offset);
if (r == -1) {
- nbdkit_error ("pread: %s: %m", h->name);
+ nbdkit_error ("pread: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
if (r == 0) {
@@ -940,7 +941,8 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
ssize_t r = pwrite (h->fd, buf, count, offset);
if (r == -1) {
- nbdkit_error ("pwrite: %s: %m", h->name);
+ nbdkit_error ("pwrite: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
buf += r;
@@ -1022,7 +1024,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1058,7 +1061,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1066,7 +1070,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
else {
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1088,7 +1093,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1116,14 +1122,16 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
h->can_fallocate = false;
} else {
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ":%m",
+ h->name, offset, count);
return -1;
}
@@ -1147,7 +1155,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (errno != ENOTTY) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1185,7 +1194,9 @@ file_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
/* Trim is advisory; we don't care if it fails for anything other
* than EIO or EPERM. */
if (errno == EPERM || errno == EIO) {
- nbdkit_error ("fallocate: %s: %m", h->name);
+ nbdkit_error ("fallocate: %s: offset=%" PRIu64 ", count=%" PRIu32 ":"
+ " %m",
+ h->name, offset, count);
return -1;
}
@@ -1306,7 +1317,9 @@ file_cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
r = posix_fadvise (h->fd, offset, count, POSIX_FADV_WILLNEED);
if (r) {
errno = r;
- nbdkit_error ("posix_fadvise: %s: %m", h->name);
+ nbdkit_error ("posix_fadvise: %s: offset=%" PRIu64 ", count=%" PRIu32 ":"
+ " %m",
+ h->name, offset, count);
return -1;
}
return 0;
--
2.47.1

9
nbdkit.spec
View File

@ -55,7 +55,7 @@
Name: nbdkit
Version: 1.42.2
Release: 5%{?dist}
Release: 6%{?dist}
Summary: NBD server
License: BSD-3-Clause
@ -103,6 +103,9 @@ Patch0020: 0020-vddk-Cache-the-readonly-flag-from-the-.open-call-in-.patch
Patch0021: 0021-vddk-Move-minimum-version-of-VDDK-to-6.7.patch
Patch0022: 0022-vddk-Unconditionally-test-QueryAllocatedBlocks.patch
Patch0023: 0023-vddk-Pre-cache-the-extents-for-readonly-connections.patch
Patch0024: 0024-file-Save-the-filename-or-equivalent-in-the-file-han.patch
Patch0025: 0025-file-Add-the-filename-or-equivalent-to-error-message.patch
Patch0026: 0026-file-Add-offset-count-to-error-messages.patch
# For automatic RPM Provides generation.
# See: https://rpm-software-management.github.io/rpm/manual/dependency_generators.html
@ -1545,7 +1548,7 @@ fi
%changelog
* Mon Jun 02 2025 Richard W.M. Jones <rjones@redhat.com> - 1.42.2-5
* Thu Jun 05 2025 Richard W.M. Jones <rjones@redhat.com> - 1.42.2-6
- Rebase to nbdkit 1.42.2
- Synch the spec file with Fedora Rawhide.
- nbdkit-ondemand-plugin moves into a new subpackage.
@ -1557,6 +1560,8 @@ fi
resolves: RHEL-89371
- vddk: Pre-cache the extents for readonly connections
resolves: RHEL-94825
- Log filename, offset and count in nbdkit-file-plugin error messages
resolves: RHEL-95364
* Mon Jan 06 2025 Richard W.M. Jones <rjones@redhat.com> - 1.40.4-3
- vddk: Avoid reading partial chunk beyond the end of the disk