linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Paulo Antonio Alvarez <pauloaalvarez@gmail.com>
Cc: linux-ext4@vger.kernel.org
Subject: What is the purpose of windows_io_manager in e2fsprogs?
Date: Tue, 28 Feb 2023 20:49:25 -0800	[thread overview]
Message-ID: <Y/7ZVZ5Wv0NnzT4g@sol.localdomain> (raw)

Hi Paulo,

I'm very confused by all the new Windows-specific code that you added to
e2fsprogs in the following commit:

	commit d04f34a050e3334456e9ee80f95869dc47b0fd9f
	Author: Paulo Antonio Alvarez <pauloaalvarez@gmail.com>
	Date:   Tue Dec 22 15:15:50 2020 -0300

	    libext2fs: add a Windows implementation of the IO manager

So, Windows now has a windows_io_manager, which is used instead of
unix_io_manager.  That *sounds* like something that makes sense.

But actually, AFAICT, Windows used unix_io_manager before, and it more or less
works fine.  The basic UNIX I/O functions like open(), read(), write(), etc.
are available in MSVCRT on Windows.  (Often with quirks, but they are
available.)  MinGW provides some compatibility definitions too, and Windows
builds of e2fsprogs are only supported through MinGW anyway.

So I'd say that unix_io_manager is misleadingly named and really is more like
"regular_io_manager"...

I'm having a hard time seeing what the windows_io_manager gives us for the price
of duplicating lots of code, such as the I/O cache, from unix_io.c.

The only *potential* benefits of windows_io_manager I can see are the following:

  * Its ext2fs_file_open() always uses O_BINARY.  This seems to be a bug fix.

    However, unix_io.c could trivially do this.

  * windows_io_manager considers a path with one leading backslash to be an NT
    namespace path, for example \Device\HarddiskVolume4.  It uses
    DefineDosDevice() to create an alias for it and access it.  Perhaps the
    intent here was to add support for Windows volumes.

    However, this doesn't actually make sense, and IMO it is a bug, not a
    feature.  Paths need to be accepted in a consistent format.  It should just
    use the normal path format that Windows programs use (Win32 namespace).
    Windows volumes can still be specified using paths like \\?\D: or
    \\?\GLOBALROOT\Device\HarddiskVolume4.  (BTW, mke2fs doesn't actually work
    on volumes regardless of any of this, since check_plausibility() fails.)

  * windows_io_manager uses 'FILE_FLAG_NO_BUFFERING | FILE_FLAG_WRITE_THROUGH',
    which is the equivalent of O_DIRECT.  However, it is broken since it does
    this regardless of IO_FLAG_DIRECT_IO, and it also doesn't honor the
    alignment restrictions.  Note: e2fsprogs doesn't use direct I/O by default
    anyway, so direct I/O support doesn't seem like an important feature here.

Would you or anyone else have any objection if I just removed windows_io.c and
made Windows use unix_io.c again, with the O_BINARY fix as that seems to have
been the only real issue with it?

- Eric

                 reply	other threads:[~2023-03-01  4:49 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y/7ZVZ5Wv0NnzT4g@sol.localdomain \
    --to=ebiggers@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=pauloaalvarez@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).