[gparted] Wait for udev to recreate /dev/PTN entries when querying partition FSs (!46)



commit f170a1e542bcaccccfde4009eda6df596dabe166
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sat Jul 13 15:46:58 2019 +0100

    Wait for udev to recreate /dev/PTN entries when querying partition FSs (!46)
    
    After the previous fix, on my CentOS 7 VM, GParted is now often
    reporting a warning that the /dev/PTN entry is missing when reading the
    label of the file system in the first partition.  This happens
    regardless of the file system type.
    
    Example error for EXT4:
        e2label: No such file or directory while trying to open /dev/sdb1
        Couldn't find valid file system superblock.
    
    Example error for XFS:
        /dev/sdb1: No such file or directory
        fatal error -- couldn't initialize XFS library
    
    As with the case in the previous commit, GParted gets the label via
    blkid instead.
    
    Again with debugging and sleeping in GParted, stracing the GParted
    thread GParted_Core::set_devices_thread() and using 'udevadm monitor' to
    watch udev events the following sequence of events is observed:
    
      gparted    |set_devices_thread(pdevices)  pdevices->["/dev/sdb"]
      gparted    |  ped_device_get("/dev/sdb")
      libparted  |      open("/dev/sdb", O_RDONLY) = 8
      libparted  |      close(8)
      gparted    |  useable_device(lp_device)  lp_device->path="/dev/sdb"
      gparted    |    ped_device_open()
      libparted  |      open("/dev/sdb", O_RDWR) = 8
      gparted    |    ped_device_read()
      gparted    |    ped_device_close()
      libparted  |      close(8)
      udev(async)|        KERNEL remove /devices/.../sdb/sdb1 (block)
      udev(async)|        KERNEL remove /devices/.../sdb/sdb2 (block)
      udev(async)|        KERNEL change /devices/.../sdb (block)
      udev(async)|        KERNEL add    /devices/.../sdb/sdb1 (block)
      udev(async)|        KERNEL add    /devices/.../sdb/sdb2 (block)
      udev(async)|        UDEV   remove /devices/.../sdb/sdb1 (block)
      udev(async)|        UDEV   remove /devices/.../sdb/sdb2 (block)
      udev(async)|        UDEV   change /devices/.../sdb (block)
      udev(async)|        UDEV   add    /devices/.../sdb/sdb1 (block)
      udev(async)|        UDEV   add    /devices/.../sdb/sdb2 (block)
      gparted    |  set_device_from_disk(device, device_path="/dev/sdb")
      gparted    |    get_device(device_path="/dev/sdb", ..., flush=true)
      gparted    |      ped_device_get()
      gparted    |      flush_device(lp_device)  lp_device->path="/dev/sdb"
      gparted    |        ped_device_open()
      libparted  |          open("/dev/sdb", O_RDWR) = 8
      gparted    |        ped_device_sync()
      gparted    |        ped_device_close()
      libparted  |          close(8)
      udev(async)|            KERNEL remove /devices/.../sdb/sdb1 (block)
      udev(async)|            KERNEL remove /devices/.../sdb/sdb2 (block)
      udev(async)|            KERNEL change /devices/.../sdb (block)
      udev(async)|            KERNEL add    /devices/.../sdb/sdb1 (block)
      udev(async)|            KERNEL add    /devices/.../sdb/sdb2 (block)
      udev(async)|            UDEV   remove /devices/.../sdb/sdb1 (block)
      udev(async)|            UDEV   remove /devices/.../sdb/sdb2 (block)
      udev(async)|            UDEV   change /devices/.../sdb (block)
      udev(async)|            UDEV   add    /devices/.../sdb/sdb1 (block)
      udev(async)|            UDEV   add    /devices/.../sdb/sdb2 (block)
      gparted    |        settle_device()
      gparted    |          Utils::execute_command("udevadm settle --timeout=1")
      gparted    |    detect_filesystem(lp_device, NULL, ...)  lp_device->path="/dev/sdb"
      gparted    |      detect_filesystem_internal(lp_device, NULL)  lp_device->path="/dev/sdb"
      gparted    |        ped_device_open()
      libparted  |          open("/dev/sdb", O_RDWR) = 8
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_read()
      gparted    |        ped_device_close()
      libparted  |          close(8)
      udev(async)|            KERNEL remove /devices/.../sdb/sdb1 (block)
      udev(async)|            KERNEL remove /devices/.../sdb/sdb2 (block)
      udev(async)|            KERNEL change /devices/.../sdb (block)
      udev(async)|            KERNEL add    /devices/.../sdb/sdb1 (block)
      udev(async)|            KERNEL add    /devices/.../sdb/sdb2 (block)
      udev(async)|            UDEV   remove /devices/.../sdb/sdb1 (block)
      udev(async)|            UDEV   remove /devices/.../sdb/sdb2 (block)
      udev(async)|            UDEV   change /devices/.../sdb (block)
      udev(async)|            UDEV   add    /devices/.../sdb/sdb1 (block)
      udev(async)|            UDEV   add    /devices/.../sdb/sdb2 (block)
      gparted    |    get_disk(lp_device, lp_disk, strict=false)
      gparted    |      ped_disk_new(lp_device)  lp_device->path="/dev/sdb"
      libparted  |        open("/dev/sdb", O_RDWR) = 8
      libparted  |        close(8)
      udev(async)|          KERNEL remove /devices/.../sdb/sdb1 (block)
      udev(async)|          KERNEL remove /devices/.../sdb/sdb2 (block)
      udev(async)|          KERNEL change /devices/.../sdb (block)
      udev(async)|          KERNEL add    /devices/.../sdb/sdb1 (block)
      udev(async)|          KERNEL add    /devices/.../sdb/sdb2 (block)
      udev(async)|          UDEV   remove /devices/.../sdb/sdb1 (block)
      udev(async)|          UDEV   remove /devices/.../sdb/sdb2 (block)
      udev(async)|          UDEV   change /devices/.../sdb (block)
      udev(async)|          UDEV   add    /devices/.../sdb/sdb1 (block)
      udev(async)|          UDEV   add    /devices/.../sdb/sdb2 (block)
      gparted    |    set_device_partitions()
      gparted    |      detect_filesystem()
      gparted    |      set_partition_label_and_uuid(partition)  partition.get_path()="/dev/sdb1"
      gparted    |        read_label()
      gparted    |          ext2::read_label()
      gparted    |            Utils::execute_command("e2label /dev/sdb1")
    
    So in this case for a partitioned drive, libparted ped_disk_new() is
    opening and closing the device read-write and triggering the udev remove
    and add partition block special entries exactly when the label is trying
    to be read from the first partition, causing the device missing error.
    The call chain is:
        set_devices_thread()
            set_device_from_disk()
                get_disk()
                    ped_disk_new()
    
    Looking at the source for ped_disk_new() [1] it is calling
    ped_device_open() and ped_device_close(), hence why it is opening the
    device read-write and triggering the udev events.
    
    Looking back at commit "Wait for udev to recreate /dev/PTN entries when
    calibrating (#762941)" [2] and re-testing it, the udev events were also
    caused by ped_disk_new() with this call chain:
        apply_operation_to_disk()
            calibrate_partition()
                get_disk()
                    ped_disk_new()
    
    Fix this probe case and keep the fix for the previous calibrate case by
    moving the waiting for udev events from calibrate_partition() into
    get_disk(), right after ped_disk_new().  The maximum wait time is
    shortened to the shorter 1 second probing timeout to avoid the longer
    10 second apply timeout in this probing case.  The calibrate case commit
    message said "The longest I have seen udev take to do this is 0.80
    seconds in a VM".
    
    [1] parted/libparted/disk.c:ped_disk_new()
        http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/disk.c?id=v3.2#n169
    
    [2] fd9013d5f6971e9282f019903d6e148e367718bf
        Wait for udev to recreate /dev/PTN entries when calibrating
        (#762941)
    
    Closes !46 - Whole device FAT32 file system reports device busy warning
                 from mlabel

 src/GParted_Core.cc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
---
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index 83846c19..88078d1d 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -3510,12 +3510,6 @@ bool GParted_Core::calibrate_partition( Partition & partition, OperationDetail &
                        destroy_device_and_disk( lp_device, lp_disk ) ;
                }
 
-               // (#762941) Above libparted partition querying triggers udev >= 219 to
-               // remove and re-add all the partition specific /dev/ entries.  Wait for
-               // this to complete to avoid FS specific commands failing because they
-               // happen to run just when the needed /dev/PTN entry doesn't exist.
-               settle_device( SETTLE_DEVICE_APPLY_MAX_WAIT_SECONDS );
-
                operationdetail.get_last_child().set_success_and_capture_errors( success );
                return success;
        }
@@ -4123,6 +4117,12 @@ bool GParted_Core::get_disk( PedDevice *& lp_device, PedDisk *& lp_disk, bool st
        {
                lp_disk = ped_disk_new( lp_device );
 
+               // (#762941)(!46) After ped_disk_new() wait for triggered udev rules to
+               // to complete which remove and re-add all the partition specific /dev
+               // entries to avoid FS specific commands failing because they happen to
+               // be running when the needed /dev/PTN entries don't exist.
+               settle_device(SETTLE_DEVICE_PROBE_MAX_WAIT_SECONDS);
+
                // if ! disk and writable it's probably a HD without disklabel.
                // We return true here and deal with them in
                // GParted_Core::set_device_from_disk().


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]