Re: [Libguestfs] [libguestfs PATCH] appliance, daemon: disable lvm2 devicesfile

On Mon, May 30, 2022 at 04:10:27PM +0200, Laszlo Ersek wrote: > In guestfs-tools commit 4fe8a03cd2d3 ('sysprep: remove lvm2's default > "system.devices" file', 2022-04-11), we disabled the use of LVM2's new > "devicesfile" feature, which could interfere with the cloning of virtual > machines. > > We suspected in > > https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c6 > > that the same lvm2 feature could affect the libguestfs appliance itself, > but decided in > > https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c8 > https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c10 > > that this would not be the case, because "appliance/init" already > constructed a pristine LVM_SYSTEM_DIR. > > Unfortunately, that's not enough: due to the "use_devicesfile=1" default > (on RHEL9 anyway), some "lvm" invocation, possibly inside the > lvm-set-filter API, *creates* "$LVM_SYSTEM_DIR/devices/system.devices". > And then we get (minimally) warnings such as > >> Please remove the lvm.conf global_filter, it is ignored with the devices >> file. >> Please remove the lvm.conf filter, it is ignored with the devices file. > > when using the lvm-set-filter API. > > Explicitly disable the "devices file" in "appliance/init", and also > whenever we rewrite "lvm.conf" -- that is, in set_filter() > [daemon/lvm-filter.c]. In the former, check for the feature by locating > the devicesfile-related utilities "lvmdevices" and "vgimportdevices". In > the C code, invoke the utilities with the "--help" option instead. (In > "appliance/init", I thought it was best not to call any lvm2 utilities > even with "--help", with our lvm2.conf still under construction there.) If > either utility is available, set "use_devicesfile = 0". > > Cc: David Teigland <teigland(a)redhat.com&gt; > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1965941 > Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; > --- > > Notes: > Tested: > > - on top of "master" (7afbf5ee4415), using a Fedora 35 host, where the > lvm2 packages copied into the appliance do not have the "devicesfile" > feature (-> regression test); > > - on top of "rhel-9.1" (ad24b9f4d695), in the form of a Brew scratch > build, using a nightly RHEL-9.1 install, where the lvm2 packages > copied into the appliance have the "devicesfile" feature (-> bugfix > test). > > daemon/lvm-filter.c | 19 +++++++++++++++++++ > appliance/init | 11 +++++++++++ > 2 files changed, 30 insertions(+) > > diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c > index c6dd35156d37..f75e2198831e 100644 > --- a/daemon/lvm-filter.c > +++ b/daemon/lvm-filter.c > @@ -68,6 +68,18 @@ free_lvm_system_dir (void) > free (lvm_system_dir); > } > > +static bool devicesfile_feature (void)

> +{ > + static bool checked, available; > + > + if (!checked) { > + checked = true; > + available = command (NULL, NULL, "lvmdevices", "--help", NULL) == 0 || > + command (NULL, NULL, "vgimportdevices", "--help", NULL) == 0; > + } > + return available; > +} > + > /* Rewrite the 'filter = [ ... ]' line in lvm.conf. */ > static int > set_filter (char *const *filters) > @@ -88,6 +100,13 @@ set_filter (char *const *filters) > } > > fprintf (fp, "devices {\n"); > + > + /* If lvm2 supports a "devices file", we need to disable its use > + * (RHBZ#1965941). > + */ > + if (devicesfile_feature ()) > + fprintf (fp, " use_devicesfile = 0\n"); > + > for (j = 0; filter_types[j] != NULL; ++j) { > fprintf (fp, " %s = [\n", filter_types[j]); > fprintf (fp, " "); > diff --git a/appliance/init b/appliance/init > index 7076821d2250..19aa151b73aa 100755 > --- a/appliance/init > +++ b/appliance/init > @@ -134,6 +134,17 @@ mdadm -As --auto=yes --no-degraded > # Empty LVM configuration file means "all defaults". > mkdir -p /tmp/lvm > touch /tmp/lvm/lvm.conf > + > +# If lvm2 supports a "devices file", we need to disable its use > +# (RHBZ#1965941). > +if command -v lvmdevices || command -v vgimportdevices; then > + { > + printf 'devices {\n' > + printf '\tuse_devicesfile = 0\n' > + printf '}\n' > + } >> /tmp/lvm/lvm.conf > +fi > + > LVM_SYSTEM_DIR=/tmp/lvm > export LVM_SYSTEM_DIR > lvmetad > > base-commit: 7afbf5ee4415f6fa2553898d3af238e794062096 > -- > 2.19.1.3.g30247aa5d201 Looks reasonable to me, so: Acked-by: Richard W.M. Jones <rjones(a)redhat.com&gt;