Only explicitly flush devices when using libparted < 3.2 (#259)

During device probing GParted always explicitly calls ped_device_sync()
to flush the caches for coherency between the whole disk device and the
partition devices [1].  As the GParted_Core::flush_device() comment
explains, since v3.1.61-gfb99ba5 [2], libparted flushes the devices
every time a device is opened [3], so also explicitly doing this in
GParted is unnecessary.  Therefore stop explicitly flushing the devices
in GParted, except when using libparted 3.1 and older which doesn't do
it itself.

The ped_device_open() and ped_device_close() needed for the
ped_device_sync() is also a trigger of device changes and udev rule
execution performing LVM Volume Group activation, because libparted
opens the device read-write [4].  This is another reason to remove it
when possible.  However even when eliminated it does not solve the issue
of LVM VG activation because other triggers remain.  Do want this change
first though so that the sequence of libparted calls isn't changed
immediately after documenting them and fixing the issue and so that
there is no doubt that this change doesn't fix the issue.

Removing this extra device flush saves a little bit of time, depending
on the speed of the drive and number of partitions.  Sample savings on
my desktop:
    Drive type and partitions       Saving (seconds)
    -----------------------------   ----------------
    fast SSD with 3 partitions      0.05
    slow SSD with 12 partitions     0.27
    HDD with 1 partition            0.05
    VHDD in VM with 1 partition     0.14
    VHDD in VM with 10 partitions   0.58

Also the settle_device() call in flush_device() needs to be kept to wait
for device changes and udev rule execution whether it is GParted
explicitly flushing the device or just libparted automatically doing it
for cache coherency in ped_device_get().  This is because:
1.  Libparted <= 3.2 opens the whole disk device read-write for
    ped_device_get() [5].  (This was changed with parted
    v3.2.26-g44d5ae0 [6] to open the whole disk device read-only).
2.  Libparted up to and including the latest 3.6 release still opens all
    partition devices read-write for ped_device_get() [7].
3.  A whole disk device FAT32 file system looks enough like a partition
    table that both the Linux kernel and libparted think it is
    partitioned:
        # mkfs.fat -F32 /dev/sdb
        mkfs.fat 4.2 (2021-01-31)
        # grep sdb /proc/partitions
           8       16    8388608 sdb
           8       17    8388607 sdb1
        # parted /dev/sdb print
        Model: ATA VBOX HARDDISK (scsi)
        Disk /dev/sdb: 8590MB
        Sector size (logical/physical): 512B/512B
        Partition Table: loop
        Disk Flags:

        Number  Start  End        Size       File system  Flags
         1      0s     16777215s  16777216s  fat32

So the ped_device_get() call on a whole disk device FAT32 file system
still triggers device change and udev rule execution which needs to be
waited for, as this is exactly the case fixed previously by commit:
    1382e0b828
    Wait for udev change on /dev/DISK when querying whole device FS (!46)

[1] 3bea067596
    Flush devices when scanning to prevent reading stale signatures (#723842)
[2] Revert "linux-commit: do not unnecessarily open partition device nodes"
    http://git.savannah.gnu.org/cgit/parted.git/commit/?id=fb99ba5ebd0dc34204fc9f1014131d5d494805bc
[3] parted libparted/arch/linux.c:_device_open()
    https://git.savannah.gnu.org/cgit/parted.git/tree/libparted/arch/linux.c?h=v3.6#n1752
        1709 static int
        1710 linux_open (PedDevice* dev)
        1711 {
        1712     return _device_open (dev, RW_MODE);
        1713 }
        1714
        1715 static int
        1716 _device_open (PedDevice* dev, int flags)
        ...
        1752     _flush_cache (dev);

[4] parted libparted/device.c:ped_device_open() v3.6
    https://git.savannah.gnu.org/cgit/parted.git/tree/libparted/device.c?h=v3.6#n226
    parted libparted/arch/linux.c v3.6
    https://git.savannah.gnu.org/cgit/parted.git/tree/libparted/arch/linux.c?h=v3.6
        ped_device_open(...)
            ped_architecture->dev_ops->open(...) = linux_open(...)
                _device_open(..., RW_MODE)
                    open(..., O_RDWR)

[5] parted libparted/device.c:ped_device_get() v3.2
    https://git.savannah.gnu.org/cgit/parted.git/tree/libparted/device.c?h=v3.2#n149
    parted libparted/arch/linux.c v3.2
    https://git.savannah.gnu.org/cgit/parted.git/tree/libparted/arch/linux.c?h=v3.2
        ped_device_get(...)
            ped_architecture->dev_ops->_new(...) = linux_new()
                init_ide(...)
                init_scsi(...)
                init_generic(...)
                    ped_device_open(...)
                        ped_architecture->dev_ops->open(...) = linux_open(...)
                            open(..., O_RDWR)
[6] libparted: Use read only when probing devices on linux (#1245144)
    http://git.savannah.gnu.org/cgit/parted.git/commit/?id=44d5ae0115c4ecfe3158748309e9912c5aede92d
[7] parted libparted/arch/linux.v v3.6
    http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/arch/linux.c?h=v3.6#n1660
        1660 static void
        1661 _flush_cache (PedDevice* dev)
        ...
        1673     for (i = 1; i < lpn; i++) {
        ...
        1680         if (!_partition_is_mounted_by_path(name)) {
        1681             fd = open (name, WR_MODE, 0);

Closes #259 - Trying to deactivate LVM PV fails
This commit is contained in:
Mike Fleetwood 2024-08-25 20:19:09 +01:00
parent cd0ed74de9
commit 3d1b2921a6
3 changed files with 59 additions and 21 deletions

View File

@ -174,6 +174,23 @@ AS_IF(
) )
dnl Check for libparted < 3.2 which needs explicit flush for coherency between
dnl whole disk device and partition devices.
LIBPARTED_FIXED_VERSION='3.2'
AC_MSG_CHECKING([for libparted < $LIBPARTED_FIXED_VERSION (need explicit flush for device coherency workaround)])
LIBPARTED_FIXED_INT=`echo "$LIBPARTED_FIXED_VERSION" |
$AWK -F. '{print $1 * 1000000 + $2 * 10000 + $3}'`
if test "$LIBPARTED_FOUND_INT" -lt "$LIBPARTED_FIXED_INT"; then
need_explicit_flush_workaround=yes
AC_DEFINE([ENABLE_EXPLICIT_FLUSH_WORKAROUND], 1,
[Define to 1 to enable explicit flush for device coherency workaround])
AC_MSG_RESULT([(cached) yes])
else
need_explicit_flush_workaround=no
AC_MSG_RESULT([(cached) no])
fi
dnl Check for libparted >= 3.2 for online resize support. dnl Check for libparted >= 3.2 for online resize support.
LIBPARTED_WANTED_VERSION='3.2' LIBPARTED_WANTED_VERSION='3.2'
AC_MSG_CHECKING([for libparted >= $LIBPARTED_WANTED_VERSION (online resize)]) AC_MSG_CHECKING([for libparted >= $LIBPARTED_WANTED_VERSION (online resize)])
@ -377,6 +394,8 @@ echo " Need delete old partitions before"
echo " creating a loop table workaround? : $need_loop_delete_old_ptns_workaround" echo " creating a loop table workaround? : $need_loop_delete_old_ptns_workaround"
echo " Have old libparted file system resizing API? : $have_old_lp_fs_resize_api" echo " Have old libparted file system resizing API? : $have_old_lp_fs_resize_api"
echo " Have new libparted file system resizing LIB? : $have_new_lp_fs_resize_lib" echo " Have new libparted file system resizing LIB? : $have_new_lp_fs_resize_lib"
echo " Need explicit flush for device coherency"
echo " workaround? : $need_explicit_flush_workaround"
echo " Enable online resize support? : $enable_online_resize" echo " Enable online resize support? : $enable_online_resize"
echo "" echo ""
echo " If all settings are OK, type make and then (as root) make install" echo " If all settings are OK, type make and then (as root) make install"

View File

@ -222,7 +222,9 @@ private:
//general.. //general..
void capture_libparted_messages( OperationDetail & operationdetail, bool success ); void capture_libparted_messages( OperationDetail & operationdetail, bool success );
#ifdef ENABLE_EXPLICIT_FLUSH_WORKAROUND
static bool flush_device( PedDevice * lp_device ); static bool flush_device( PedDevice * lp_device );
#endif
static bool get_device( const Glib::ustring & device_path, PedDevice *& lp_device, bool flush = false ); static bool get_device( const Glib::ustring & device_path, PedDevice *& lp_device, bool flush = false );
static bool get_disk(PedDevice *lp_device, PedDisk*& lp_disk); static bool get_disk(PedDevice *lp_device, PedDisk*& lp_disk);
static bool get_device_and_disk(const Glib::ustring& device_path, static bool get_device_and_disk(const Glib::ustring& device_path,

View File

@ -4064,17 +4064,19 @@ bool GParted_Core::useable_device(const PedDevice* lp_device)
return success; return success;
} }
//Flush the Linux kernel caches, and therefore ensure coherency between the caches of the
// whole disk device and the partition devices. #ifdef ENABLE_EXPLICIT_FLUSH_WORKAROUND
// Flush the Linux kernel caches to ensure coherency between the caches of the whole disk
// device and the partition devices.
// //
// Libparted >= 2.0 works around this by calling ioctl(fd, BLKFLSBUF) to flush the cache // Libparted >= 2.0 works around this by calling ioctl(fd, BLKFLSBUF) and fsync(fd) to
// when opening the whole disk device, but only for kernels before 2.6.0. // flush the caches when opening the whole disk device, but only for kernels before 2.6.0.
// Ref: parted v3.1-52-g1c659d5 ./libparted/arch/linux.c linux_open() // Ref: parted v1.9.0-112-g2a6936f ./libparted/arch/linux.c linux_open()
// 1657 /* With kernels < 2.6 flush cache for cache coherence issues */ // 1578 /* With kernels < 2.6 flush cache for cache coherence issues */
// 1658 if (!_have_kern26()) // 1579 if (!_have_kern26())
// 1659 _flush_cache (dev); // 1580 _flush_cache (dev);
// //
// Libparted >= v3.1-61-gfb99ba5 works around this for all kernel versions. // Libparted >= 3.2 works around this for all kernel versions.
// Ref: parted v3.1-61-gfb99ba5 ./libparted/arch/linux.c linux_open() // Ref: parted v3.1-61-gfb99ba5 ./libparted/arch/linux.c linux_open()
// 1640 _flush_cache (dev); // 1640 _flush_cache (dev);
bool GParted_Core::flush_device( PedDevice * lp_device ) bool GParted_Core::flush_device( PedDevice * lp_device )
@ -4084,15 +4086,11 @@ bool GParted_Core::flush_device( PedDevice * lp_device )
{ {
success = ped_device_sync( lp_device ) ; success = ped_device_sync( lp_device ) ;
ped_device_close( lp_device ) ; ped_device_close( lp_device ) ;
// (!46) Wait for udev rules to complete after this ped_device_open() and
// ped_device_close() pair to avoid busy /dev/DISK entry when running file
// system specific querying commands on the whole disk device in the call
// sequence after get_device() in set_device_from_disk().
settle_device(SETTLE_DEVICE_PROBE_MAX_WAIT_SECONDS);
} }
return success ; return success ;
} }
#endif
bool GParted_Core::get_device( const Glib::ustring & device_path, PedDevice *& lp_device, bool flush ) bool GParted_Core::get_device( const Glib::ustring & device_path, PedDevice *& lp_device, bool flush )
{ {
@ -4100,11 +4098,30 @@ bool GParted_Core::get_device( const Glib::ustring & device_path, PedDevice *& l
if ( lp_device ) if ( lp_device )
{ {
if ( flush ) if ( flush )
// Force cache coherency before going on to read the partition {
// table so that libparted reading the whole disk device and the #ifdef ENABLE_EXPLICIT_FLUSH_WORKAROUND
// file system tools reading the partition devices read the same // Force cache coherency explicitly ourselves with libparted < 3.2
// data. // which doesn't flush the Linux kernel caches when opening
// devices. This is so that libparted reading the whole disk
// device and file system tools reading the partition devices read
// the same data.
flush_device( lp_device ); flush_device( lp_device );
#endif
// (!46) Wait for udev rules to complete after either GParted
// explicit device flush or libparted inbuilt device flush in
// ped_device_get(). This is to avoid busy /dev/DISK entry when
// running file system specific query commands on the whole disk
// device in the call sequence after get_device(..., flush=true)
// in set_device_from_disk().
//
// This is still needed even with libparted 3.6 because a whole
// disk device FAT32 file system looks like it's partitioned and
// ped_device_get() still opens partition devices read-write when
// flushing, so still triggers device changes and udev rule
// execution.
settle_device(SETTLE_DEVICE_PROBE_MAX_WAIT_SECONDS);
}
return true; return true;
} }