Bug 1236649

Okay, thinking of it, I could imagine a case where this breaks (root in top level btrfs, but with subvolume in root filesystem), will test another day.

Created attachment 1044462 [details]
new patch

New version of the patch, sorry for the inconvenience.

See https://bugzilla.redhat.com/attachment.cgi?id=1044461&action=diff for easier review.

This bug appears to have been reported against 'rawhide' during the Fedora 23 development cycle.
Changing version to '23'.

(As we did not run this process for some time, it could affect also pre-Fedora 23 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 23 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora23

os-prober 1.68-1.fc22 fails to detect my debian and debian-testing installs using btrfs filesystem with no btrfs subvolume. 

OS-prober DOES detect my kubuntu install on sda3, which does use btrfs subvolumes.

I can mount the debian & deb-testing partitions easily, with default mount options:

/dev/sda6 on /mnt/debian type btrfs (rw,relatime,ssd,space_cache,subvolid=5,subvol=/)
/dev/sda7 on /mnt/deb-testing type btrfs (rw,relatime,ssd,space_cache,subvolid=5,subvol=/)

But all os-prober gives me is this:

os-prober
/dev/sda2:Linux Mint 17.2 Rafaela (17.2):LinuxMint:linux
/dev/sda3:Debian GNU/Linux (jessie/sid):Debian:linux:btrfs:UUID=7fdfa51b-2b8e-4ddf-bd5d-a1dca1a1b915:subvol=@
/dev/sdb1:Windows 7 (loader):Windows:chain
/dev/sdb2:openSUSE 13.1 (x86_64):SuSE:linux

Note: os-prober misidentifies the Kubuntu on /dev/sda3 as Debian (grub correctly identifies it as Kubuntu in the Adbanced options menu):

Note: os-prober gives no listing for debian or debian testing on sda6 & sda7


This needs to be fixed.

You guys do realise the impact of this bug is that any poor bastard who installs fedora after installing Kubuntu or somseuch, has just lost access to their Kubuntu install (unless they have the skills to hack their grub config, whcih is an unreasonable expectation)?

Do you not realise how serious that is? 

How happy would you be, if Ubuntu didn't give a damn, and missed Fedora and RHEL when it installed grub?

How is it possible that this bug still exists on fedora 23? 

The os-prober used by Kubuntu obviously detects their subvolumes. How does it not work in the os-prober version used by F23?

Sorry, some confusion in my last sarcastic comment. The Kubuntu does use btrfs subvolumes by default, it's the debian which doesn't, and which gets nuked by Fedora's grub/os-prober.

I just have to say that I am well sick of these issues, because I've just upgraded from fedora 21 to 22, to 23, and 23 is an abortion with the nvidia/xorg incompatibilities, and inability to probe my debian OSes.

And the irony is that in F21 which i was running up until a few days ago, i had the opposite issue - i'm moderately sure the os-prober/grub in F21 was finding my debian and debian-testing installs, and i could mount them. But i don't think i could mount the kubuntu OS with btrfs subvolumes in f21...

Now in f23 I can mount all my other os-volumes, and os-prober has magically re-discovered my kubuntu, but I get kicked in the nuts by this os-prober which has suddenly become unable to probe my debian installs.

So i'm going to have to boot into kubuntu to reinstall grub, in order to regain access to debian, and then i'm going to go back to using debian.


I'll give you threee guesses whether I'm going to bother using fedora much in the near future...

John, could you try my patch?
This would really help, if someone else than me would confirm it ... so the fedora devs could include the patch ...

It's not fedora's fault ... os-prober has bad btrfs support in any distribution ... Fedora's btrfs support is superior to the other distro's (acutally a few other distros also ship a patched os-prober like Opensuse ...), but the patch fedora is using for os-prober has one bug: it does not detect os installed on a simple btrfs partition without root being on a subvolume ...

@Helmut: Thank you for your support despite the fact that I (unfortunately) was unable to consider & integrate your fix. I hope to be able to do it soon.

@John: You can easily try upstream os-prober (which you should already have in Debian) and see how it behaves before blaming me. We've patched os-prober heavily and sent many of the fixes to upstream, but except a few simple ones, other are still waiting for review in Debian Bugzilla. BTRFS support in Fedora is contributed from a Fedora contributor. What is used in OpenSUSE (and probably others) is based on Fedora patch. If you are unhappy with os-prober status (as I am), you can complain upstream for being too conservative. If you find any distribution to have a better os-prober, it is certainly because of their own patches.

Sorry Hedayat, i didn't mean to imply any blame on you.

The way i see it, filesystem developers eg btrfs devs shouldn't implement features like these subvolumes, unless they also going to help fix the tools their new feature breaks, or depends on.

And if the filesystem devs haven't ensured the requisite tools are available, then the filesystem/features shouldn't be used.

Obviously installing several of my oses to to btrfs was a mistake, i expected too much.

Stable-type distros should be refusing to offer btrfs as an option for installing root filesystem - until btrfs devs have helped to properly fix these issues in os-prober.

Just discovered i don't have enough space to upgrade my kubuntu to latest release but i do have enough space to upgrade my debian and debian testing. 

So when i'm up-to-date I'll report back on how the latest os-prober goes in debian.


Helmut! I just realised your patch is to a script which makes things easy. I will patch it into my F23 os-prober later today and give it a test.

Hi Helmut, Hedayat

Helmut's patch fails to apply against the current os-prober-1.68-1.fc23.x86_64

It looks like much of the original patch has already gone in at some stage, so i was able to juggle things a bit and find a simple patch that addresses the last issue - when btrfs has no subvolumes.

With the attached 50mounted-tests.vs.1.68-1.fc23.x86_64.patch applied, i can get my btrfs OSes with no subvols (ie debian) into grub at last.

I hope this can be applied upstream ASAP.

Created attachment 1093187 [details]
Working patch vs 1.68-1.fc23

Ah, i've just had a look at the link Helmut suggested in comment 3

"See https://bugzilla.redhat.com/attachment.cgi?id=1044461&action=diff for easier review."

That does make it easier to see what the patch was doing, and i can see my patch also misses the case where you have a btrfs with a subvolume, but the root is in the top of the btrfs (not in a subvolume). If that's possible.

Tomorrow night I'll have another go at sticking closer to Helmut's original patch and catching this case too.

Here we go, I'm attaching a new patch to replace my first attempt. This one tests the top-level of the btrfs partition, and then continues to test the subvolumes if there are any. It also fixes a disembodied continue statement that shouldn't have been in earlier patches.

Note: the new patch of one file is all i need in order to get my btrfs debian volumes (with or without subvols) probed and into grub now.

It's been tested and works for:
 1) kubuntu style btrfs partitions (no os in the default volume, root in "@" subvolume, home in "@home" subvolume), 
 2) debian style (root in main volume, no subvolumes)
 3) "mongrel" style (root in main volume, plus extra subvolumes)

