Here are two patches.
The first is just renaming and clean-up.
The meaty 2nd one has the following long commit log.
--------------------------------------------------------------
libparted: avoid race in informing the kernel of partition table changes
When sync'ing a partition table change using the latest
code, sometimes we'd get an unwarranted failure like this:
Warning: Partition(s) 1 on /dev/sdd 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.
To be precise, when running the partition-resizing root-only test
in a loop:
for i in $(seq 240); do make -C tests check VERBOSE=yes \
TESTS=t3000-resize-fs.sh >& log.$i && printf . || echo $i $?; done
I would typically see about 50% of them fail on a Fedora 13 system.
It was obvious that this was due to a race condition when I found that
modifying that tests' parted...resize invocation to go via strace changed
the timing enough to make the test pass every time.
The fix is to retry the partition-removal step upon any EBUSY failure,
currently for up to 1 second (retrying up to 100 times, sleeping 10ms
after each failure).
* libparted/arch/linux.c (_disk_sync_part_table): Allocate "ok" using
calloc, now that its initial values matter.
Retry each removal upon EBUSY-failure.
* bootstrap.conf (gnulib_modules): Use gnulib's usleep module.
>From 9acc5869985c8616066af4825f52fb6c474f63dd Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
Date: Fri, 30 Apr 2010 12:01:08 +0200
Subject: [PATCH 1/2] libparted: variable renaming, minor "goto" reorg
* libparted/arch/linux.c (_disk_sync_part_table): Rename local array:
s/rets/ok/, for readability.
Use only a single label, "cleanup:", rather than two: free_rets
and free_errnums.
---
libparted/arch/linux.c | 27 +++++++++++++--------------
1 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index d8f3393..8116c76 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2452,29 +2452,29 @@ _disk_sync_part_table (PedDisk* disk)
if (lpn < 0)
return 0;
int ret = 0;
- int *rets = ped_malloc(sizeof(int) * lpn);
- if (!rets)
+ int *ok = ped_malloc(sizeof(int) * lpn);
+ if (!ok)
return 0;
int *errnums = ped_malloc(sizeof(int) * lpn);
if (!errnums)
- goto free_rets;
+ goto cleanup;
int i;
for (i = 1; i <= lpn; i++) {
- rets[i - 1] = _blkpg_remove_partition (disk, i);
+ ok[i - 1] = _blkpg_remove_partition (disk, i);
errnums[i - 1] = errno;
}
for (i = 1; i <= lpn; i++) {
const PedPartition *part = ped_disk_get_partition (disk, i);
if (part) {
- if (!rets[i - 1] && errnums[i - 1] == EBUSY) {
+ if (!ok[i - 1] && errnums[i - 1] == EBUSY) {
struct hd_geometry geom;
unsigned long long length = 0;
/* get start and length of existing partition
*/
char *dev_name = _device_get_part_path
(disk->dev, i);
if (!dev_name)
- goto free_errnums;
+ goto cleanup;
int fd = open (dev_name, O_RDONLY);
if (fd == -1
|| ioctl (fd, HDIO_GETGEO, &geom)
@@ -2487,14 +2487,14 @@ _disk_sync_part_table (PedDisk* disk)
if (fd != -1)
close (fd);
free (dev_name);
- goto free_errnums;
+ goto cleanup;
}
free (dev_name);
length /= disk->dev->sector_size;
close (fd);
if (geom.start == part->geom.start
&& length == part->geom.length)
- rets[i - 1] = 1;
+ ok[i - 1] = 1;
/* If the new partition is unchanged and the
existing one was not removed because it was
in use, then reset the error flag and do not
@@ -2509,7 +2509,7 @@ _disk_sync_part_table (PedDisk* disk)
PED_EXCEPTION_RETRY_CANCEL,
_("Failed to add partition %d (%s)"),
i, strerror (errno));
- goto free_errnums;
+ goto cleanup;
}
}
}
@@ -2517,12 +2517,12 @@ _disk_sync_part_table (PedDisk* disk)
char *bad_part_list = NULL;
/* now warn about any errors */
for (i = 1; i <= lpn; i++) {
- if (rets[i - 1] || errnums[i - 1] == ENXIO)
+ if (ok[i - 1] || errnums[i - 1] == ENXIO)
continue;
if (bad_part_list == NULL) {
bad_part_list = malloc (lpn * 5);
if (!bad_part_list)
- goto free_errnums;
+ goto cleanup;
bad_part_list[0] = 0;
}
sprintf (bad_part_list + strlen (bad_part_list), "%d, ", i);
@@ -2543,10 +2543,9 @@ _disk_sync_part_table (PedDisk* disk)
ret = 1;
free (bad_part_list);
}
- free_errnums:
+ cleanup:
free (errnums);
- free_rets:
- free (rets);
+ free (ok);
return ret;
}
--
1.7.1.328.g9993c
>From 0f850220b3f26bb969a1a7ff78dc550691a89566 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
Date: Fri, 30 Apr 2010 12:28:16 +0200
Subject: [PATCH 2/2] libparted: avoid race in informing the kernel of partition
table changes
When sync'ing a partition table change using the latest
code, sometimes we'd get an unwarranted failure like this:
Warning: Partition(s) 1 on /dev/sdd 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.
To be precise, when running the partition-resizing root-only test
in a loop:
for i in $(seq 240); do make -C tests check VERBOSE=yes \
TESTS=t3000-resize-fs.sh >& log.$i && printf . || echo $i $?; done
I would typically see about 50% of them fail on a Fedora 13 system.
It was obvious that this was due to a race condition when I found that
modifying that tests' parted...resize invocation to go via strace changed
the timing enough to make the test pass every time.
The fix is to retry the partition-removal step upon any EBUSY failure,
currently for up to 1 second (retrying up to 100 times, sleeping 10ms
after each failure).
* libparted/arch/linux.c (_disk_sync_part_table): Allocate "ok" using
calloc, now that its initial values matter.
Retry each removal upon EBUSY-failure.
* bootstrap.conf (gnulib_modules): Use gnulib's usleep module.
---
bootstrap.conf | 1 +
libparted/arch/linux.c | 28 +++++++++++++++++++++++-----
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 6c9287d..4ca51a7 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -58,6 +58,7 @@ gnulib_modules="
unlink
update-copyright
useless-if-before-free
+ usleep
vc-list-files
version-etc-fsf
warnings
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 8116c76..73a8e6f 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2452,17 +2452,35 @@ _disk_sync_part_table (PedDisk* disk)
if (lpn < 0)
return 0;
int ret = 0;
- int *ok = ped_malloc(sizeof(int) * lpn);
+ int *ok = calloc (lpn, sizeof *ok);
if (!ok)
return 0;
int *errnums = ped_malloc(sizeof(int) * lpn);
if (!errnums)
goto cleanup;
- int i;
- for (i = 1; i <= lpn; i++) {
- ok[i - 1] = _blkpg_remove_partition (disk, i);
- errnums[i - 1] = errno;
+ /* Attempt to remove each and every 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);
+ int i;
+ for (i = 0; i < n_sleep; i++) {
+ if (i)
+ usleep (sleep_microseconds);
+ bool busy = false;
+ int j;
+ for (j = 0; j < lpn; j++) {
+ if (!ok[j]) {
+ ok[j] = _blkpg_remove_partition (disk, j + 1);
+ errnums[j] = errno;
+ if (!ok[j] && errnums[j] == EBUSY)
+ busy = true;
+ }
+ }
+ if (!busy)
+ break;
}
for (i = 1; i <= lpn; i++) {
--
1.7.1.328.g9993c
_______________________________________________
bug-parted mailing list
bug-parted@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-parted
