On Tue, Jul 16, 2024 at 05:51:21PM +0200, Martin Wilck wrote:
> On Mon, 2024-07-15 at 18:41 -0400, Benjamin Marzinski wrote:
> > On Fri, Jul 12, 2024 at 07:14:27PM +0200, Martin Wilck wrote:
> > I wonder if we should wrap ibmp_mapinfo in a macro that grabs the
> > function name of the calling function and passes it down to
> > libmp_mapinfo__() otherwise there will be a lot of messages like this
> > or
> >
> > condlog(2, "%s: map %s doesn't exist", __func__, map_id);
> >
> > that use "libmp_mapinfo__" as the function, which isn't super useful.
>
> Right. I think what I'll do is just hardcode "libmp_mapinfo" in these
> messages instead of using __func__.
Actually, I was thinking more of something like:
int do_libmp_mapinfo(int flags, mapid_t id, mapinfo_t info,
const char *func)
{
char idbuf[BLK_DEV_SIZE];
return libmp_mapinfo__(flags, id, info,
libmp_map_identifier(flags, id, idbuf));
}
#define libmp_mapinfo(flags, id, info) \
do_libmp_mapinfo(flags, id, info, __func__)
So we would retain the caller info, but I don't feel that strongly about
it.
>
> >
> > > + condlog(2, "%s: map %s not found",
> > > __func__, map_id);
> > > + return DMP_NOT_FOUND;
> > > + } else
> > > + return DMP_ERR;
> > > + }
> > > +
> >
> > I mentioned tihs before, but the DM_MAP_BY_* identifiers only take up
> > 3
> > bytes so __DM_MAP_BY_MASK can just be 3 or (1 << 2) - 1, unless you
> > are
> > reserving space for more identifiers which is fine but probably
> > unnecessary, since libmultipath isn't a stable API. Or is there some
> > other reason I'm missing.
>
> Is it important that this bit field is "dense"? Indeed I wanted to
> leave some space, even though your're right that it most probably won't
> be necessary. I thought it might be helpful during debugging, for
> example.
>
> Perhaps I should have used the entire low byte for the map_by part.
> That might even provide optimization opportunities for the compiler.
> Unless you object, this is what I'll do.
That's fine. Like I said, it's not a stable API. If it turns out we
don't like it later, we can always change it then.
-Ben