On Wed, 2024-07-10 at 18:53 -0400, Benjamin Marzinski wrote:
> On Tue, Jul 09, 2024 at 11:39:08PM +0200, Martin Wilck wrote:
>
> This is a great idea! I just have a few minor issues.
>
> > libmp_mapinfo() is intended as a generic abstraction for retrieving
> > information from
> > the kernel device-mapper driver. It retrieves the information that
> > the caller
> > needs, with a minimal set of DM ioctls, and never more then 2 ioctl
> > calls.
> >
> > libdm's DM_DEVICE_TABLE and DM_DEVICE_STATUS calls map to the
> > kernel's
> > DM_TABLE_STATUS ioctl, with or without the DM_STATUS_TABLE_FLAG
> > set,
> > respectively. DM_TABLE_STATUS always retrieves the basic map status
> > (struct
> > dm_info) and the map UUID and name, too.
> >
> > Note: I'd prefer to use an unnamed struct instead of _u in
> > union libmp_map_identifer. But doing using an unnamed struct and
> > and
> > initializing the union like this in a function argument:
> >
> > func((mapid_t) { .major = major, .minor = minor })
>
> This is a nitpick, but instead of using _u, couldn't you just pass
> the
> major and minor as a dev_t? If you're working with an actual major
> and
> minor, you would need to do something like:
>
> (mapid_t) { .dev = makedev(major, minor) }
>
> but we are often already dealing with a dev_t, so instead of doing:
>
> (mapid_t) { ._u = { major(dev), minor(dev) } }
>
> we could just do:
>
> (mapid_t) { .dev = dev }
>
> To get the major and minor out in the libmp_mapinfo() you would need
> to
> use major(id.dev) and minor(id.dev), instead of id._u.major and
> id._u.minor, but I think first set is more readable anyway.
Nice idea. I wonder if I should add this as an additional option, or
replace the current approach.
Martin
