17.01.2022 14:50, Karel Zak wrote:
On Mon, Jan 03, 2022 at 09:43:11AM +0100, Karel Zak wrote:
Hi! Are you working on this? If not I can try to make a v2.
I had a vacation in the last 14 days; nothing is done. Go ahead if you
have time for this task. I'm going to work on something else this
week.
It seems you're busy with something else ;-), so I have implemented it,
but it's not tested yet.
Oh seems yes ;) Great that you've done it!
Please, please, can you review and test it with your environment?
Ohh. Will try to remember and restore the environment this week :)
Thanks!
See the patch bellow or topic/blkid-floppy branch on github.
Karel
From 49be1b256ad351cde2e0cf5000c7046e7010cc02 Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@xxxxxxxxxx>
Date: Mon, 17 Jan 2022 12:37:13 +0100
Subject: [PATCH] libblkid: reopen floppy without O_NONBLOCK
Vladimir Sementsov-Ogievskiy wrote:
The commit "floppy: reintroduce O_NDELAY fix" was removed from kernel,
so we faced the bug described and discussed here:
https://bugzilla.suse.com/show_bug.cgi?id=3D1181018
Discussion in kernel list on reverting the commit:
https://www.spinics.net/lists/stable/msg493061.html
In short, I can quote Jiri Kosina's comment:
opening floppy device node with O_NONBLOCK is asking for all kinds
of trouble
So opening floppy with O_NONBLOCK in blkid leads to failure of blkid,
probable failure of mount and unpleasant error messages in dmesg (see
also patch 02 for details).
Based on patch from Vladimir.
CC: Jiri Kosina <jkosina@xxxxxxx>
Reported-by: Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx>
Signed-off-by: Karel Zak <kzak@xxxxxxxxxx>
---
include/fileutils.h | 3 +++
lib/fileutils.c | 17 +++++++++++++++++
libblkid/src/probe.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+)
diff --git a/include/fileutils.h b/include/fileutils.h
index c36ce6353..8722ed59b 100644
--- a/include/fileutils.h
+++ b/include/fileutils.h
@@ -97,4 +97,7 @@ extern void ul_close_all_fds(unsigned int first, unsigned int last);
#define UL_COPY_WRITE_ERROR (-2)
int ul_copy_file(int from, int to);
+
+extern int ul_reopen(int fd, int flags);
+
#endif /* UTIL_LINUX_FILEUTILS */
diff --git a/lib/fileutils.c b/lib/fileutils.c
index 7a8fce26f..5b15d4916 100644
--- a/lib/fileutils.c
+++ b/lib/fileutils.c
@@ -288,3 +288,20 @@ int ul_copy_file(int from, int to)
return copy_file_simple(from, to);
#endif
}
+
+int ul_reopen(int fd, int flags)
"reopen" sounds a bit like we are going to close old fd, but we don't... No better suggestion :(
+{
+ ssize_t ssz;
+ char buf[PATH_MAX];
+ char fdpath[ sizeof(_PATH_PROC_FDDIR) + sizeof(stringify_value(INT_MAX)) ];
Extra whitespace after '+'
Also, I think you should add +1 for '/' (you use it below), and +1 for finishing \0.
Or alternatively we may just use PATH_MAX here too and not care.
+
+ snprintf(fdpath, sizeof(fdpath), _PATH_PROC_FDDIR "/%d", fd);
+
+ ssz = readlink(fdpath, buf, sizeof(buf) - 1);
+ if (ssz <= 0)
+ return -EINVAL;
alternatively you may return -errno on ssz < 0, and assert that ssz > 0, I hope 0 can't be returned.
+
+ buf[ssz] = '\0';
+
+ return open(buf, flags);
+}
diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
index 6168370e2..d69ff530f 100644
--- a/libblkid/src/probe.c
+++ b/libblkid/src/probe.c
@@ -103,6 +103,9 @@
#ifdef HAVE_ERRNO_H
#include <errno.h>
#endif
+#ifdef HAVE_LINUX_FD_H
+#include <linux/fd.h>
+#endif
#include <inttypes.h>
#include <stdint.h>
#include <stdarg.h>
@@ -113,6 +116,7 @@
#include "sysfs.h"
#include "strutils.h"
#include "list.h"
+#include "fileutils.h"
/*
* All supported chains
@@ -907,6 +911,35 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
if (fd < 0)
return 1;
+#ifdef FDGETFDCSTAT
+ {
+ /*
+ * Re-open without O_NONBLOCK for floppy device.
+ *
+ * Since kernel commit c7e9d0020361f4308a70cdfd6d5335e273eb8717
+ * floppy drive works bad when opened with O_NONBLOCK.
+ */
+ struct floppy_fdc_state flst;
hmm, extra whitespace?
+
+ if (ioctl(fd, FDGETFDCSTAT, &flst) >= 0) {
+ int flags = fcntl(fd, F_GETFL, 0);
+
+ if (flags < 0)
+ goto err;
+ if (flags & O_NONBLOCK) {
+ flags &= ~O_NONBLOCK;
+
+ fd = ul_reopen(fd, flags | O_CLOEXEC);
+ if (fd < 0)
+ goto err;
+
+ pr->flags |= BLKID_FL_PRIVATE_FD;
+ pr->fd = fd;
+ }
+ }
+ }
+#endif
+
#if defined(POSIX_FADV_RANDOM) && defined(HAVE_POSIX_FADVISE)
/* Disable read-ahead */
posix_fadvise(fd, 0, 0, POSIX_FADV_RANDOM);
Looks good to me. I hope I'll be able also to test it soon.
--
Best regards,
Vladimir
