Created attachment 675446 [details] return correct device name for /boot on LV. Description of problem: If one of multiple installed systems has /boot located on a separate logical volume, that system will not be recognized as multiboot from another system. Yje problem is that os-prober is returning an incorrect value to grub2 (30_os_prober). The attached patch fixes the problem. Note: if /boot is not saparate but part of the root installed on an LV, it already works. Version-Release number of selected component (if applicable): F18
Created attachment 676313 [details] rebased patch assumes patch from https://bugzilla.redhat.com/show_bug.cgi?id=893997 has been applied
Thank you for the patch. Just to better know what I'm going to maintain and report upstream, would you please shed some light on the patch? Why it is needed (why the current code doesn't recognize /boot on LVM)? Apparently, this patch skips some code for such volumes, I'd like to know why? Thanks again.
Comment on attachment 676313 [details] rebased patch The bottom line is that without the patch, linux-boot-prober will not find and mount a separate /boot which happens to be on a LV. But, I believe you want a little more of an explanation so I hope the following is good enough.. The purpose of the linux_mount_boot() in common.sh is to locate and mount a separate /boot. First of all, the root partition is mounted and linux_mount_boot simply scans the /etc/fstab in the mounted root. If you look at the unpatched common.sh, you will notice it looks for LABEL= or UUID= but not for how an LV would be: /dev/mapper/<whatever> The patch adds the check for handling an /boot on an LV
But it should also work fine with simple device address for /boot like: /dev/sda1 . I wonder why it should work for /dev/sda1 but not for /dev/mapper/<whatever> ? Sorry if I'm missing a clear point.
I wish I had a good answer for you but I do not. It has been a while since I worked on this an I am sorry but I have forgotten some of the details. I do remember that something like /dev/sda3 works fine without the patch and the /dev/mapper/VGone-boot`1 does not. It is coming back to me a little. The /dev/mapper/<whatever> is really a symbolic link to something of the form /dev/dm-<nn> and that does not match what is in /etc/fstab. The patch "fakes it" by recognizing the LV specification. You will notice that some function called mapdevfs() is used. Looking that up, you can see that it is really "readlink -f <partition>". When this is used with a LV /dev/mapper/<whatever>, you get /dev/dm-<nn> Hope this helps.
Thank you for the explanation.
Well, I finally were able to test the setup with /boot on a separate LVM volume. os-prober correctly recognizes the distribution and can successfully mount its /boot partition. The only thing is that it returns device names like /dev/dm-3 rather than /dev/mapper/<label>. But, is this problematic? and why? looks like that /dev/dm-X device names should work fine. Doesn't them? And if it is, fixing mapdevfs() to return symbolic name for LVM devices might be a better solution?!
OK, I see the problem now. It doesn't correctly parse grub options.
OK, No, it was a mistake. I don't see any problems and apparently correct output is generated. The only thing is that the /dev/dm-X name is used rather than /dev/mapper/XXXXXX, but it should be harmless (?!). But probably using /dev/mapper name makes more sense, in which case I think I should fix mapdevfs() function instead.
works for me
I noticed that os-prober-1.57 does not fix this problem ... I do not really care if my patch is used or you fix mapdevfs()
To be honest, I'm not sure about the change yet. It effectively disables mapdevfs() for dev mapper volumes, and I wonder if it doesn't break anything. (mapdevfs() exists for a reason which I'm not sure it doesn't apply for dm volumes. Also, the fix looks to just improve the output (to use volume names rather than names like dm-1), right? since the current implementation (without the fix) does correctly detect a distro with /boot on LVM (at least in my test). Doesn't it? I was thinking about submitting the fix upstream if it only improves output readability. Am I missing a real problem (when the fix is not applied)?
Created attachment 692071 [details] fix /boot on LV by changing mapdevfs() OK, here is a patch which fixes the problem by changing mapdevfs() If the parameter being passed begins /dev/mapper the just return the whole parameter. Otherwise, pass it through readlink
That last patch is completely wrong! [debian has a program called mapdevfs.] First of all, why this concern about supporting /boot in a logical volume ... because grub2 supports it in the /etc/grub.d/10_linux. I am not sure why you would want to use a separate LV when you could keep in with the root LV but the capability exists so someone will use it. There are four ways to specify an LV: 1. /dev/disk/by-uuid/<uuid> 2. /dev/<vgname>/<lvname> 3. /dev/mapper/<vgname>-<lv-name> 4. /dev/dm-<n> The first three are all links to the forth one. What is wrong with the current code and which my original patch addresses is the /dev/dm-<n> is created at bootup and can change if you add or subtract logical volumes. On the other hand, the first three are constant and the third one seems to be the preferred form. This has only become an issue recently because putting /boot on a LV is only recently been possible. So, I stick with my original patch. The important thing is to pass back to grub2 a boot device specification /dev/mapper/<vg>-<lv> so that is will stay constant.
Looks like that you've misunderstood me. I didn't question supporting /boot on LVM, I said that it is already supported but it uses dm-X rather than /dev/mapper/something naming. Also I didn't ask for a patch for mapdevfs() since it was trivial. What I'm concerned about is: why mapdevfs is used in the first place? I've not examined thoroughly, but I want to make sure that skipping it for LVM volumes doesn't break the code in any situations. I'll try to check the code, and if it doesn't matter I'll apply one of the fixes (probably the mapdevfs one) and report upstream (probably with the other patch so that it'll work in debian installer too).
OK, looks like that it is safe to skip mapdevfs for LVM volumes, as they are handled differently. I'll change mapdevfs though, since it seems that not modifying that will break the code if some of those partitions are already mounted.
A couple things. A little googling showed that debian has a program mapdevfs. I believe that os-prober/linux-boot-prober should just use the one defined in the common.sh ... that way things may be a little more repeatable. (IMHO) For some reason, when I was testing earlier, it did not appear that /boot in a LV all by itself worked at all. When I did some testing yesterday, it was supported with the /dev/dm-<n>. But, as I pointed out in an earlier comment, the grub.cfg may have an invalid entry for /boot if LVs are added or removed. I believe that the intent of mapdevfs() was to make sure that a good partition or LV was being used. This idea was a good one but it causes more problems for LVs.
AFAIK, mapdevfs is available in debian installer (probably not in debian itself). Anyway, IMHO it should be either fixed for LVM volumes just like the change we make in makedevfs() function, or another function should be used to not call mapdevfs for LVM volumes. If I understood correctly from the bugs related to mapdevfs, it is used to make sure that os-prober can always detect mounted volumes even if they are mounted from a symbolic link. Fixing mapdevfs makes sure that os-prober won't miss mounted LVM partitions while they are mounted using /dev/mapper/NAME. (However, it WILL miss the partition if it is mounted using any of the alternative naming schemas you mentioned in comment #14, and it means that fixing this bug by both patches can potentially break os-prober). Therefore, I wonder if we can relay on /dev/mapper/name style to be always used. It seems that we should be more careful about this.
I believe the "right way" this should be implemented may be a bit more complicated than is desirable. First, there is a fifth way to point to an LV ... use LABEL=. Second, no matter what systems software such as anaconda may use, the user could change what is in /etc/fstab to any of the five types and things would still work. Third, returning something of the form /dev/dm-<n> from linux-boot-prober should be avoided for reasons previously stated. Forth, it would be "nice" to use the same form as found in /etc/fstab to be returned by linux-boot-prober. However, it that gets overly complicated then I believe using /dev/mapper/<vgname>-<lvname> would be a good choice. Fifth, with respect to linux_mount_boot(), I believe it would be a good idea to use mapdevfs() to regularize the device name internally (e.g., /dev/dm-<n> so long as we return one of the other forms such as /dev/mapper/<vgname>-<lvname>. For example, at the end of linux_boot_mount(), if grep -q "^/dev/dm-", then invoke yet another function which converts "/dev/dm-<n>" into "/dev/mapper/<vgname>-<lvname>".
Yes, I was also thinking about a similar solution is given in point Five. But probably there is no need to map /dev/dm-X back to /dev/mapper/NAME; instead we can save the original (unmapped) name (if it is not already saved in a variable) and return the unmapped name. I'll look into the code as soon as possible.
I'm going to apply a fix, which tries to use the same name as used in /etc/fstab. It doesn't work if the partition is specified using LABEL or UUID. Anyway, that should be fine while the corresponding 'search' lines are added by grub2-mkconfig, so that grub will try to look up the device using uuid and/or label too.
os-prober-1.57-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/os-prober-1.57-2.fc17
os-prober-1.57-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/os-prober-1.57-2.fc18
I have a qemu-kvm/libvirt setup for testing os-prober. One small virtual system has a bunch of system disks from other virtual systems with both btrfs and LVM in various configurations. With 1.57-1, none of the systems on LV were found. But, with 1.57-2, they were all found and grub.conf was properly configured (natually I have the modified grub2 installed also). This is good as far as I am concerned.
So, would you please add a karma to the update you tested (either f17 update or f18 update)?
Sorry about that detail. This was tested under F18 both host and virtual. I have some older F17 virtuals I could test with but even there some of the code (e.g., NetworkManager) is more F18 and beyond. As far as os-prober and grub2 are concerned, it will be the same code but built for F17 since it is fairly easy to have mock produce more versions. Overall, I am very please with the os-prober package as implemented in 1.57-2.
Thanks for the testing (and reporting the bug in the first place) :) I think I should be more clear: would you please go to the updates page (e.g. https://admin.fedoraproject.org/updates/os-prober-1.57-2.fc18 for Fedora 18), login with your FAS account (have you)? and then add a comment and select "Works for me" below it? It is not necessary (I'll be able to push the update to stable repository after some time), but it'd be better if you can. (it'll be pushed to updates repo automatically if 3 persons confirm that it works). But anyway thank you for testing the update. It includes a number of changes and I'd like to be sure that it doesn't break anything. So not adding a comment to the update is not a big deal. Regards
I did not have an ID before which is why I did not understand your "karma" comment. I have one now and have commented.
Thanks :)
Package os-prober-1.57-2.fc17: * should fix your issue, * was pushed to the Fedora 17 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing os-prober-1.57-2.fc17' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2013-2012/os-prober-1.57-2.fc17 then log in and leave karma (feedback).
os-prober-1.57-2.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.
os-prober-1.57-2.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report.