GNOME Bugzilla – Bug 754649
Display Logical Volumes in Volume Group of LVM2 Partition
Last modified: 2015-10-27 17:05:24 UTC
Created attachment 310770 [details] Patch file to implement functionality Display Logical Volumes in Volume Group of LVM2 Partition
Hi Wrolf, Thank you for this patch. I have given the code a test and the dialog definitely needs the heading 'Logical Volumes:' adding so that the user knowns what is being reported. Otherwise OK. Thanks, Mike
Created attachment 310771 [details] [review] Improved with "Logical Volumes" heading Added "Logical Volume:" header
(In reply to Mike Fleetwood from comment #1) > Hi Wrolf, > > Thank you for this patch. I have given the code a test and the dialog > definitely needs the heading 'Logical Volumes:' adding so that the user > knowns what is being reported. Otherwise OK. > > Thanks, > Mike Done.
Created attachment 310772 [details] Screenshot
Created attachment 310803 [details] Screen shots of extra blank lines with 2 members, before and after Hi Wrolf, With the new code I did a bit more testing and found a few issues: 1) Now also displays Logical Volumes heading for btrfs file systems. Not wanted. In ::Display_Info() need to put the display of logical volumes into a separate if ( partition.filesystem == FS_LVM2_PV ) as the one for displaying members is shared by LVM2 PV and BTRFS. 2) There are extra blank lines appearing in the display. The more the number of members in the VG the more the blank lines. (See attached before and after screen shot). Not sure what is causing this though. Please also squash the two commits into one. (Each code increment pushed into the public repo needs to be finished and working correctly). Thanks, Mike
Created attachment 310844 [details] [review] Improved patch to handle case of LVM2 partitions not VG
Created attachment 310854 [details] [review] Improved patch to handle case of VGs with multiple PVs
Hi Wrolf, 1) Now the Members and Logical Volumes labels are disappearing when there is no data to display in those fields. The labels must always be displayed (for the relevant file system type) so users know the values are blank. 2) Still getting one blank line in the list of logical volumes. This is because of the content of the lvm_vg_cache. (See example near the top of the LVM2_PV_Info.cc file). Need to exclude adding the empty LV name in get_vg_lvs(). Thanks, Mike
Created attachment 311372 [details] [review] Improved patch to eliminate bogus blank lines, always display Members and LVs headers
Created attachment 311406 [details] [review] Display Logical Volumes (v6) Hi Wrolf, The code works well now. Rather than make you jump unnecessary hoops I'm made a few cosmetic changes: 1) Remove unnecessary changes in Display_Info() for display of Members. Helps make using git blame more useful / easier. 2) Added and removed an occasional space character. GParted code is a mess in terms of white space consistency. Trying to set a consistent layout going forward. 3) Minor comment rewording. The following commit has been pushed to the public GParted GIT repo and will be included in the next release of GParted. Display list of Logical Volumes in the Partition Information dialog (754649) https://git.gnome.org/browse/gparted/commit/?id=9e950e89b42158753c47bb846336e83b4bf3c100 Thanks, Mike
Hi Wrolf, Thank you for helping to improve GParted. We like to provide credit where credit is due. Would you mind if we add your name and email address to the files listed below in order to recognize your contribution? AUTHORS file: https://git.gnome.org/browse/gparted/tree/AUTHORS Help -> About dialog: https://git.gnome.org/browse/gparted/tree/src/Win_GParted.cc?id=GPARTED_0_23_0#n1631 Thanks, Curtis
(In reply to Curtis Gedak from comment #11) > Hi Wrolf, > > Thank you for helping to improve GParted. We like to provide credit where > credit is due. > > Would you mind if we add your name and email address to the files listed > below in order to recognize your contribution? > > > AUTHORS file: > https://git.gnome.org/browse/gparted/tree/AUTHORS > > Help -> About dialog: > https://git.gnome.org/browse/gparted/tree/src/Win_GParted. > cc?id=GPARTED_0_23_0#n1631 > > Thanks, > Curtis Thank you, love to be added, it's only 43 lines of code, but thanks for the recognition. Wrolf Courtney wrolf@wrolf.net
Created attachment 311704 [details] [review] Add recognition for patch v1 Hi Mike, When you have time please review this patch for recognition of work on GParted. Thanks, Curtis
Hi Curtis, These recognition patches look fine. As such I have pushed the following commits upstream into the GParted Git repo. I've made a cosmetic update to the first commit message: Add bug number to the summary line and line wrap the commit body text at column 72. (If the commit message rates a bug description footer the bug number should be at the end of the summary line). Thank, Mike Provide credit for patch by Wrolf Courtney (#754649) https://git.gnome.org/browse/gparted/commit/?id=dbc81f1537cf7bfd538d2306c84b860333b527a3 Provide overall credit for work done by Mike Fleetwood https://git.gnome.org/browse/gparted/commit/?id=7fc3e816cae95014c19e9b554db07f7f1aff6895
This enhancement was included in the GParted 0.24.0 release on October 27, 2015.