14.12.2021 14:45, Michal Suchánek wrote:
On Thu, Dec 09, 2021 at 03:12:33PM +0100, Vladimir Sementsov-Ogievskiy wrote:
Since c7e9d0020361f4308a70cdfd6d5335e273eb8717
"Revert "floppy: reintroduce O_NDELAY fix"" commit in linux kernel,
floppy drive works bad when opened with O_NONBLOCK: first read may
fail. This cause probing fail and leave error messages in dmesg. So, if
we detect that openedfd is floppy, reopen it without O_NONBLOCK flag.
Reproduce is simple:
1. start the linux system (kernel should include the mentioned commit)
in QEMU virtual machine with floppy device and with floppy disk
inserted.
2. If floppy module is not inserted, modprobe it.
3. Try "blkid /dev/fd9": it will show nothing, errors will appear in
dmesg
4. Try "mount /dev/fd0 /mnt": it may succeed (as mount not only probing
but also try filesystems one by one, if you have vfat in
/etc/filesytems or in /proc/filesystems, mount will succeed), but
errors about failed read still appear in dmesg, as probing was done.
Mentioned errors in dmesg looks like this:
floppy0: disk absent or changed during operation
blk_update_request: I/O error, dev fd0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
floppy0: disk absent or changed during operation
blk_update_request: I/O error, dev fd0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
Buffer I/O error on dev fd0, logical block 0, async page read
Note also, that these errors also appear in early dmesg messages, if
probing is done on system startup. For example, it happens when
cloud-init package is installed.
Note2: O_NONBLOCK flag for probing is used since
39f5af25982d8b0244000e92a9d0e0e6557d0e17
"libblkid: open device in nonblock mode", which was done to fix the
issue with cdrom: if tray is open and we call open() without O_NONBLOCK
the tray may be automatically closed, which is not what we want in
blkid.
Good discussion on this bug is here:
https://bugzilla.suse.com/show_bug.cgi?id=1181018
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx>
---
Note, that this commit is done as a "minimal change", i.e. I only try to
rollback O_NONBLOCK for floppy. The other way is to detect CDROM
instead, and reopen with original flags for everything except CDROM.
I also tried fcntl instead of close/open, and that didn't help.
libblkid/src/probe.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
index 70e3dc0eb..68a644597 100644
--- a/libblkid/src/probe.c
+++ b/libblkid/src/probe.c
@@ -94,6 +94,9 @@
#ifdef HAVE_LINUX_CDROM_H
#include <linux/cdrom.h>
#endif
+#ifdef HAVE_LINUX_FD_H
+#include <linux/fd.h>
+#endif
#ifdef HAVE_LINUX_BLKZONED_H
#include <linux/blkzoned.h>
#endif
@@ -200,10 +203,32 @@ blkid_probe blkid_clone_probe(blkid_probe parent)
* We add O_NONBLOCK flag to the mode, as opening CDROM without this flag may
* load to closing the rom (if it's open), which is bad thing in context of
* blkid: we don't want to change the actual device state.
+ *
+ * Still, since c7e9d0020361f4308a70cdfd6d5335e273eb8717
+ * "Revert "floppy: reintroduce O_NDELAY fix"" commit in linux kernel, floppy
+ * drive works bad when opened with O_NONBLOCK: first read may fail. This cause
+ * probing fail and leave error messages in dmesg. So, if we detect that opened
+ * fd is floppy, reopen it without O_NONBLOCK flag.
*/
int blkid_safe_open(const char *filename, int mode)
{
- return open(filename, mode | O_NONBLOCK);
+ int fd = open(filename, mode | O_NONBLOCK);
+ if (fd < 0) {
+ return fd;
+ }
+
+#ifdef FDGETDRVTYP
+ {
+ char name[1000];
Hello,
I wonder if it's better to use FDGETFDCSTAT which seems to be meant as
stable API.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/fd.h#n271
As is this allocates 1k from stack and can be presumably called from
application context with arbitrarily deep stack so it seems a bit
wasteful. floppy_fdc_state has under 60 bytes.
Also if you are not interested in the result you can make the buffer
static. Not sure it makes sense to bother if the buffer size is
reasonable.
Thanks for advice, will do for v2. I'm rather far of kernel development, and not fan of floppy disks:) So my choice was "something I can understand from drivers/block/floppy.c and which never fail". I tried FDGETPRM (because it used somewhere else in util-linux), but it fails in described case.. and in kernel code it's obvious that it may fail.
+
+ if (ioctl(fd, FDGETDRVTYP, &name) >= 0) {
+ close(fd);
+ fd = open(filename, mode);
+ }
+ }
+#endif /* FDGETDRVTYP */
+
+ return fd;
}
--
2.31.1
--
Best regards,
Vladimir
