[parted-devel] [PATCH V3 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."

As the parted create command couldn't get the start and length of the
partition due to missing sysfs entry due to which its going to do
add_partition causing it to fail with EBUSY.

The fix is to do a few retries to get the start and length of the
partition. These retries should be done once the add_partition() returns
the EBUSY. At some point of time, the sysfs entries will appear as
systemd will add them.

Changes V3 <-- V2
	1. Changed the logic to do retry in _disk_sync_part_table() when
	   add_partition() function returns EBUSY. In V2, the retry
	   logic was in _sysfs_ull_entry_from_part() when the open()
	   call to the sysfs entry fails. This retry in open() call was
	   giving problem for the new partition being created. As the
	   new partition doesn't exist, it will always retry till max
	   sleep and causes performance issues. The new logic in V3 is
	   similar to the one when we get the EBUSY while we do
	   remove_partition().

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

diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index ccbba8656bc7..4d89cf4af4e8 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -291,6 +291,10 @@ struct blkdev_ioctl_param {
 /* Maximum number of partitions supported by linux. */
 #define MAX_NUM_PARTS		64
 
+#define	MAX_SLEEP_SECONDS	1
+#define	SLEEP_MICRO_SECONDS	10000
+#define	NSEC_PER_MICROSEC	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);
@@ -3151,16 +3155,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 = (MAX_SLEEP_SECONDS
+                                        * NSEC_PER_MICROSEC / 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 */
@@ -3175,6 +3177,10 @@ _disk_sync_part_table (PedDisk* disk)
                         continue;
                 unsigned long long length;
                 unsigned long long start;
+		unsigned int n_sleep = (MAX_SLEEP_SECONDS
+					* NSEC_PER_MICROSEC / SLEEP_MICRO_SECONDS);
+
+retry:
                 /* get start and length of existing partition */
                 if (get_partition_start_and_length(part,
                                                    &start, &length)
@@ -3197,6 +3203,12 @@ _disk_sync_part_table (PedDisk* disk)
                 }
                 /* add the (possibly modified or new) partition */
                 if (!add_partition (disk, part)) {
+			if (errno == EBUSY) {
+				usleep (SLEEP_MICRO_SECONDS);
+				n_sleep--;
+				if (n_sleep)
+					goto retry;
+			}
                         ok[i - 1] = 0;
                         errnums[i - 1] = errno;
                 }
-- 
2.43.5