9:01 a.m.
virt-v2v fails when traverses each partition:
inspect_sources.ml:
...
and is_uefi_bootable_part part =
let dev = g#part_to_dev part in
parttype_is_gpt dev && is_uefi_ESP dev part
in
let partitions = Array.to_list (g#list_partitions ()) in
let partitions = List.filter is_uefi_bootable_part partitions in
log:
...
libguestfs: trace: v2v: list_partitions
guestfsd: main_loop: new request, len 0x28
guestfsd: main_loop: proc 8 (list_partitions) took 0.00 seconds
libguestfs: trace: v2v: list_partitions = ["/dev/sda1", "/dev/sda2",
"/dev/sda3",
"/dev/sdb1", "/dev/sdb2", "/dev/sdb5",
"/dev/sdc1", "/dev/sdc2", "/dev/sdc5",
"/dev/sdd1", "/dev/sdd2", "/dev/sdd5",
"/dev/sde1", "/dev/sde2", "/dev/sde5",
"/dev/sdf1", "/dev/sdf2", "/dev/sdf5",
"/dev/sdg1", "/dev/sdg2", "/dev/sdg5"]
...
libguestfs: trace: v2v: part_get_parttype "/dev/sdb"
guestfsd: main_loop: new request, len 0x34
commandrvf: stdout=y stderr=y flags=0x0
commandrvf: parted -m -s -- /dev/sdb unit b print
libguestfs: trace: v2v: part_get_parttype = "msdos"
libguestfs: trace: v2v: part_get_parttype "/dev/sdb"
guestfsd: main_loop: proc 214 (part_get_parttype) took 0.00 seconds
guestfsd: main_loop: new request, len 0x34
commandrvf: stdout=y stderr=y flags=0x0
commandrvf: parted -m -s -- /dev/sdb unit b print
libguestfs: trace: v2v: part_get_parttype = "msdos"
libguestfs: trace: v2v: part_to_dev "/dev/sdb5"
guestfsd: main_loop: proc 214 (part_get_parttype) took 0.00 seconds
[ 9.164666] sdb: sdb1 sdb2 < sdb5 >
guestfsd: main_loop: new request, len 0x38
/dev/sdb5: No such file or directory
call part_to_dev "/dev/sdb5" failed in is_root_device():
part_to_dev_stub()
RESOLVE_DEVICE()
is_root_device ():
{
struct stat statbuf;
if (stat (device, &statbuf) == -1) {
perror (device);
return 0;
...
let's look at the udev event log after the error:
KERNEL[10.527855] change
/devices/pci0000:00/0000:00:03.0/virtio1/host2/target2:0:1/2:0:1:0/block/sdb (block)
UDEV [10.528693] change
/devices/pci0000:00/0000:00:03.0/virtio1/host2/target2:0:0/2:0:0:0/block/sda/sda1 (block)
KERNEL[10.529156] add
/devices/pci0000:00/0000:00:03.0/virtio1/host2/target2:0:1/2:0:1:0/block/sdb/sdb1 (block)
KERNEL[10.529618] add
/devices/pci0000:00/0000:00:03.0/virtio1/host2/target2:0:1/2:0:1:0/block/sdb/sdb2 (block)
KERNEL[10.530096] add
/devices/pci0000:00/0000:00:03.0/virtio1/host2/target2:0:1/2:0:1:0/block/sdb/sdb5 (block)
...
Apparently is_root_device("/dev/sdb5") was called before "/dev/sdb5"
was added.
To solve issue we need to wait adding a partition before we can access it.
Another solution if we wait for adding all partitions somewhere in one place.
But there is at least one drawback, not for each task need to touch the disk
partitions and waiting for all partitions will slow such tasks.
Pavel Butsykin (2):
daemon: run 'udevadm settle' with --exit-if-exists option.
daemon: add udev_settle_file to is_root_device
daemon/daemon.h | 2 ++
daemon/guestfsd.c | 23 +++++++++++++++++++----
2 files changed, 21 insertions(+), 4 deletions(-)
--
2.11.0
9:01 a.m.
New subject: [PATCH 1/2] daemon: run 'udevadm settle' with --exit-if-exists option
Add udev_settle_file() to run 'udevadm settle' with --exit-if-exists option. It
will slightly reduce the waiting-time for pending events if we need to wait
for events related to a particular device/file.
Signed-off-by: Pavel Butsykin <pbutsykin(a)virtuozzo.com>
---
daemon/daemon.h | 2 ++
daemon/guestfsd.c | 20 ++++++++++++++++----
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 79a5288f6..90ebaafbe 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -141,6 +141,8 @@ extern int parse_btrfsvol (const char *desc, mountable_t *mountable);
extern int prog_exists (const char *prog);
+extern void udev_settle_file (const char *file);
+
extern void udev_settle (void);
extern int random_name (char *template);
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 85ce5d2ad..dccfa15bc 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -1213,13 +1213,18 @@ random_name (char *template)
* fussed if it fails.
*/
void
-udev_settle (void)
+udev_settle_file (const char *file)
{
- char cmd[80];
+ size_t cmd_size = strlen (str_udevadm) +
+ sizeof (" settle") +
+ sizeof (" --debug") +
+ (file ? sizeof (" --exit-if-exists=") + strlen (file) :
0);
+ char *cmd = malloc (cmd_size);
int r;
- snprintf (cmd, sizeof cmd, "%s%s settle",
- str_udevadm, verbose ? " --debug" : "");
+ snprintf(cmd, cmd_size, "%s%s settle%s%s",
+ str_udevadm, verbose ? " --debug" : "",
+ file ? " --exit-if-exists=" : "", file ? file :
"");
if (verbose)
printf ("%s\n", cmd);
r = system (cmd);
@@ -1227,6 +1232,13 @@ udev_settle (void)
perror ("system");
else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
fprintf (stderr, "warning: udevadm command failed\n");
+ free (cmd);
+}
+
+void
+udev_settle (void)
+{
+ udev_settle_file (NULL);
}
char *
--
2.11.0
4:11 p.m.
New subject: [PATCH 1/2] daemon: run 'udevadm settle' with --exit-if-exists option
On Wed, Apr 12, 2017 at 05:01:10PM +0300, Pavel Butsykin wrote:
Add udev_settle_file() to run 'udevadm settle' with
--exit-if-exists option. It
will slightly reduce the waiting-time for pending events if we need to wait
for events related to a particular device/file.
Signed-off-by: Pavel Butsykin <pbutsykin(a)virtuozzo.com>
---
daemon/daemon.h | 2 ++
daemon/guestfsd.c | 20 ++++++++++++++++----
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 79a5288f6..90ebaafbe 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -141,6 +141,8 @@ extern int parse_btrfsvol (const char *desc, mountable_t
*mountable);
extern int prog_exists (const char *prog);
+extern void udev_settle_file (const char *file);
+
extern void udev_settle (void);
extern int random_name (char *template);
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 85ce5d2ad..dccfa15bc 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -1213,13 +1213,18 @@ random_name (char *template)
* fussed if it fails.
*/
void
-udev_settle (void)
+udev_settle_file (const char *file)
{
- char cmd[80];
+ size_t cmd_size = strlen (str_udevadm) +
+ sizeof (" settle") +
+ sizeof (" --debug") +
+ (file ? sizeof (" --exit-if-exists=") + strlen (file) :
0);
+ char *cmd = malloc (cmd_size);
int r;
- snprintf (cmd, sizeof cmd, "%s%s settle",
- str_udevadm, verbose ? " --debug" : "");
+ snprintf(cmd, cmd_size, "%s%s settle%s%s",
+ str_udevadm, verbose ? " --debug" : "",
+ file ? " --exit-if-exists=" : "", file ? file :
"");
if (verbose)
printf ("%s\n", cmd);
r = system (cmd);
@@ -1227,6 +1232,13 @@ udev_settle (void)
perror ("system");
else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
fprintf (stderr, "warning: udevadm command failed\n");
+ free (cmd);
There are some ugly quoting (and hence, possibly, security) problems
with this patch.
It's more secure and also much simpler to call ADD_ARG + commandv
here. For an example of how to use it, see:
https://github.com/libguestfs/libguestfs/blob/master/daemon/mkfs.c#L41-L247
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
3:55 a.m.
New subject: [PATCH 1/2] daemon: run 'udevadm settle' with --exit-if-exists option
On 13.04.2017 00:11, Richard W.M. Jones wrote:
On Wed, Apr 12, 2017 at 05:01:10PM +0300, Pavel Butsykin wrote:
> Add udev_settle_file() to run 'udevadm settle' with --exit-if-exists option.
It
> will slightly reduce the waiting-time for pending events if we need to wait
> for events related to a particular device/file.
>
> Signed-off-by: Pavel Butsykin <pbutsykin(a)virtuozzo.com>
> ---
> daemon/daemon.h | 2 ++
> daemon/guestfsd.c | 20 ++++++++++++++++----
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/daemon/daemon.h b/daemon/daemon.h
> index 79a5288f6..90ebaafbe 100644
> --- a/daemon/daemon.h
> +++ b/daemon/daemon.h
> @@ -141,6 +141,8 @@ extern int parse_btrfsvol (const char *desc, mountable_t
*mountable);
>
> extern int prog_exists (const char *prog);
>
> +extern void udev_settle_file (const char *file);
> +
> extern void udev_settle (void);
>
> extern int random_name (char *template);
> diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
> index 85ce5d2ad..dccfa15bc 100644
> --- a/daemon/guestfsd.c
> +++ b/daemon/guestfsd.c
> @@ -1213,13 +1213,18 @@ random_name (char *template)
> * fussed if it fails.
> */
> void
> -udev_settle (void)
> +udev_settle_file (const char *file)
> {
> - char cmd[80];
> + size_t cmd_size = strlen (str_udevadm) +
> + sizeof (" settle") +
> + sizeof (" --debug") +
> + (file ? sizeof (" --exit-if-exists=") + strlen (file)
: 0);
> + char *cmd = malloc (cmd_size);
> int r;
>
> - snprintf (cmd, sizeof cmd, "%s%s settle",
> - str_udevadm, verbose ? " --debug" : "");
> + snprintf(cmd, cmd_size, "%s%s settle%s%s",
> + str_udevadm, verbose ? " --debug" : "",
> + file ? " --exit-if-exists=" : "", file ? file :
"");
> if (verbose)
> printf ("%s\n", cmd);
> r = system (cmd);
> @@ -1227,6 +1232,13 @@ udev_settle (void)
> perror ("system");
> else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
> fprintf (stderr, "warning: udevadm command failed\n");
> + free (cmd);
There are some ugly quoting (and hence, possibly, security) problems
with this patch.
It's more secure and also much simpler to call ADD_ARG + commandv
here. For an example of how to use it, see:
https://github.com/libguestfs/libguestfs/blob/master/daemon/mkfs.c#L41-L247
I agree, it would be better, thanks!
Rich.
9:01 a.m.
New subject: [PATCH 2/2] daemon: add udev_settle_file to is_root_device
It fixes a fail with ENOENT when device is not yet present after system boot.
Signed-off-by: Pavel Butsykin <pbutsykin(a)virtuozzo.com>
---
daemon/guestfsd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index dccfa15bc..9aa69301f 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -402,6 +402,9 @@ int
is_root_device (const char *device)
{
struct stat statbuf;
+
+ udev_settle_file(device);
+
if (stat (device, &statbuf) == -1) {
perror (device);
return 0;
--
2.11.0
4:12 p.m.
New subject: [PATCH 2/2] daemon: add udev_settle_file to is_root_device
On Wed, Apr 12, 2017 at 05:01:11PM +0300, Pavel Butsykin wrote:
It fixes a fail with ENOENT when device is not yet present after
system boot.
Signed-off-by: Pavel Butsykin <pbutsykin(a)virtuozzo.com>
---
daemon/guestfsd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index dccfa15bc..9aa69301f 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -402,6 +402,9 @@ int
is_root_device (const char *device)
{
struct stat statbuf;
+
+ udev_settle_file(device);
Add a space after the function name, but with that change ACK.
if (stat (device, &statbuf) == -1) {
perror (device);
return 0;
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
2815
days inactive
2816
days old
5 comments
2 participants
participants (2)
-
Pavel Butsykin -
Richard W.M. Jones