From d86d9ae8300605e5c1f8744224b5559c0301fe12 Mon Sep 17 00:00:00 2001 From: Mike Fleetwood Date: Wed, 27 Jan 2021 22:05:43 +0000 Subject: [PATCH] Remove extra execution of blkid for any unreported paths (#131) Again on Fedora 31 with a slightly different disk layout to the previous commit. sdb is partitioned with 1 empty partition and sdc remains completely empty: # lsblk -o name,maj:min,rm,size,ro,type,fstype,label,mountpoint NAME MAJ:MIN RM SIZE RO TYPE FSTYPE LABEL MOUNTPOINT sda 8:0 0 20G 0 disk |-sda1 8:1 0 1G 0 part ext4 /boot \-sda2 8:2 0 19G 0 part LVM2_member |-fedora-root 253:0 0 17G 0 lvm ext4 / \-fedora-swap 253:1 0 2G 0 lvm swap [SWAP] sdb 8:16 0 8G 0 disk \-sdb1 8:17 0 1G 0 part sdc 8:32 0 8G 0 disk sr0 11:0 1 1024M 0 rom # blkid -v blkid from util-linux 2.34 (libblkid 2.34.0, 14-Jun-2019) # blkid /dev/sda /dev/sda1 /dev/sda2 /dev/sdb /dev/sdb1 /dev/sdc /dev/sda: PTUUID="5012fb1f" PTTYPE="dos" /dev/sda1: UUID="3cd48816-7817-4636-9fec-5f1afe76c1b2" TYPE="ext4" PARTUUID="5012fb1f-01" /dev/sda2: UUID="PH94ej-C8xU-bnMJ-UIh8-ZimI-4B7f-dHlZxh" TYPE="LVM2_member" PARTUUID="5012fb1f-02" /dev/sdb: PTUUID="1d120b57" PTTYPE="dos" /dev/sdb1: PARTUUID="1d120b57-01" Stracing GParted shows these executions of blkid: # strace -f -q -bexecve -eexecve ./gpartedbin 2>&1 1> /dev/null | egrep -v 'ENOENT|SIGCHLD' ... [pid 160040] execve("/usr/sbin/blkid", ["blkid", "/dev/sda", "/dev/sda1", "/dev/sda2", "/dev/sdb", "/dev/sdb1", "/dev/sdc"], 0xa4e1b0 /* 32 vars */ [pid 160041] execve("/usr/sbin/blkid", ["blkid", "/dev/sdc"], 0xa4e1b0 /* 32 vars */ ... On Fedora 31 with blkid from util-linux 2.34 it reports information for sdb (partitioned drive) and sdb1 (empty partition) with only no information for sdc (empty whole disk drive). Hence no FS_Info cache entry and re-execution of blkid just for sdc. On older CentOS 7 with the same disk layout blkid reports this: # blkid -v blkid from util-linux 2.23.2 (libblkid 2.23.0, 25-Apr-2013) # blkid /dev/sda /dev/sda1 /dev/sda2 /dev/sdb /dev/sdb1 /dev/sdc /dev/sda: PTTYPE="dos" /dev/sda1: UUID="e7d559e4-3e1d-4fbc-b034-3fdeb498f44d" TYPE="xfs" /dev/sda2: UUID="B7ODFx-BfTE-hq7N-UlrF-f5ML-CPRe-klSy26" TYPE="LVM2_member" /dev/sdb: PTTYPE="dos" And stracing GParted shows these executions of blkid: # strace -f -q -bexecve -eexecve ./gpartedbin 2>&1 1> /dev/null | egrep -v 'ENOENT|SIGCHLD' ... [pid 1889] execve("/sbin/blkid", ["blkid", "/dev/sda", "/dev/sda1", "/dev/sda2", "/dev/sdb", "/dev/sdb1", "/dev/sdc"], 0x10b8b10 /* 26 vars */ [pid 1890] execve("/sbin/blkid", ["blkid", "/dev/sdb1"], 0x10b8b10 /* 26 vars */ [pid 1891] execve("/sbin/blkid", ["blkid", "/dev/sdc"], 0x10b8b10 /* 26 vars */ ... This time on CentOS 7 with blkid from util-linux 2.23.2 it reports information for only sdb (partitioned drive), but not sdb1 (empty partition) or sdc (empty whole disk drive). Hence no FS_info cache entries and re-execution of blkid for both sdb1 and sdc. GParted needs blkid identification of file system images, LVM Logical Volumes or any other partitions named on the command line which it wouldn't normally scan [1]. Now every name of interest is passed to blkid, additional executions of blkid won't get any extra information and are redundant. Therefore remove this unnecessary code. Note that these last 2 commits remove creation of "blank" cache entries (just block special with blank fstype and other attributes) when blkid reports no information for a particular path. Those entry were needed to suppress unnecessary additional execution of blkid. However now that blkid is only executed once (excluding querying the label) this is no longer necessary. All the getter functions return suitable blank values when no cache entry is found. [1] e8f0504b13d98e23e70021f70638184daab228ec Make sure that FS_Info cache is loaded for all named paths (#787181) Closes #131 - GParted hangs when non-named device is hung --- include/FS_Info.h | 1 - src/FS_Info.cc | 30 ++---------------------------- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/include/FS_Info.h b/include/FS_Info.h index 1bc56144..44e14a28 100644 --- a/include/FS_Info.h +++ b/include/FS_Info.h @@ -50,7 +50,6 @@ private: static void set_commands_found(); static const FS_Entry & get_cache_entry_by_path( const Glib::ustring & path ); static void load_fs_info_cache(const std::vector& paths); - static void load_fs_info_cache_extra_for_path( const Glib::ustring & path ); static bool run_blkid_load_cache(const std::vector& paths); static void update_fs_info_cache_all_labels(); static bool run_blkid_update_cache_one_label( FS_Entry & fs_entry ); diff --git a/src/FS_Info.cc b/src/FS_Info.cc index 750a2d49..fbd754f9 100644 --- a/src/FS_Info.cc +++ b/src/FS_Info.cc @@ -55,19 +55,9 @@ void FS_Info::load_cache_for_paths(const std::vector& paths) set_commands_found(); load_fs_info_cache(paths); fs_info_cache_initialized = true; - - const BlockSpecial empty_bs = BlockSpecial(); - for (unsigned int i = 0; i < paths.size(); i++) - { - const FS_Entry& fs_entry = get_cache_entry_by_path(paths[i]); - if ( fs_entry.path == empty_bs ) - { - // Run "blkid PATH" and load entry into cache for missing entries. - load_fs_info_cache_extra_for_path(paths[i]); - } - } } + // Retrieve the file system type for the path Glib::ustring FS_Info::get_fs_type( const Glib::ustring & path ) { @@ -112,8 +102,7 @@ Glib::ustring FS_Info::get_label( const Glib::ustring & path, bool & found ) { // Already have the label or this is a blank cache entry // for a whole disk device containing a partition table, - // so no label (as created by - // load_fs_info_cache_extra_for_path()). + // so no label. found = fs_info_cache[i].have_label; return fs_info_cache[i].label; } @@ -213,21 +202,6 @@ void FS_Info::load_fs_info_cache(const std::vector& paths) } -void FS_Info::load_fs_info_cache_extra_for_path( const Glib::ustring & path ) -{ - std::vector one_path; - one_path.push_back(path); - bool entry_added = run_blkid_load_cache(one_path); - if ( ! entry_added ) - { - // Ran "blkid PATH" but didn't find details suitable for loading as a - // cache entry so add a blank entry for PATH name here. - FS_Entry fs_entry = {BlockSpecial( path ), "", "", "", false, ""}; - fs_info_cache.push_back( fs_entry ); - } -} - - bool FS_Info::run_blkid_load_cache(const std::vector& paths) { // Parse blkid output line by line extracting mandatory field: path and optional