GNOME Bugzilla – Bug 757781
Always use blkid file system detection before libparted
Last modified: 2016-01-18 17:41:44 UTC
I've been thinking that GParted should use blkid detection before libparted for a while now. Limitations with libparted detection have always been worked around in the past so we were never forced to switch the order. Do that now. Having a separate bug report so that the commit is tagged with that bug number and any required discussion can be looked up later.
Hi Mike, Following is a note of caution regarding the importance of libparted recognizing certain file systems. When resizing FAT16/32 and HFS/+ file systems it is critical that libparted recognize the file system as such. If only blkid recognizes these file systems then libparted will be unable perform the resize. Unfortunately I do not have a sample file system that demonstrates this issue, but I have seen it happen at least once before. Curtis
Hi Curtis, This is exactly the sort of discussion we need to have so that we can get to the bottom of any issues. Assuming a case where libparted does not recognise a file system as either FAT16/32 or HFS/+ but blkid does. A) With the existing code libparted detection is tried first and fails. Blkid detection is tried second and succeeds. Then when resizing in GParted it requests libparted to do the resizing but will presumably fail because libparted doesn't recognise the file system. B) With the new code blkid detection is tried first and succeeds. Libparted detection is never tried. Then when resizing in GParted it requests libparted to do the resizing but will presumably fail because libparted doesn't recognise the file system. So with either code GParted will behave the same, regardless of whether blkid or libparted is first choice for detection. Thanks, Mike
If both (A) and (B) react the same, then this change would not introduce new problems. Having said that it would be better for the user if we detected a mismatch in file system recognition that would cause a failure when the operation is applied. In such situations it might be better to notify the user of the mismatch. On thinking about FS recognition mismatch, I have seen cases where blkid recognizes a file system as one type and libparted sees it as another. This is similar to the mismatch problem you addressed in Bug 756829 - SWRaid member detection enhancements. We'll have to keep an eye out for these situations. I'm not saying that having libparted perform the file system recognition first is the correct way to proceed. Ideally we would know exactly what the format on disk was most recently. But since that is not the case we should try to take the path of least chance of data loss. Overall since the file system recognition code in libparted is languishing, I agree that switching to blkid FS recognition is the correct path forward. Curtis
Created attachment 315091 [details] [review] Use blkid detection before libparted (v1) Hi Curtis, I've wanted to do this since I worked on this Red Hat bug report: Minor text parsing error https://bugzilla.redhat.com/show_bug.cgi?id=1081624 And inparticular comment number 12 where I summarised the problem as partition contained an ext4 file system, created SWRaid over the top resulting in blkid reporting swraid member and libparted reporting ext4. I said GParted should use blkid identification before libparted to fix that case. As you identified above, bug 756829 "SWRaid member detection enhancements" is about equivalent cases of mis-identification of partition content. In the end bug 756829 was fixed without using blkid detection before libparted. I still think it is the right way. Anyway as we're in agreement, here's the patch for this. Mike
Hi Mike, Patch v1 in comment #4 looks good to me. The only minor change I suggest is adding the word "for" in the commit message. This was the primary reason for bug 688882 "Improve clearing ^^^ of file system signatures" and a number of other changes to GParted. I can make this change prior to committing. If there are no objections I will commit this change tomorrow. Curtis
Created attachment 315149 [details] [review] Use blkid detection before libparted (v2) Hi Curtis, Here's the patch with "for" added to the commit message, and the paragraph re-line-wrapped at column 72. (Bit of OCD kicking in :-). Mike
FYI: This is how I format commit messages when I remember everything Line wrap composed lines after column 72. Including quoted text and document fragments. Git use do make Vim do this automatically but doesn't seem to any more. For quoted command line output generally don't wrap. When including GParted commit IDs just include SHA1 value. Both gitk and CGit on http://git.gnome.org/ convert them into local clickable links. When quoting a commit ID of another package use the full Git web URL so that, at least to humans, it is clear it is a foreign commit ID. (gitk and cgit still make just the SHA1 clickable to a non-existent local commit though). Then the usual header and footer we use. One line summary (#XXXXXX) : Bug XXXXXX - Bug summary
Thanks Mike for describing the tips you use to format git commit messages. I added these formatting tips to the "Developing GParted using Git" web page. http://gparted.org/git.php Thanks also for the updated patch v2. Patch v2 from comment #6 has been committed to the git repository for inclusion in the next release of GParted. The relevant git commit can be viewed at the following link: Always use blkid file system detection before libparted (#757781) https://git.gnome.org/browse/gparted/commit/?id=6e97a63f49016ea6b0b48f37aedb10fe6ec808bb
This enhancement was included in the GParted 0.25.0 release on January 18, 2016.