On Wed, Nov 20, 2024 at 09:49:17AM +0100, Martin Wilck wrote: > On Tue, 2024-11-19 at 15:33 -0500, Benjamin Marzinski wrote: > > On Tue, Nov 19, 2024 at 05:39:26PM +0100, Martin Wilck wrote: > > > On Fri, 2024-11-15 at 18:22 -0500, Benjamin Marzinski wrote: > > > > if libmp_mapinfo() is run on a device that has no active table, > > > > it will now return with a new exit code, DMP_EMPTY, to signal > > > > this. > > > > Currently all code paths treat this identically to DMP_NOT_FOUND. > > > > > > > > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> > > > > > > I just looked through the code again. I think with this change, we > > > need > > > to modify dm_flush_map__() and do_foreach_partmaps(). They should > > > remove / act on empty maps. What do you think? > > > > I'm not sure that we need to add extra code to handle the possiblity > > that an empty device could appear at any time, since like I said, > > this is a corner case that I've never actually seen in the wild. So > > if > > a device was previously a valid multipath device, I don't think we > > need > > to worry about the possibility that it suddenly became empty. > > > > But I can see the value in running something like > > > > # multipath -F > > > > and having it clean up any empty multipath devices. As for > > do_foreach_partmaps(), are you thinking about cleaning up empty > > partition devices or non-empty partition devices on top of empty > > multipath devices? > > > > Non-empty partition devices on top of empty multipath devices would > > imply that a multipath device was valid at one point, and then became > > empty, which I don't see an easy way of happening. > > > > The problem with empty partition devices is that partition devices > > are > > created by kpartx completely asynchronously to us. That empty > > partition > > device could be in the process of being created. > > Right, but multipathd is in the process of deleting the map. If there's > actually a race and kpartx finishes creating the partition map, > multipathd will fail to remove the multipath map. The likely outcome > will be a multipath map with just one partition device. If multipathd > comes first, kpartx will fail, but there's a good chance that > multipathd will succeed in flushing the multipath map, so we'll end up > with a consistent state. > > If kpartx had run a little earlier and had finished setting up the map, > multipathd would have removed it, and if kpartx had run a little later, > it would have failed because of a missing target. > If we make do_foreach_partmaps() remove empty partition maps, it has keep the is_mpath_part_uuid() call. Otherwise we would remove any empty partition device, including ones that aren't for this multipath device, which breaks your argument above. > > So I'm not against checking for DMP_EMPTY in dm_flush_map__() and > > removing the empty device if its opencount is 0. But I'd rather not > > try messing with partmaps. Do you have a specific case you are > > worried > > about? > > >From the argument above, I'd say that flushing empty partition maps is > the right thing to do, for consistency reasons. > > But now I may be overlooking something. How would you feel about adding a parameter to do_foreach_partmaps() to say whether or not it should remove empty partmaps (or possibly just checking if the partmap_func is remove_partmaps). Your argument makes sense when you are removing a device. But what about functions like dm_cancel_remove_partmaps() and dm_rename_partmaps()? I'm not sure that these should automatically empty (a possibly being created) partition devices. -Ben > Martin
