On Mon, 2024-11-25 at 13:05 -0500, Benjamin Marzinski wrote:
> On Fri, Nov 22, 2024 at 10:54:31PM +0100, Martin Wilck wrote:
> > On Fri, 2024-11-22 at 16:11 -0500, Benjamin Marzinski wrote:
> > >
> > > + if (mpp->action == ACT_CREATE) {
> > > + char alias[WWID_SIZE];
> > > + int rc = dm_find_map_by_wwid(mpp->wwid, alias,
> > > NULL);
> > >
> > > + if (rc == DMP_NO_MATCH) {
> > > + condlog(1, "%s: wwid \"%s\" already in
> > > use
> > > by non-multipath map \"%s\"",
> > > + mpp->alias, mpp->wwid, alias);
> >
> > Nitpick: This is almost the same message that dm_find_map_by_wwid()
> > has
> > printed immediately before. Similar messages are also printed in
> > all
> > other callers of dm_find_map_by_wwid(). I suggest that we either
> > remove
> > the log message in the callers or in dm_find_map_by_wwid() itself.
> > Probably the latter, then we can decide on a case by case basis in
> > the
> > caller whether the situation needs to be logged.
>
> I'm confused here. dm_find_map_by_wwid() doesn't print any message
> itself. It calls libmp_mapinfo__(), which could print either:
>
> condlog(lvl, "%s: map %s has multiple targets", fname__, map_id);
>
> or:
>
> condlog(lvl, "%s: target type mismatch: \"%s\" != \"%s\"",
> fname__, tgt_type, target_type);
>
> in this case, but at level 4 verbosity, and these are used to
> distinguish between the various reasons why DMP_NO_MATCH could be
> returned, which seems like a reasonable thing at that verbosity.
You are right. Not sure what I was looking at. Probably I was just
reviewing your patch too late at night :-/
Forget this. So your series LGTM except for the typo in 08 and the
suggestion to squash patch 9 and 10.
Thanks
Martin
