Hide Forgot
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.
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
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.
Created attachment 1044429 [details] new btrfs patch for easier review
attachment 1044429 [details] is the new diff, that also includes the changes for https://bugzilla.redhat.com/show_bug.cgi?id=1236649
Created attachment 1044461 [details] Full btrfsfix patch for easier review Covering this patch and the patch for bug 1236649.
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
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.
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 ...
Thanks! For now, I decided to replace 9 by 14, but will keep your solution in mind.
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.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
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.