[parted-devel] [PATCH V2 1/2] arch: Fix race between systemd and parted command

[Gulam Mohamed]

When parted commands like print, delete and others along with create
command, are run continuously, at some point of time a race has been
noticed between the create command and systemd.

The parted create command when tries to sync the partition table by calling
"_disk_sync_part_table()" function, it will try to fetch the start and
length of the partition. For this, it will try to open the sysfs entry
of the partition (for example /sys/block/loop0/loop0p1/start).

At the same time, if systemd calls rescan_partitions, then the sysfs
entries will disapper as these partitions will be removed and added.

Now, since the parted create command is trying to open the same sysfs
entry to get the start and length, it will fail when systemd removes the
partition  and hence the parted command thinks that this is a new
partition and will try to add the existing partition again by calling
BLKPG_ADD_PARTITION ioctl. At kernel level, the kernel ioctl code will
check for the overlap. As the partition was already existing, it will
return EBUSY to the userspace.

This is causing the parted command to throw the below exception:

"Error: Partition(s) 1 on /dev/loop276070 have been written, but we have
been unable to inform the kernel of the change, probably because it/they
are in use.As a result, the old partition(s) will remain in use.You should
reboot now before making further changes."

This can be fixed by retrying to open the sysfs entry of the partition
device with few retries, if the open call fails.

Changes V2<--V1
	1. Implemented a new function open_sys_block() to open a path
           with timeout
	2. Provided the defines for device response timeout and sleep
           time in micrseconds
	3. Added a test case to test the fix which is in next patch

Signed-off-by: Gulam Mohamed <gulam.mohamed at oracle.com>
---
 libparted/arch/linux.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index ccbba8656bc7..acfc04727604 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -291,12 +291,17 @@ struct blkdev_ioctl_param {
 /* Maximum number of partitions supported by linux. */
 #define MAX_NUM_PARTS		64
 
+#define DEV_RESPONSE_TIMEOUT   1       /* In Seconds */
+#define SLEEP_MICRO_SECONDS    10000
+#define NSEC_PER_MSEC          1000000
+
 static char* _device_get_part_path (PedDevice const *dev, int num);
 static int _partition_is_mounted_by_path (const char* path);
 static unsigned int _device_get_partition_range(PedDevice const* dev);
 static int _device_open (PedDevice* dev, int flags);
 static int _device_open_ro (PedDevice* dev);
 static int _device_close (PedDevice* dev);
+static FILE* open_sys_block(char *path, const char *mode, int timeout);
 
 static int
 _read_fd (int fd, char **buf)
@@ -1044,8 +1049,8 @@ read_device_sysfs_file (PedDevice *dev, const char *file)
         snprintf (name_buf, 127, "/sys/block/%s/device/%s",
                   last_component (dev->path), file);
 
-        if ((f = fopen (name_buf, "r")) == NULL)
-                return NULL;
+	if ((f = open_sys_block(name_buf, "r", DEV_RESPONSE_TIMEOUT)) == NULL)
+		return NULL;
 
         if (fgets (buf, 255, f) == NULL) {
                 fclose (f);
@@ -2715,7 +2720,7 @@ _sysfs_int_entry_from_dev(PedDevice const* dev, const char *entry, int *val)
         if (r < 0 || r >= sizeof(path))
                 return false;
 
-        FILE *fp = fopen(path, "r");
+	FILE *fp = open_sys_block(path, "r", DEV_RESPONSE_TIMEOUT);
         if (!fp)
                 return false;
 
@@ -2745,7 +2750,8 @@ _sysfs_ull_entry_from_part(PedPartition const* part, const char *entry,
         if (r < 0 || r >= sizeof(path))
                 return false;
 
-        FILE *fp = fopen(path, "r");
+	FILE *fp = open_sys_block(path, "r", DEV_RESPONSE_TIMEOUT);
+
         if (!fp)
                 return false;
 
@@ -3151,16 +3157,14 @@ _disk_sync_part_table (PedDisk* disk)
                 }
                 /* Attempt to remove the partition, retrying for
                    up to max_sleep_seconds upon any failure due to EBUSY. */
-                unsigned int sleep_microseconds = 10000;
-                unsigned int max_sleep_seconds = 1;
-                unsigned int n_sleep = (max_sleep_seconds
-                                        * 1000000 / sleep_microseconds);
+		unsigned int n_sleep = (DEV_RESPONSE_TIMEOUT *
+					NSEC_PER_MSEC / SLEEP_MICRO_SECONDS);
                 do {
                         ok[i - 1] = remove_partition (disk, i);
                         errnums[i - 1] = errno;
                         if (ok[i - 1] || errnums[i - 1] != EBUSY)
                                 break;
-                        usleep (sleep_microseconds);
+                        usleep (SLEEP_MICRO_SECONDS);
                 } while (n_sleep--);
                 if (!ok[i - 1] && errnums[i - 1] == ENXIO)
                         ok[i - 1] = 1; /* it already doesn't exist */
@@ -3378,6 +3382,22 @@ s390_get_optimum_alignment(const PedDevice *dev)
 }
 #endif
 
+FILE* open_sys_block(char *path, const char *mode, int timeout)
+{
+       FILE *fp = NULL;
+       unsigned int n_sleeps = (DEV_RESPONSE_TIMEOUT *
+                                NSEC_PER_MSEC / SLEEP_MICRO_SECONDS);
+       do {
+               fp = fopen(path, mode);
+
+               if (fp)
+                       break;
+               usleep(SLEEP_MICRO_SECONDS);
+       } while (n_sleeps--);
+
+       return fp;
+}
+
 static PedDeviceArchOps linux_dev_ops = {
         _new:           linux_new,
         destroy:        linux_destroy,
-- 
2.43.5