[systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Mon Dec 1 05:28:33 PST 2014
On Mon, Dec 01, 2014 at 11:24:58AM +0100, Karel Zak wrote:
> On Fri, Nov 28, 2014 at 09:27:43PM +0100, Lennart Poettering wrote:
> > On Fri, 28.11.14 20:50, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> >
> > > On Sun, Nov 23, 2014 at 08:33:41PM -0800, Chris Leech wrote:
> > > > This adds auto detection for iSCSI and some FCoE drivers and treats
> > > > mounts to file-systems on those devices as remote-fs.
> > > >
> > > > Signed-off-by: Chris Leech <cleech at redhat.com>
> > > No need for this.
> > >
> > > I now pushed patches 1-4 with some small changes here and there.
> > > Since libmount is not optional, I removed if from the version string.
> > >
> > > This patch I didn't push: this seems like something that would be better
> > > done through udev rules, by setting some tags. Then systemd could simply
> > > check for the presence of a tag on the device without having any
> > > special knowledge about iscsi and firends.
> >
> > Honestly, I am really not sure I like this patch.
> >
> > One one hand we now have a hard dep on libmount. Which I figure is
> > mostly OK. However, what I find really weird about this is that even
> > though libmount is supposed to abstract access to the "utab" away, it
> > doesn't sufficiently: we still hardcode the utab path now so that we
> > can watch it. I mean, either we use an abstracted interface or we
> > don't. THis really smells to me as either libmount should provide some
> > form of inotify iface to utab, or we should parse utab directly. If
> > libmount is the only and official API for utab, then we should
> > that. If the utab file however is API too, then we can well go ahead
> > and parse it directly.
>
> I'd like to keep the utab file private. The whole libmount API is
> abstraction and completely hides the difference between original
> /etc/mtab and new /run/mount/utab.
>
> The original Chris' purpose for the patches has been _netdev, it
> means to improve detection of dependence between filesystem and
> network. I still believe that the right way is to check for network
> block devices than rely on crazy _netdev userspace mount option.
>
> Wouldn't be enough to use Chris' iSCSI and FCoE auto detection?
Please see previous discussion... Detecting network might not be trivial
if the devices are layered and there's a network-requiring device somewhere
lower in the stack.
Using _netdev is supposed to be an easy mechanism which admins can use
in case whatever other schemes we have in place don't work.
> Or we can add udev rule to have NET_DEVBLK tag in udevdb, or maybe we
> can ask kernel developers to add /sys/block/sdb/network (as we already
> have "removable" in /sys).
>
> Chris, why do you want both ways (utab and the auto detection)?
>
> IMHO it would be really nice to completely kill _netdev in systemd
> universe ;-) It's legacy from shell init scripts.
>
>
> Anyway, if you still want to read userspace mount options than I can
> add something like:
>
> mnt_inotify_mtab_add_watch()
> mnt_inotify_mtab_rm_watch()
> mnt_inotify_mtab_changed()
>
> to hide all the paths and private mtab/utab stuff.
Maybe something more like what journal client code does here:
mnt_mtab_get_fd() -> epoll fd
mnt_mtab_get_events() -> EPOLLIN|EPOLLPRI
mnt_mtab_process() -> information what changed
and then mnt_mtab_wait() can be constructed on top of this,
but systemd wouldn't be using it.
> > THe new code looks also buggy to me. For example, am I missing
> > something or is nobody creating /run/mount? I mean, we need to make
> > sure that it exists before we can install an inotify watch on it.
>
> yep, it should be hidden within libmount
>
> > Also, it leaves the cescape() calls in for what we read from libmount,
> > which makes a lot of alarm bells ring in my head: doesn't libmount do
> > that on its own?
>
> sure, libmount encodes/decodes all when read/write the files.
Zbyszek
More information about the systemd-devel
mailing list