GNOME Bugzilla – Bug 766349
Resolve code ugliness with partition path getting set to "copy of /dev/SRC"
Last modified: 2016-06-13 17:30:37 UTC
Quoting the relevant comments from GParted_Core::calibrate_partition(): Re-add the real partition path from libparted. When creating a copy operation the list of paths for the partition object was set to ["copy of /dev/SRC"] to display in the UI before the operations are applied. When pasting into an existing partition, this re-adds the real path to the start of the list making it ["/dev/DST", "copy of /dev/SRC"]. This provides the real path for any file system specific tools needed during the copy operation. Such as file system check and grow steps added when the source and destination aren't identical sizes or when file system specific tools are used to perform the copy as for XFS or recent EXT2/3/4 tools. FIXME: Having this work just because "/dev/DST" happens to sort before "copy of /dev/SRC" is ugly! Probably have a separate display path which can be changed at will without affecting the list of real paths for the partition. I'm working on fixing this. Mike
Created attachment 327945 [details] [review] Resolve code ugliness with temporary path "copy of /dev/SRC" (v1) Hi Curtis, Here's patchset v1 to fix this. Initially I tried adding the concept of display_path into the the Partition object. But that's adding another type of path and paths are already difficult enough to handle now; with Devices and Partitions having a list of paths, the first according to sort order, is special becuase that is used everywhere and passed to the FS tools. The secondary paths are only use for mount point detection. So after we chatted and I said I would only use "Copy of /dev/SRC" for the partition path when pasting into unallocated space, and keep the path when pasting into existing partitions, I realised there was a much simpler way to fix this. Only in the case of the path being "Copy of /dev/SRC" replace the list of paths with what libparted reports when calibrating; otherwise just add what libparted reports if not already the primary path. My set of test cases for this has been: 1) Paste into unallocated space creating "Copy of /dev/SRC" Format "Copy of /dev/SRC" Apply 2) Paste into existing partition /dev/sdb2 Format /dev/sdb2 Apply 3) Paste into unallocated space creating "Copy of /dev/SRC" Resize/Move "Copy of /dev/SRC" Apply 4) Paste into existing partition /dev/sdb2 Resize/Move /dev/sdb2 Apply In addition to the testing described in patch number 1. Testing any weird combinations involving copy-paste is welcomed. Thanks, Mike
Hi Mike, I'm in the process of migrating to kubuntu 16.04 so I haven't yet started my testing of patch set v1. From a quick code review I noticed one typo. In the comment of the 3 of 4 patch: ...Copy the whole path list is the correct action, It makes ^ Should be: ...Copy the whole path list is the correct action. It makes If the patch set passes my testing then I can make this minor update before committing. Curtis
Hi Mike, In my testing, patch set (v1) from comment #1 behaved as expected and corrected the "copy of ... copy of" partition path issue. This patch set has been committed to the git master branch. The relevant git commits can be viewed at the following links: Stop overriding real path when pasting into existing partitions (#766349) https://git.gnome.org/browse/gparted/commit/?id=302cc8041e867d8fe9ad45c9d8b2d042a6cfbc74 Stop relying on sort order when adding real paths in calibrate (#766349) https://git.gnome.org/browse/gparted/commit/?id=b77fef0dd5a85277210ca0cced77cc8abba99c8f Replace whole path list after calibrate in apply_operation_to_disk() (#766349) https://git.gnome.org/browse/gparted/commit/?id=ab923121defaaf847225c772e68ed11ab7377ad1 Remove outdated comment fragment from activate_mount_partition() https://git.gnome.org/browse/gparted/commit/?id=29be7547cd0761cc3a9c58d2c10c77add06e159a Curtis
This enhancement was included in the GParted 0.26.1 release on June 13, 2016.