> > > creating the logical volumes will. We don't currently do a devsync() > > > (which does a settle using the --exit-if-exists=/path/to/device/expected > > > > I generally think that '--exit-if-exists=' is a bug. > > consider this: > > a.) /dev/sda has 1 partition on it, so '/dev/sda1' exists (from previous > > install). > > b.) we repartition /dev/sda, blockdev --rereadpt, and and then call > > 'udevadm settle --exit-if-exists=/dev/sda1' > > We wipe the disk's partition table, and then force a reread of the > partition table > and ensure that the disk no longer has *any* partitions, whatever > their name was or the location. > > The disk is clean before we create any new partitions. > > > c.) udevadm checks to see if /dev/sda1 exists. It does (it always did) > > d.) udev events continue, and re-read the partition table, delete all > > the old partitions and re-create them. > > e.) while 'd' is happening our code thinks it is fine to use /dev/sda1 > > > > I think we have this general fallacy in logic at places. > > I disagree. The order of events that happen in curtin are as follows: > > 1) clear-holders calls wipe_superblock on /dev/sda (which has a /dev/sda1) > 2) if the device being wiped has partitions (it does), then we wipe > the partition table > and then reread the partition table, settle and query the disk > for any partitions, repeat > until there are no partitions reported from the kernel, we fail > if we cannot wipe the partitions. > 3) after all disks are clear (no partitions are left on any disk > specified which has wipe superblock > 4) we create a partition table (empty) on /dev/sda > 5) we create a first partition on /dev/sda, say /dev/sda1 > 6) we query the path to /dev/sda1 via the get_path_to_storage() which > calls devsync() which > does a udevadm settle --exit-if-exists > > There isn't a logic bug here because we know that *we* were the ones > to create the partition and we know what the expected /dev path is. > > > > > /dev/sda1 above could just as well be /dev/vg0 or any other device. > > It won't be a volume group, but yes, any other block device that we > create. The logic above holds for those as well. Where do you think > it fails after accounting for how we wipe devices and ensure they're > clear (including partitions)? > > > > > > > to have a fast exit path if the device node has been created) at the end > > > of lvm logical volume creation, but > the subsequent call of the user > > > of the lvm device uses get_path_to_storage() which will do a devsync(). > > > > udevadm settle is *not* a slow operation if there are no events in the > > queue. It is a very important one if there *is* events in the queue. > > > > $ time udevadm settle > > real 0m0.013s > > user 0m0.004s > > sys 0m0.001s > > At best, at worst, the default timeout for udevadm settle is 120 > seconds; and without an --exit-if-exists, we wait on *all* events in > the queue at the time of the settle call. > > > > > So... I'd much rather have any functions that may cause udevadm events > > to call settle by default and be safe than try to shave off an extra > > 1/100th of a second in a OS install. > > The point isn't to shave off seconds, it's to not bother waiting on > events that are unrelated to the work we're performing. Any time we > avoid calls to settle, we help avoid potential 120 second slowdowns. non-sense. have you ever seen 'udevadm settle' take 120 seconds? Can you give an example of when it *could* do that? The only thing I can reasonably come up with are disk based operations on large md or something. Those are exactly the reasons we want to settle. Anything else that was seemingly unrelated but is taking significant time honestly probably something important to an installer. I'd much rather see a log entry that showed a timeout failed on a settle than try to diagnose some related failure. If you use --exit-if-exists without having settle'd and checked that the thing didn't exist before you perform an operation that would cause it to it is a logic flaw. and its one you're going to pull your hair out trying to find. Your only argument against calling udevadm settle is to not waste time. And my argument is that it is not a waste of time both because a.) it is a very insignificant amount of time except for when it is important. b.) it is important in most cases. I'd much prefer that any function that makes changes to disks settle by default. If the caller wants to *not* settle (because they have a better way or more knowledge) then we can do that. The function is just more caller friendly if it doesn't have race conditions waiting to explode in the user's face.