Libguestfs - Jul 2016 - [PATCH v2 0/4] Improve LVM handling in the appliance

Hi,

this series improves the way LVM is used in the appliance: in
particular, now lvmetad can eventually run at all, and with the correct
configuration.

Also improve the listing strategies.

Changes in v2:
- dropped patch #5, will be sent separately
- move lvmetad statup in own function (patch #2)

Thanks,


Pino Toscano (4):
  daemon: lvm-filter: set also global_filter
  daemon: lvm-filter: start lvmetad better
  daemon: lvm: improve filter for LVs with activationskip flag set
  appliance: run systemd-tmpfiles also for /run

 appliance/init      |  3 +--
 daemon/daemon.h     |  1 +
 daemon/guestfsd.c   |  4 +++-
 daemon/lvm-filter.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
 daemon/lvm.c        |  4 ++--
 5 files changed, 53 insertions(+), 9 deletions(-)

-- 
2.7.4

When editing the lvm configuration to set the LVM filter, edit the
'global_filter' key in addition to 'filter': the latter is not
used when
lvmetad is running, when only the former works.
---
 daemon/lvm-filter.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c
index 44045b3..8629aab 100644
--- a/daemon/lvm-filter.c
+++ b/daemon/lvm-filter.c
@@ -148,7 +148,12 @@ set_filter (char *const *filters)
   /* Remove all the old filters ... */
   r = aug_rm (aug, "/files/lvm/lvm.conf/devices/dict/filter/list/*");
   if (r == -1) {
-    AUGEAS_ERROR ("aug_rm");
+    AUGEAS_ERROR ("aug_rm/filter");
+    return -1;
+  }
+  r = aug_rm (aug,
"/files/lvm/lvm.conf/devices/dict/global_filter/list/*");
+  if (r == -1) {
+    AUGEAS_ERROR ("aug_rm/global_filter");
     return -1;
   }
 
@@ -161,7 +166,16 @@ set_filter (char *const *filters)
               count + 1);
 
     if (aug_set (aug, buf, filters[count]) == -1) {
-      AUGEAS_ERROR ("aug_set: %d: %s", count, filters[count]);
+      AUGEAS_ERROR ("aug_set/filter: %d: %s", count, filters[count]);
+      return -1;
+    }
+
+    snprintf (buf, sizeof buf,
+             
"/files/lvm/lvm.conf/devices/dict/global_filter/list/%d/str",
+              count + 1);
+
+    if (aug_set (aug, buf, filters[count]) == -1) {
+      AUGEAS_ERROR ("aug_set/global_filter: %d: %s", count,
filters[count]);
       return -1;
     }
   }
@@ -170,13 +184,23 @@ set_filter (char *const *filters)
   r = aug_match (aug,
"/files/lvm/lvm.conf/devices/dict/filter/list/*/str",
                  NULL);
   if (r == -1) {
-    AUGEAS_ERROR ("aug_match");
+    AUGEAS_ERROR ("aug_match/filter");
     return -1;
   }
   if (r != count) {
     reply_with_error ("filters# vs matches mismatch: %d vs %d",
count, r);
     return -1;
   }