It might also work for
 4) "mongrel-bastard" style (crud in main volume, root in subvolume)
But that would depend more on whether the other scripts can cope with that, and I'm not even sure it's a bootable/feasible configuration.

Created attachment 1093594 [details]
Better patch vs os-prober 1.68-1

Thanks John. I'll try to review you and Helmut patches ASAP. Helmut: it'd be nice if you could also try John's patch.

os-prober-1.70-1.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-77188a4636

os-prober-1.70-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-067aae5bb6

os-prober-1.70-1.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update os-prober'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-77188a4636

os-prober-1.70-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update os-prober'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-067aae5bb6

John, from what I can see, your patch is identical to mine, or did you change something?
I do not have time to test exactly and diff your patch and mine but from quickly looking over my patch and yours, the only difference I notice, is that you do not indent the code in the new "else" statement, hence your patch looks shorter ...
I have not tested why my patch does not apply anymore, I do not have much time currently ... but when I do, I'll report back ...
Meanwhile it would be great if you could tell me what you changed, compared to my patch.

Yes, I also found it quite the same. The most notable difference was removing a 'continue' in your patch. Otherwise I see your patches to be quite the same.

Actually, I would do things a little differently (e.g. refactoring the repeated code into a separate function or remove the duplication in another way), but for now I applied the patch almost unmodified. I'll do that later.

BTW, thank you both of you for reporting the bug & a fix. And sorry for the late review.

Yes, the continue statement it not needed outside the for loop, happened, because of copy and paste.
Regarding functions: Yes, would be cool.
Are you interested in changing the btrfsfix.patch a little bit?

Imagine the following situation: Your root is inside a subvol @. Now you take a snapshot (not readonly) of @ called @-backup.
os-prober as of now will list @-backup additionally, despite the fact, that booting that entry would simply boot @, because the fstab of @-backup, talks of booting @.

If you would be interested in resolving that issue, then I'll see if I can find the time to open a new bug and propose my patch (which I have tested locally for quite some time) there.

Well, I'm not sure if os-prober should go that far, but it could be a nice addition. However, what if the backup boots the same / partition but different /home (or any other difference).

(In reply to Hedayat Vatankhah from comment #22)
> Yes, I also found it quite the same. The most notable difference was
> removing a 'continue' in your patch. Otherwise I see your patches to be
> quite the same.

Yep that's pretty much all i did for the 50-mounted-tests, fix the continue in Helmut's patch - though I could've sworn that when i first looked at Helmuts patch file i thought it had some other additions that had already gone into the version i was comparing it too, 1.68. But I was pretty fatigued at the time, I might've got my wires crossed with some of Helmut's patches for the other os-prober scripts...

I'm just happy this last bit has gone in to 50-mounted-tests and I'll keep my boot entries for Debian now, when i update my fedora kernels :-)

Thanks guys.

os-prober-1.70-1.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

os-prober-1.70-1.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.