Richard W.M. Jones
2023-Jul-20 10:24 UTC
[Libguestfs] [PATCH libguestfs] daemon: lvm: Do reverse device name translation on pvs_full device fields
Intermittent test failures in virt-filesystems showed that when using
the pvs_full API, the pv_name field in the returned list of structures
was not being reverse translated. As a result internal partition
names could appear in the output of virt-filesystems.
See: https://listman.redhat.com/archives/libguestfs/2023-July/032058.html
---
daemon/lvm.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/daemon/lvm.c b/daemon/lvm.c
index 7e76e17ccc..b8c01f718c 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -146,7 +146,34 @@ do_vgs (void)
guestfs_int_lvm_pv_list *
do_pvs_full (void)
{
- return parse_command_line_pvs ();
+ guestfs_int_lvm_pv_list *r;
+ size_t i;
+ char *din, *dout;
+
+ r = parse_command_line_pvs ();
+ if (r == NULL)
+ /* parse_command_line_pvs has already called reply_with_error */
+ return NULL;
+
+ /* The pv_name fields contain device names which must be reverse
+ * translated. The problem here is that the generator does not have
+ * a "FMountable" field type in types.mli.
+ */
+ for (i = 0; i < r->guestfs_int_lvm_pv_list_len; ++i) {
+ din = r->guestfs_int_lvm_pv_list_val[i].pv_name;
+ if (din) {
+ dout = reverse_device_name_translation (din);
+ if (!dout) {
+ /* reverse_device_name_translation has already called
reply_with_error*/
+ /* XXX memory leak here */
+ return NULL;
+ }
+ r->guestfs_int_lvm_pv_list_val[i].pv_name = dout;
+ free (din);
+ }
+ }
+
+ return r;
}
guestfs_int_lvm_vg_list *
--
2.41.0
Richard W.M. Jones
2023-Jul-20 11:39 UTC
[Libguestfs] [PATCH libguestfs] daemon: lvm: Do reverse device name translation on pvs_full device fields
This is another failure, this time in virt-diff (see attachment for full build log). virt-diff works by opening two handles to different disk images and the comparing the filesystems in each. When we stat files in the two handles: handle 'g1' returns st_dev = 2065 [8:17, /dev/sdb1] handle 'g2' returns st_dev = 2049 [8:1, /dev/sda1] (for files under the /boot mountpoint). Indeed looking at the log we can see that (as in the previous case) /dev/sda and /dev/sdb are swapped. Reverse device name translation has never even considered the case of rewriting st_dev numbers, but I guess we'll have to do that now. I wonder why this has suddenly started happening. Kernel update? I cannot reproduce it locally. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit -------------- next part -------------- A non-text attachment was scrubbed... Name: build.log.xz Type: application/x-xz Size: 24152 bytes Desc: not available URL: <http://listman.redhat.com/archives/libguestfs/attachments/20230720/dfb5344a/attachment.xz>
Richard W.M. Jones
2023-Jul-20 14:07 UTC
[Libguestfs] [PATCH libguestfs] daemon: lvm: Do reverse device name translation on pvs_full device fields
Upstream in commit 32cb5b45cf. I tested this using guestfs-tools & virt-v2v and nothing failed. However I was never able to reproduce the original issue. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org