[Libguestfs] [PATCH 2/2] blockdev, parted: Call udev_settle before and after commands. (RHBZ#769304)
Richard W.M. Jones
rjones at redhat.com
Mon Feb 6 21:56:08 UTC 2012
From: "Richard W.M. Jones" <rjones at redhat.com>
See comments in the code for details.
This is an alternate fix to
commit a9c8123c72db47bcab8dd738e8d5256a9ae87f11.
---
daemon/blockdev.c | 14 +++++
daemon/parted.c | 153 +++++++++++++++++++++++++++++++++++------------------
2 files changed, 115 insertions(+), 52 deletions(-)
diff --git a/daemon/blockdev.c b/daemon/blockdev.c
index d056abe..a7fd2cb 100644
--- a/daemon/blockdev.c
+++ b/daemon/blockdev.c
@@ -46,6 +46,20 @@ call_blockdev (const char *device, const char *switc, int extraarg, int prints)
};
char buf[64];
+ /* When you call close on any block device, udev kicks off a rule
+ * which runs blkid to reexamine the device. We need to wait for
+ * this rule to finish running (from a previous operation) since it
+ * holds the device open and can cause other operations to fail,
+ * notably BLKRRPART.
+ *
+ * This is particularly a problem where we have just written to a
+ * device (eg. zeroing it) and immediately call blockdev --rereadpt.
+ *
+ * Therefore, wait for udev to finish all outstanding events before
+ * performing any blockdev command.
+ */
+ udev_settle ();
+
if (extraarg > 0) {
snprintf (buf, sizeof buf, "%d", extraarg);
argv[2] = buf;
diff --git a/daemon/parted.c b/daemon/parted.c
index 16f0843..2610042 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -33,45 +33,14 @@
* Parted 1.9 sends error messages to stdout, hence use of the
* COMMAND_FLAG_FOLD_STDOUT_ON_STDERR flag.
*
- * parted occasionally fails to do ioctl(BLKRRPART) on the device,
- * apparently because of some internal race in the code. We attempt
- * to detect and recover from this error if we can.
+ * There is a reason why we call udev_settle both before and after
+ * each command. When you call close on any block device, udev kicks
+ * off a rule which runs blkid to reexamine the device. We need to
+ * wait for this rule to finish running (from a previous operation)
+ * since it holds the device open. Since parted also closes the block
+ * device, it can cause udev to run again, hence the call to
+ * udev_settle afterwards.
*/
-static int
-recover_blkrrpart (const char *device, const char *err)
-{
- int r;
-
- if (!strstr (err,
- "Error informing the kernel about modifications to partition"))
- return -1;
-
- r = command (NULL, NULL, "blockdev", "--rereadpt", device, NULL);
- if (r == -1)
- return -1;
-
- udev_settle ();
-
- return 0;
-}
-
-#define RUN_PARTED(error,device,...) \
- do { \
- int r; \
- char *err; \
- \
- r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR, \
- "parted", "-s", "--", (device), __VA_ARGS__); \
- if (r == -1) { \
- if (recover_blkrrpart ((device), err) == -1) { \
- reply_with_error ("%s: parted: %s: %s", __func__, (device), err); \
- free (err); \
- error; \
- } \
- } \
- \
- free (err); \
- } while (0)
static const char *
check_parttype (const char *parttype)
@@ -101,13 +70,25 @@ check_parttype (const char *parttype)
int
do_part_init (const char *device, const char *parttype)
{
+ int r;
+ char *err;
+
parttype = check_parttype (parttype);
if (!parttype) {
reply_with_error ("unknown partition type: common choices are \"gpt\" and \"msdos\"");
return -1;
}
- RUN_PARTED (return -1, device, "mklabel", parttype, NULL);
+ udev_settle ();
+
+ r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
+ "parted", "-s", "--", device, "mklabel", parttype, NULL);
+ if (r == -1) {
+ reply_with_error ("parted: %s: %s", device, err);
+ free (err);
+ return -1;
+ }
+ free (err);
udev_settle ();
@@ -118,6 +99,8 @@ int
do_part_add (const char *device, const char *prlogex,
int64_t startsect, int64_t endsect)
{
+ int r;
+ char *err;
char startstr[32];
char endstr[32];
@@ -146,12 +129,22 @@ do_part_add (const char *device, const char *prlogex,
snprintf (startstr, sizeof startstr, "%" PRIi64 "s", startsect);
snprintf (endstr, sizeof endstr, "%" PRIi64 "s", endsect);
+ udev_settle ();
+
/* XXX Bug: If the partition table type (which we don't know in this
* function) is GPT, then this parted command sets the _partition
* name_ to prlogex, eg. "primary". I would essentially describe
* this as a bug in the parted mkpart command.
*/
- RUN_PARTED (return -1, device, "mkpart", prlogex, startstr, endstr, NULL);
+ r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
+ "parted", "-s", "--",
+ device, "mkpart", prlogex, startstr, endstr, NULL);
+ if (r == -1) {
+ reply_with_error ("parted: %s: %s", device, err);
+ free (err);
+ return -1;
+ }
+ free (err);
udev_settle ();
@@ -161,6 +154,9 @@ do_part_add (const char *device, const char *prlogex,
int
do_part_del (const char *device, int partnum)
{
+ int r;
+ char *err;
+
if (partnum <= 0) {
reply_with_error ("partition number must be >= 1");
return -1;
@@ -169,15 +165,28 @@ do_part_del (const char *device, int partnum)
char partnum_str[16];
snprintf (partnum_str, sizeof partnum_str, "%d", partnum);
- RUN_PARTED (return -1, device, "rm", partnum_str, NULL);
+ udev_settle ();
+
+ r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
+ "parted", "-s", "--", device, "rm", partnum_str, NULL);
+ if (r == -1) {
+ reply_with_error ("parted: %s: %s", device, err);
+ free (err);
+ return -1;
+ }
+ free (err);
udev_settle ();
+
return 0;
}
int
do_part_disk (const char *device, const char *parttype)
{
+ int r;
+ char *err;
+
parttype = check_parttype (parttype);
if (!parttype) {
reply_with_error ("unknown partition type: common choices are \"gpt\" and \"msdos\"");
@@ -195,12 +204,21 @@ do_part_disk (const char *device, const char *parttype)
const char *startstr = "128s";
const char *endstr = "-128s";
- RUN_PARTED (return -1,
- device,
- "mklabel", parttype,
- /* See comment about about the parted mkpart command. */
- "mkpart", STREQ (parttype, "gpt") ? "p1" : "primary",
- startstr, endstr, NULL);
+ udev_settle ();
+
+ r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
+ "parted", "-s", "--",
+ device,
+ "mklabel", parttype,
+ /* See comment about about the parted mkpart command. */
+ "mkpart", STREQ (parttype, "gpt") ? "p1" : "primary",
+ startstr, endstr, NULL);
+ if (r == -1) {
+ reply_with_error ("parted: %s: %s", device, err);
+ free (err);
+ return -1;
+ }
+ free (err);
udev_settle ();
@@ -210,6 +228,9 @@ do_part_disk (const char *device, const char *parttype)
int
do_part_set_bootable (const char *device, int partnum, int bootable)
{
+ int r;
+ char *err;
+
if (partnum <= 0) {
reply_with_error ("partition number must be >= 1");
return -1;
@@ -219,8 +240,17 @@ do_part_set_bootable (const char *device, int partnum, int bootable)
snprintf (partstr, sizeof partstr, "%d", partnum);
- RUN_PARTED (return -1,
- device, "set", partstr, "boot", bootable ? "on" : "off", NULL);
+ udev_settle ();
+
+ r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
+ "parted", "-s", "--",
+ device, "set", partstr, "boot", bootable ? "on" : "off", NULL);
+ if (r == -1) {
+ reply_with_error ("parted: %s: %s", device, err);
+ free (err);
+ return -1;
+ }
+ free (err);
udev_settle ();
@@ -230,6 +260,9 @@ do_part_set_bootable (const char *device, int partnum, int bootable)
int
do_part_set_name (const char *device, int partnum, const char *name)
{
+ int r;
+ char *err;
+
if (partnum <= 0) {
reply_with_error ("partition number must be >= 1");
return -1;
@@ -239,7 +272,16 @@ do_part_set_name (const char *device, int partnum, const char *name)
snprintf (partstr, sizeof partstr, "%d", partnum);
- RUN_PARTED (return -1, device, "name", partstr, name, NULL);
+ udev_settle ();
+
+ r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
+ "parted", "-s", "--", device, "name", partstr, name, NULL);
+ if (r == -1) {
+ reply_with_error ("parted: %s: %s", device, err);
+ free (err);
+ return -1;
+ }
+ free (err);
udev_settle ();
@@ -686,6 +728,8 @@ do_part_get_mbr_id (const char *device, int partnum)
char *out, *err;
int r;
+ udev_settle ();
+
r = command (&out, &err, "sfdisk", "--print-id", device, partnum_str, NULL);
if (r == -1) {
reply_with_error ("sfdisk --print-id: %s", err);
@@ -693,9 +737,10 @@ do_part_get_mbr_id (const char *device, int partnum)
free (err);
return -1;
}
-
free (err);
+ udev_settle ();
+
/* It's printed in hex ... */
int id;
if (sscanf (out, "%x", &id) != 1) {
@@ -725,6 +770,8 @@ do_part_set_mbr_id (const char *device, int partnum, int idbyte)
char *err;
int r;
+ udev_settle ();
+
r = command (NULL, &err, "sfdisk",
"--change-id", device, partnum_str, idbyte_str, NULL);
if (r == -1) {
@@ -732,7 +779,9 @@ do_part_set_mbr_id (const char *device, int partnum, int idbyte)
free (err);
return -1;
}
-
free (err);
+
+ udev_settle ();
+
return 0;
}
--
1.7.6
More information about the Libguestfs
mailing list