[systemd-devel] [PATCH v3] dev-root.device is not active, results in an umount spree

[Martin Pitt]

Hello,

Lennart Poettering [2015-05-18 22:57 +0200]:
> > +        if (dev) {
> > +                sysfs = udev_device_get_syspath(dev);
> > +                if (!sysfs)
> > +                        return 0;
> > +        }
> 
> Why move this down? In order to keep the patch small and easy to grok,
> can this stay up where it was, but simply be enclosed in the "if (dev) {}" check?

Right, that was probably just the result of multiple edits/iterations;
moved back.

> > -                if (!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode))
> > -                        return 0;
> > +                        dev = udev_device_new_from_devnum(m->udev, S_ISBLK(st.st_mode) ? 'b' : 'c', st.st_rdev);
> > +                        if (!dev && errno != ENOENT)
> > +                                return log_oom();
> 
> Hmm, why are all these errors supposed to be OOM?

Not sure, perhaps hysterical raisins? But..

> udev_device_new_from_devnum sets errno correctly, hence we should just
> print what it really is set to with log_error_errno(), unless of
> course it is ENOENT.

Makes sense, done now. It's an unrelated change to this patch, but if
you don't mind let's clean it up.


> > +                } else {
> > +                        if (errno != ENOENT)
> > +                                return log_error_errno(errno, "Failed to stat device node file %s: %m", node);
> 
> The if "else {" and "if (errn..." lines could be merged into one 
> "else if (errno != ...", no?

Right, done.

Updated patch attached. It  doesn't look significantly smaller, mostly
because lots of it is an indentation change :/

Re-tested on a normal system, nspawn, LXC, and with ejecting a mounted
CD.

> > Subject: [PATCH 2/3] device: never transition from "tentative" to "dead"
> Not following on this patch

Will reply on your other response. TL/DR: Current code in master is
overzealous, this patch is overcautious, this needs some more thought
and work; I don't have an updated patch yet.

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-device-create-units-with-intended-found-value.patch
Type: text/x-diff
Size: 5099 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20150519/03f23119/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20150519/03f23119/attachment-0001.sig>