After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 754684 - Updates to FileSystem:: and Utils::execute_command() functions
Updates to FileSystem:: and Utils::execute_command() functions
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
GIT HEAD
Other Linux
: Normal normal
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2015-09-07 14:26 UTC by Mike Fleetwood
Modified: 2015-10-27 17:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Execute commands updates (v1) (64.32 KB, patch)
2015-09-08 16:34 UTC, Mike Fleetwood
none Details | Review

Description Mike Fleetwood 2015-09-07 14:26:46 UTC
Found a few inconsistencies with the FileSystem:: and
Utils::execute_command() functions so I'm doing some tidy ups.  Some of
the issues are:
1) Some commands used to manipulate file systems are timed and show a
   check mark in the operation details and others don't.
2) Return codes from these functions are not the command exit statuses.
3) Some bits of ununsed code.

Mike
Comment 1 Mike Fleetwood 2015-09-08 16:34:24 UTC
Created attachment 310918 [details] [review]
Execute commands updates (v1)

Hi Curtis,

Here's the patchset for this.

For testing I did the following ...

On Ubuntu 14.04 LTS ran every file system operation which runs external
commands:

btrfs      - create, grow, shrink, check, label, UUID[1]
ext4       - create, grow, shrink, move(e2image), copy(e2image), check,
             label, UUID
fat32      - create, check[2], label, UUID
hfs        - create, check
hfsplus    - create, check
jfs        - create, grow, check, label, UUID
linux-swap - create, grow(recreate), label, UUID
lvm2 pv    - create, grow, shrink, check, delete
nilfs2     - create, grow[3], shrink[3], label, UUID
ntfs       - create, grow, shrink, copy(ntfsclone), check, label, UUID
reiser4    - create, check
resierfs   - create, grow, shrink, check, label, UUID
xfs        - create, grow, copy (xfsdump|xfsrestore), check, label, UUID

[1] btrfs UUID - Ran successfully on Fedora 22 with btrfs-progs 4.1 for
    required btrfstune with -u option.
[2] fat32 check - Ran successfully on Fedora 22 and CentOS 6.
    Crashes GParted on Ubuntu 14.04 LTS using GParted HEAD and
    GParted 0.23.0.  Resizing fat32 also crashes.  Assume this is a
    libparted version / distro specific issue.
[3] nilfs2 resize - After resizing the file system the umount steps
    fails because it is still busy.  Correctly detected and reported.
    Fails because the temporary mount is busy.  Have to manually
    "killall nilfs_cleanerd" before it can be unmounted.

Also performed these specific tests for non-default the shell exit
status handling cases:

1) Resize btrfs to same size on pre 3.2 kernel.
   On Fedora 14 (with kernel 2.6.35), check btrfs.
   Performs "btrfs filesystem resize 1:max /mnt" and gets exit status 30
   reporting unable to resize as documented in the code.  Accepted by
   the code as success.
2) Check ext4 file system needing repair.
   Mount an ext4 FS, write a file to it, sync.  While it's still mounted
   use dd to copy it to another partition.  This creates a file system
   which e2fsck will modify and report exit status 1.  Perform a check
   of the file system in GParted.  Accepted as a success.

Also performed this specific test of the change from bool cancel_safe
parameter to bitwise flag EXEC_CANCEL_SAFE:

1) Use GParted to copy populated XFS file system and cancel when it's
   running the xfsdump | xfsrestore command.  Cancels immediately as
   EXEC_CANCEL_SAFE implies, rather than doing the 5 second countdown to
   force cancel.


Thanks,
Mike
Comment 2 Mike Fleetwood 2015-09-09 11:49:45 UTC
Now re-tested Ubuntu 14.04 LTS, with local install of libparted 3.2 with
GParted HEAD.  Resizing of fat32 works correctly.  Definitely a
libparted version / distro specific issue.

Mike
Comment 3 Mike Fleetwood 2015-09-13 20:33:42 UTC
The crash when resizing fat32 with libparted 2.3 will be this bug again:
Bug 735471 - fat32 resize crash - error "corrupted double-linked list"
Comment 4 Curtis Gedak 2015-09-21 17:29:46 UTC
Thank you Mike for this cleanup work on FileSystem:: and
Utils::execute_command() functions.

On Kubuntu 12.04 LTS I ran the same commands as noted in comment #1
testing:

btrfs      - create, grow, shrink, check, label[1], UUID[1]
ext4       - create, grow, shrink, move(e2image), copy(e2image), check,
             label, UUID
fat32      - create, check, label, UUID
hfs        - create, check[1]
hfsplus    - create, check
jfs        - create, grow, check, label, UUID
linux-swap - create, grow(recreate), label, UUID
lvm2 pv    - create, grow, shrink, check, delete
nilfs2     - create, grow, shrink, label, UUID
ntfs       - create, grow, shrink, copy(ntfsclone), check, label, UUID
reiser4    - create, check
resierfs   - create, grow, shrink, check, label, UUID
xfs        - create, grow, copy (xfsdump|xfsrestore), check, label, UUID

[1] Operation not available with kubuntu 12.04 LTS packages.

All of these above tests executed as expected, and the code looks good
to me.

Patch set v1 in comment #1 has been committed to the git repository
for inclusion in the next release of GParted.

The relevant git commits can be viewed at the following links:

Remove unused execute_command_timed() (#754684)
https://git.gnome.org/browse/gparted/commit/?id=d3e17f448482332d99f103cccf0fa51fc6d03934

Refactor flags in method FileSystem::execute_command() (#754684)
https://git.gnome.org/browse/gparted/commit/?id=83ecae49181a572e2bee67b73e290ed7ab2f62e7

Time and check nearly all file system action commands (#754684)
https://git.gnome.org/browse/gparted/commit/?id=3eccd01f428723bea90db1c3917a8278d7a1d2c0

Time and check commands setting fat16/32 labels and UUIDs (#754684)
https://git.gnome.org/browse/gparted/commit/?id=a202b4569a11455adf3ed15dc47f1ee13fd67cb4

Implement shell style exit status decoding (#754684)
https://git.gnome.org/browse/gparted/commit/?id=2b57229fc21e6bd12ba6d50ea451691ec7408a11

Remove unused public member variable FileSystem::success
https://git.gnome.org/browse/gparted/commit/?id=0e32d39189ebd7be88358b09df20756e3278963d
Comment 5 Curtis Gedak 2015-10-27 17:02:54 UTC
This enhancement was included in the GParted 0.24.0 release on October 27, 2015.