+  r = aug_match (aug,
"/files/lvm/lvm.conf/devices/dict/global_filter/list/*/str",
+                 NULL);
+  if (r == -1) {
+    AUGEAS_ERROR ("aug_match/global_filter");
+    return -1;
+  }
+  if (r != count) {
+    reply_with_error ("global_filter# vs matches mismatch: %d vs %d",
count, r);
+    return -1;
+  }
 
   if (aug_save (aug) == -1) {
     AUGEAS_ERROR ("aug_save");
-- 
2.7.4

Currently lvmetad is started in init, and thus using the system
(= appliance) configuration of lvm.  Later on, in the daemon, a local
copy of the lvm configuration is setup, and set it for use using the
LVM_SYSTEM_DIR environment variable: this means only the programmes
executed by the daemon will use the local lvm configuration, and not
lvmetad.

Thus manually start lvmetad from the daemon, right after having setup
the local lvm configuration, and still without failing if it cannot be
executed.

Additionally, since lvmetad now respects the right configuration, make
sure to update its cache when rescanning the VGs by passing --cache to
vgscan.
---
 appliance/init      |  1 -
 daemon/daemon.h     |  1 +
 daemon/guestfsd.c   |  4 +++-
 daemon/lvm-filter.c | 20 +++++++++++++++++++-
 daemon/lvm.c        |  2 +-
 5 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/appliance/init b/appliance/init
index 3816dfd..d440007 100755
--- a/appliance/init
+++ b/appliance/init
@@ -131,7 +131,6 @@ mdadm -As --auto=yes --run
 
 # Scan for LVM.
 modprobe dm_mod ||:
-lvmetad ||:
 
 lvm vgchange -aay --sysinit
 
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 7716981..9fb0472 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -189,6 +189,7 @@ extern int lv_canonical (const char *device, char **ret);
 
 /*-- in lvm-filter.c --*/
 extern void copy_lvm (void);
+extern void start_lvmetad (void);
 
 /*-- in zero.c --*/
 extern void wipe_device_before_mkfs (const char *device);
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index bd3f7de..b343d7e 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -274,8 +274,10 @@ main (int argc, char *argv[])
   /* Make a private copy of /etc/lvm so we can change the config (see
    * daemon/lvm-filter.c).
    */
-  if (!test_mode)
+  if (!test_mode) {
     copy_lvm ();
+    start_lvmetad ();
+  }
 
   /* Connect to virtio-serial channel. */
   if (!channel)
diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c
index 8629aab..d53d64b 100644
--- a/daemon/lvm-filter.c
+++ b/daemon/lvm-filter.c
@@ -39,6 +39,7 @@
 GUESTFSD_EXT_CMD(str_lvm, lvm);
 GUESTFSD_EXT_CMD(str_cp, cp);
 GUESTFSD_EXT_CMD(str_rm, rm);
+GUESTFSD_EXT_CMD(str_lvmetad, lvmetad);
 
 /* This runs during daemon start up and creates a complete copy of
  * /etc/lvm so that we can modify it as we desire.  We set
@@ -101,6 +102,23 @@ copy_lvm (void)
   atexit (rm_lvm_system_dir);
 }
 
+/* Try to run lvmetad, without failing if it couldn't. */
+void
+start_lvmetad (void)
+{
+  char cmd[64];
+  int r;
+
+  snprintf (cmd, sizeof cmd, "%s", str_lvmetad);
+  if (verbose)
+    printf ("%s\n", cmd);
+  r = system (cmd);
+  if (r == -1)
+    perror ("system/lvmetad");
+  else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
+    fprintf (stderr, "warning: lvmetad command failed\n");
+}
+
 static void
 rm_lvm_system_dir (void)
 {
@@ -247,7 +265,7 @@ rescan (void)
   unlink (lvm_cache);
 
   CLEANUP_FREE char *err = NULL;
-  int r = command (NULL, &err, str_lvm, "vgscan", NULL);
+  int r = command (NULL, &err, str_lvm, "vgscan",
"--cache", NULL);
   if (r == -1) {
     reply_with_error ("vgscan: %s", err);
     return -1;
diff --git a/daemon/lvm.c b/daemon/lvm.c
index d4909ad..e21f7c1 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -794,7 +794,7 @@ do_vgscan (void)
   int r;
 
   r = command (NULL, &err,
-               str_lvm, "vgscan", NULL);
+               str_lvm, "vgscan", "--cache", NULL);
   if (r == -1) {
     reply_with_error ("%s", err);
     return -1;
-- 
2.7.4

Commit 2e16e3e99324112845446c82b6a6e8b3e652e10d added lv_active=active
as additional condition when listing LVs, to ignore those with the
activationskip flag set.  OTOH, this check is too broad, and matches
also other kind of LVs.

Change the condition to lv_skip_activation!=1, so matching precisely
what was meant, and only that.

Related to: RHBZ#1306666
---
 daemon/lvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemon/lvm.c b/daemon/lvm.c
index e21f7c1..831c56e 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -264,7 +264,7 @@ do_lvs (void)
     r = command (&out, &err,
                  str_lvm, "lvs",
                  "-o", "vg_name,lv_name",
-                 "-S", "lv_role=public &&
lv_active=active",
+                 "-S", "lv_role=public &&
lv_skip_activation!=1",
                  "--noheadings",
                  "--separator", "/", NULL);
     if (r == -1) {
-- 
2.7.4

Setup the volatile /run in the appliance also with the tmpfiles
configurations available.  In particular, setting up correctly the lvm
bits allow lvmetad to run.
---
 appliance/init | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/appliance/init b/appliance/init
index d440007..e678e42 100755
--- a/appliance/init
+++ b/appliance/init
@@ -88,7 +88,7 @@ machine_id=$(dd if=/dev/urandom bs=16 count=1 status=none | od
-x -A n)
 echo "${machine_id// /}" > /etc/machine-id
 
 # Set up tmpfiles (must run after kmod.conf is created above).
-systemd-tmpfiles --prefix=/dev --create --boot
+systemd-tmpfiles --prefix=/dev --prefix=/run --create --boot
 
 # Find udevd and run it directly.
 for f in /lib/systemd/systemd-udevd /usr/lib/systemd/systemd-udevd \
-- 
2.7.4

On Tue, Jul 26, 2016 at 06:43:01PM +0200, Pino Toscano
wrote:
> Setup the volatile /run in the appliance also with the tmpfiles
> configurations available.  In particular, setting up correctly the lvm
> bits allow lvmetad to run.
> ---
>  appliance/init | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/appliance/init b/appliance/init
> index d440007..e678e42 100755
> --- a/appliance/init
> +++ b/appliance/init
> @@ -88,7 +88,7 @@ machine_id=$(dd if=/dev/urandom bs=16 count=1 status=none
| od -x -A n)
>  echo "${machine_id// /}" > /etc/machine-id
>  
>  # Set up tmpfiles (must run after kmod.conf is created above).
> -systemd-tmpfiles --prefix=/dev --create --boot
> +systemd-tmpfiles --prefix=/dev --prefix=/run --create --boot
>  
>  # Find udevd and run it directly.
>  for f in /lib/systemd/systemd-udevd /usr/lib/systemd/systemd-udevd \
ACK series.

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