Bug 1236358 - os-prober duplicates grub entries for read/write btrfs subvolumes
Summary: os-prober duplicates grub entries for read/write btrfs subvolumes
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: os-prober
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Hedayat Vatankhah
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-06-28 07:53 UTC by Helmut Horvath
Modified: 2016-03-02 08:23 UTC (History)
2 users (show)

Fixed In Version: os-prober-1.70-1.fc23 os-prober-1.70-1.fc22
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-11-17 11:58:11 UTC
Type: Bug


Attachments (Terms of Use)
patch (841 bytes, patch)
2015-06-28 07:53 UTC, Helmut Horvath
no flags Details | Diff
new btrfs patch for easier review (14.54 KB, patch)
2015-06-29 17:24 UTC, Helmut Horvath
no flags Details | Diff
Full btrfsfix patch for easier review (14.45 KB, patch)
2015-06-29 20:16 UTC, Helmut Horvath
no flags Details | Diff

Description Helmut Horvath 2015-06-28 07:53:18 UTC
Created attachment 1043992 [details]
patch

Description of problem:

This bug is against a downstream patch of fedora: http://pkgs.fedoraproject.org/cgit/os-prober.git/tree/os-prober-btrfsfix.patch

The section of excluding snapshot subvolumes is broken, because the parsing of btrfs output is wrong (cut -d ' ' -f 9), at least as of btrfs-tools 3.17-1.1.

The output of btrfs subvolume list -s "$tmpmnt" has more rows than the output of the other btrfs subvolume list outputs, but whats common is, that the subvolume name is the very right row.

This means, that if you have many snapshots on a btrfs partition, that are not marked ro, os-prober also returns all these snapshots as subvolumes, which leads to duplicated grub entries on my debian system.


Additional info:

Hedayat Vatankhah, are you still interested in trying to upstreaming the btrfs patch? I think you should try again. I mean I can ping there too again, but I do not have your expertise to respond in case of a code review.
It seems, the original author (Gene Czarcinski) of the btrfs patch has disappeared, I have not been able to reach him for quite some time, his website is down too.
Upstream bug report: http://bugs.debian.org/688336

On another note: You should consider that patch too: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=688336#35

I have tested my patch on debian/ubuntu systems, I do not have a fedora system anymore, but I want to contribute back to the fedora os-prober maintainers, since I use your patches.

Comment 1 Hedayat Vatankhah 2015-06-29 11:44:20 UTC
Hi Helmut,
Thanks for your detailed report, plus the patch. It seems that I should create some btrfs volumes finally (till now, Gene took care of btrfs related issues; I should contact him to see if I can receive any replies). 

To be honest, I'm not much more familiar with this patch either (since I didn't have btrfs volumes); so I'm not sure if I can help with code review more than you. However, if they were really interested to pick up the patch, I would try my best to help. But I've host all my hope over os-prober upstream; since I've opened many bugs for our patches but they've hardly responded to even simple ones (except a few one liner patches). 

Anyway, thank you for your support. I'll review your patch and the other patch you mentioned. I should set-up some sample btrfs volumes though. Then I might ping upstream maintainers again, while I don't have much hope about it. :P

Comment 2 Helmut Horvath 2015-06-29 16:23:03 UTC
For the other patch, please see https://bugzilla.redhat.com/show_bug.cgi?id=1236649
I came up with my own patch, since the other given one, worked, but had a bug.

Comment 3 Helmut Horvath 2015-06-29 17:24:52 UTC
Created attachment 1044429 [details]
new btrfs patch for easier review

Comment 4 Helmut Horvath 2015-06-29 17:31:59 UTC
attachment 1044429 [details] is the new diff, that also includes the changes for https://bugzilla.redhat.com/show_bug.cgi?id=1236649

Comment 5 Helmut Horvath 2015-06-29 20:16:24 UTC
Created attachment 1044461 [details]
Full btrfsfix patch for easier review

Covering this patch and the patch for bug 1236649.

Comment 6 Jan Kurik 2015-07-15 13:54:01 UTC
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

Comment 7 Hedayat Vatankhah 2015-11-11 19:26:10 UTC
Helmut, have I correctly understood that your actual patch is https://bugzilla.redhat.com/attachment.cgi?id=1043992&action=diff ?
(Actually, 'full' patch is harder for review. The best kind of patch for review is the one applied on the patched code, like the one you've provided in bug #1236649).
Thanks.

Comment 8 Helmut Horvath 2015-11-11 20:24:58 UTC
Yes ... One thing I do not like of my solution is the fact I use awk, which is not used anywhere else in os-prober, but this shouldn't be a problem ... I do not know how likely the output of the btrfs command is to change, but if it does change, the awk solution is certainly less likely to break than the sed solution with the hard coded position (with awk we simply get the last field).

The Opensuse guys have silently fixed this without reporting back to Fedora it seems:

https://build.opensuse.org/package/view_file/openSUSE:Factory/os-prober/os-prober-btrfsfix.patch?expand=1

see line 317.
So if you would like the fix to be minimal, it should be enough to replace 9 by 14 in one line, can't check right now though and I do not remember anymore ...

Comment 9 Hedayat Vatankhah 2015-11-13 18:49:45 UTC
Thanks! For now, I decided to replace 9 by 14, but will keep your solution in mind.

Comment 10 Fedora Update System 2015-11-13 20:24:32 UTC
os-prober-1.70-1.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-77188a4636

Comment 11 Fedora Update System 2015-11-14 03:21:36 UTC
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

Comment 12 Fedora Update System 2015-11-14 04:52:41 UTC
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

Comment 13 Fedora Update System 2015-11-17 11:58:04 UTC
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.

Comment 14 Fedora Update System 2016-03-02 08:23:30 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.