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 Michal > + > + if (ioctl(fd, FDGETDRVTYP, &name) >= 0) { > + close(fd); > + fd = open(filename, mode); > + } > + } > +#endif /* FDGETDRVTYP */ > + > + return fd; > } > > > -- > 2.31.1 >
