Some small-ish changes to auto-detect the loop devices used

[mvastola]

This is needed if the system already has one or more loop devices assigned.

• Also added in support for a loop device as the target (useful when using this script to create an installer for a VM) by running kpartx on the target device.
• Finally fixed an issue wherein umounting of the target device will fail if it currently has more than one partition.

[eprigorodov]

The point itself is very valid. It is incorrect to hard-code a specific loop device because to the moment of the script run any number of them can be allocated.

I would just propose the following:

• Do not call map_partitions() for finding out the layout of USB drive. Rationale: a few lines above it was just re-partitioned by sgdisk to a predefined GPT with only one HFS+ partition.
• Do not remove the partprobe call, without it the kernel would still see the old GPT of USB stick, whereas it just has been changed.
• Simplify the sed regexp in the map_partitions(), assuming that it always use loop devices, so its output must contain loop\d+p\d+.
• Do not suppress errors in map_partitions() so it better fails early rather than does unpredictable damage.

[mvastola]

Thanks for checking out this PR. I addressed all but the first point by replying to your comments. Please reply back to those and we'll figure them out.

Re the first bullet point, I'm not sure I understand fully. Are you simply saying it's redundant, if so, i disagree, unless we add back partprobe (a possibility I addressed in the inline comments). In any case, it just seemed simpler to not have two different procedures based on the type of the underlying device. Also, since partprobe and kpartx can put the block devices in different folders (/dev vs /dev/mapper, respectively) and since loop devices have the letter p prepended to the partition number, the code would get needlessly more complex in my opinion.

Let me know your thoughts on all this and we can resolve these issues.

I'll go ahead and make the changes I mentioned above re the first inline comment tomorrow as I'm not currently near a computer.

[eprigorodov]

Thank you for considering my comments :)

With regard to the first point let me rephrase the same concern in another way:

sgdisk re-partitions the USB drive to a fixed layout that contains a single HFS+ partition. So device name "$stick_dev"1 will be surely correct. What is much more important, this device name will always be based on the user-supplied value for $stick_dev. Remember, the script does some precaution checks and warns user that everything under e.g. /dev/sdc will be destroyed.

The map_partitions() creates a new mapper device and then tries to detect its name. That fails now because regexp expects to parseadd map loop1p1 ...and cannot match add map sdc1 returned for the USB stick. It is possible to fix the regexp, but I don't like the very idea of formatting device that was dynamically identified by using regular expressions and sed.

[eprigorodov]

I am fully fine about using map_partitions() for image files, because that is only read access and possible errors cannot bring much harm.

Should we agree on the first point then please commit your fixes and I will merge the PR.

[mvastola]

My point though is that "$stick_dev"1 is not sure to be correct, at least on loop devices.

In that case, at the very least it will be "$stick_dev"p1, but as I mentioned, some Linux installs don't map loop device partitions to /dev/loopNpX. Using kpartx ensures consistency there so the point of this function is a one-size fits all.

So if we didn't use the regex for physical devices, you'd need to have an if/then in map_partitions and return the prefix without the partition number. This could be done by simply returning the argument to map_partitons if it doesn't contain loop, but it just seemed simpler to do everything the same way.

If you want though, I'll change it. Just didn't see the point.

[eprigorodov]

I am starting to get the idea. You mean using the script not for creating a physical USB installer stick, but rather for creating an installer image file (to attach it to VM), is that right?

[mvastola]

Exactly. So you should be able to do like:

dd if=/dev/zero of=install-disk.img bs=1G count=8
losetup /dev/loopX install-disk.img
./mkosxinstallusb.sh /dev/loopX install-dir
losetup -d /dev/loopX

[eprigorodov]

That changes a lot and really makes kpartx a universal way of addressing partitions. I will merge the PR as is and then will fix work with physical USB devices and will add yet another confirmation request before formatting. Thank you for the good observation, for the fix and for your patience in explanation :)

[mvastola]

Ack. Thanks for approving, but I never got to make that change to the regex. Can you do it, or should I make another PR? Things that need to be done:

• Change (\S+)p in the regex to ([a-z]+)
• Anywhere I put ${result_of_map_partition}pX, the 'p' needs to be deleted.
• The set +e should be removed from map_partitions
• set -e should be added to the top of the script

[eprigorodov]

I will do that, thank you.