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