Re: [dm-devel] [PATCH v2 27/37] multipathd: watch bindings file with inotify + timestamp

[Martin Wilck <mwilck@suse.com>]

On Thu, 2023-09-14 at 15:25 +0200, Martin Wilck wrote:
> On Wed, 2023-09-13 at 17:07 -0500, Benjamin Marzinski wrote:
> > 
> > I'm not sure if we should assert that the file has changed if we
> > can't stat() it.
> 
> I think it's better to (try to) reread the file than pretend that the
> file hadn't changed ("if in doubt, reread"). Rationale:
> 
> In general, the most likely cause for stat() to fail would be that
> the
> file (or the directory or file system containing it) had been
> removed.
> Actually, almost every possible error documented in stat(2) (except
> ENOMEM) indicates such a condition in one way or the other. But in an
> IN_MOVED_TO handler for just this file, that seems quite unlikely, so
> we're really looking at a corner case situation here. A non-existing
> file means no bindings; a "reconfigure" operation would cause
> existing
> bindings to be preserved, newly probed maps would get a new alias
> assigned. Looking at it that way, "rereading" a non-existing file
> doesn't do much harm. Our current bindings list may contain
> additional
> bindings that might be lost by re-reading, but still I think we have
> to
> assume that the file was intentionally deleted, and act accordingly.

Ouch, I'm embarrassed. My own code does not what I described here, it
returns an error if reading the bindings file fails, and does not
replace the current global bindings with an empty list. 

We could discuss changing this behavior with a later patch set.

Anyway, I think the basic reasoning was correct: we received an update
event for the file and couldn't prove that our current internal state
was up-to-date; thus we should try to re-read the file next time.

Martin








--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel