9:55 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 | 42 +++++++++++++++++++++++++++++++-----------
2 files changed, 33 insertions(+), 11 deletions(-)
--
2.11.0
9:55 a.m.
New subject: [PATCH v2 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 | 39 ++++++++++++++++++++++++++++-----------
2 files changed, 30 insertions(+), 11 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..b82c98acd 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -63,6 +63,8 @@
#include "daemon.h"
+#define MAX_ARGS 64
+
GUESTFSD_EXT_CMD(str_udevadm, udevadm);
GUESTFSD_EXT_CMD(str_uuidgen, uuidgen);
@@ -1213,20 +1215,35 @@ random_name (char *template)
* fussed if it fails.
*/
void
-udev_settle (void)
+udev_settle_file (const char *file)
{
- char cmd[80];
+ const char *argv[MAX_ARGS];
+ CLEANUP_FREE char *err = NULL;
+ size_t i = 0;
int r;
- snprintf (cmd, sizeof cmd, "%s%s settle",
- str_udevadm, verbose ? " --debug" : "");
- if (verbose)
- printf ("%s\n", cmd);
- r = system (cmd);
- if (r == -1)
- perror ("system");
- else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
- fprintf (stderr, "warning: udevadm command failed\n");
+ ADD_ARG (argv, i, str_udevadm);
+ if (verbose) {
+ ADD_ARG (argv, i, "--debug");
+ }
+
+ ADD_ARG (argv, i, "settle");
+ if (file) {
+ ADD_ARG (argv, i, "-E");
+ ADD_ARG (argv, i, file);
+ }
+ ADD_ARG (argv, i, NULL);
+
+ r = commandv (NULL, &err, argv);
+ if (r == -1) {
+ reply_with_error ("udevadm command failed: %s", err);
+ }
+}
+
+void
+udev_settle (void)
+{
+ udev_settle_file (NULL);
}
char *
--
2.11.0
10:10 a.m.
New subject: [PATCH v2 1/2] daemon: run 'udevadm settle' with --exit-if-exists option
On Thursday, 13 April 2017 16:55:26 CEST 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 | 39 ++++++++++++++++++++++++++++-----------
2 files changed, 30 insertions(+), 11 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..b82c98acd 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -63,6 +63,8 @@
#include "daemon.h"
+#define MAX_ARGS 64
+
GUESTFSD_EXT_CMD(str_udevadm, udevadm);
GUESTFSD_EXT_CMD(str_uuidgen, uuidgen);
@@ -1213,20 +1215,35 @@ random_name (char *template)
* fussed if it fails.
*/
void
-udev_settle (void)
+udev_settle_file (const char *file)
{
- char cmd[80];
+ const char *argv[MAX_ARGS];
+ CLEANUP_FREE char *err = NULL;
+ size_t i = 0;
int r;
- snprintf (cmd, sizeof cmd, "%s%s settle",
- str_udevadm, verbose ? " --debug" : "");
- if (verbose)
- printf ("%s\n", cmd);
- r = system (cmd);
- if (r == -1)
- perror ("system");
- else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
- fprintf (stderr, "warning: udevadm command failed\n");
+ ADD_ARG (argv, i, str_udevadm);
+ if (verbose) {
+ ADD_ARG (argv, i, "--debug");
+ }
+
+ ADD_ARG (argv, i, "settle");
+ if (file) {
+ ADD_ARG (argv, i, "-E");
+ ADD_ARG (argv, i, file);
+ }
+ ADD_ARG (argv, i, NULL);
+
+ r = commandv (NULL, &err, argv);
+ if (r == -1) {
+ reply_with_error ("udevadm command failed: %s", err);
IMHO reply_with_error is not correct here -- so far a failure to run
udevadm is not fatal at all, so it should be kept that way.
Also most probably commandrv, so we can still
- show perror when udevadm could not be run (return code = -1)
- show the warning when udevadm failed to run (exit code != 0)
The rest of the changes looks good.
Thanks,
--
Pino Toscano
11:01 a.m.
New subject: [PATCH v2 1/2] daemon: run 'udevadm settle' with --exit-if-exists option
On 13.04.2017 18:10, Pino Toscano wrote:
On Thursday, 13 April 2017 16:55:26 CEST 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 | 39 ++++++++++++++++++++++++++++-----------
> 2 files changed, 30 insertions(+), 11 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..b82c98acd 100644
> --- a/daemon/guestfsd.c
> +++ b/daemon/guestfsd.c
> @@ -63,6 +63,8 @@
>
> #include "daemon.h"
>
> +#define MAX_ARGS 64
> +
> GUESTFSD_EXT_CMD(str_udevadm, udevadm);
> GUESTFSD_EXT_CMD(str_uuidgen, uuidgen);
>
> @@ -1213,20 +1215,35 @@ random_name (char *template)
> * fussed if it fails.
> */
> void
> -udev_settle (void)
> +udev_settle_file (const char *file)
> {
> - char cmd[80];
> + const char *argv[MAX_ARGS];
> + CLEANUP_FREE char *err = NULL;
> + size_t i = 0;
> int r;
>
> - snprintf (cmd, sizeof cmd, "%s%s settle",
> - str_udevadm, verbose ? " --debug" : "");
> - if (verbose)
> - printf ("%s\n", cmd);
> - r = system (cmd);
> - if (r == -1)
> - perror ("system");
> - else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
> - fprintf (stderr, "warning: udevadm command failed\n");
> + ADD_ARG (argv, i, str_udevadm);
> + if (verbose) {
> + ADD_ARG (argv, i, "--debug");
> + }
> +
> + ADD_ARG (argv, i, "settle");
> + if (file) {
> + ADD_ARG (argv, i, "-E");
> + ADD_ARG (argv, i, file);
> + }
> + ADD_ARG (argv, i, NULL);
> +
> + r = commandv (NULL, &err, argv);
> + if (r == -1) {
> + reply_with_error ("udevadm command failed: %s", err);
IMHO reply_with_error is not correct here -- so far a failure to run
udevadm is not fatal at all, so it should be kept that way.
I missed that that reply_with_error is not just error output, but may
lead to termination of the client. Agree.
Also most probably commandrv, so we can still
- show perror when udevadm could not be run (return code = -1)
- show the warning when udevadm failed to run (exit code != 0)
will fix.
The rest of the changes looks good.
Thanks! Can you add me in CC when you reply to my messages?
Thanks,
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
11:06 a.m.
New subject: [PATCH v2 1/2] daemon: run 'udevadm settle' with --exit-if-exists option
On Thu, Apr 13, 2017 at 07:01:09PM +0300, Pavel Butsykin wrote:
I missed that that reply_with_error is not just error output, but
may
lead to termination of the client. Agree.
That's right -- reply_with_error sends a protocol message back to the
client, so it's important to only call it when the client (ie. the
library) is expecting it. In this case, just ignore the error or call
perror or fprintf (stderr, ...).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
9:55 a.m.
New subject: [PATCH v2 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 b82c98acd..e6a4c65c4 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -404,6 +404,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
11:07 a.m.
New subject: [PATCH v2 2/2] daemon: add udev_settle_file to is_root_device
On Thu, Apr 13, 2017 at 05:55:27PM +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 b82c98acd..e6a4c65c4 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -404,6 +404,9 @@ int
is_root_device (const char *device)
{
struct stat statbuf;
+
+ udev_settle_file (device);
+
if (stat (device, &statbuf) == -1) {
perror (device);
return 0;
This one is fine now, so ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
2780
days inactive
2780
days old
6 comments
3 participants
participants (3)
-
Pavel Butsykin -
Pino Toscano -
Richard W.M. Jones