thr3ads.net - Libguestfs - [Libguestfs] [PATCH 0/5] Improve LVM handling in the appliance [Jul 2016]

If this information is useful, please help other people find it:
Share via:

Pino Toscano

2016-Jul-26 15:41 UTC

[Libguestfs] [PATCH 0/5] 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.

Thanks,


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

 appliance/init      |  3 +--
 daemon/lvm-filter.c | 43 +++++++++++++++++++++++++++++++++++++++----
 daemon/lvm.c        | 14 ++++++++------
 generator/daemon.ml |  1 +
 4 files changed, 49 insertions(+), 12 deletions(-)

-- 
2.7.4

Pino Toscano

2016-Jul-26 15:41 UTC

head link

[Libguestfs] [PATCH 1/5] daemon: lvm-filter: set also global_filter

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

Pino Toscano

2016-Jul-26 15:41 UTC

head link

[Libguestfs] [PATCH 2/5] daemon: lvm-filter: start lvmetad better

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/lvm-filter.c | 13 ++++++++++++-
 daemon/lvm.c        |  2 +-
 3 files changed, 13 insertions(+), 3 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/lvm-filter.c b/daemon/lvm-filter.c
index 8629aab..2add95a 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
@@ -97,6 +98,16 @@ copy_lvm (void)
   snprintf (env, sizeof env, "%s/lvm", lvm_system_dir);
   setenv ("LVM_SYSTEM_DIR", env, 1);
 
+  /* Try to run lvmetad, without failing if it couldn't. */
+  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");
+
   /* Set a handler to remove the temporary directory at exit. */
   atexit (rm_lvm_system_dir);
 }
@@ -247,7 +258,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

Pino Toscano

2016-Jul-26 15:41 UTC

head link

[Libguestfs] [PATCH 3/5] daemon: lvm: improve filter for LVs with activationskip flag set

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

Pino Toscano

2016-Jul-26 15:41 UTC

head link

[Libguestfs] [PATCH 4/5] daemon: lvm: list PVs/VGs/LVs with --foreign

The appliance has no LVM system ID set, which means that lvm commands
will ignore VGs with a system ID set to anything.  Since we want to work
with them, pass --foreign at least when listing them to see them.

See also lvmsystemid(7).
---
 daemon/lvm.c        | 10 ++++++----
 generator/daemon.ml |  1 +
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/daemon/lvm.c b/daemon/lvm.c
index 831c56e..0123ae5 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -194,7 +194,7 @@ do_pvs (void)
   int r;
 
   r = command (&out, &err,
-               str_lvm, "pvs", "-o", "pv_name",
"--noheadings", NULL);
+               str_lvm, "pvs", "--foreign", "-o",
"pv_name", "--noheadings", NULL);
   if (r == -1) {
     reply_with_error ("%s", err);
     free (out);
@@ -212,7 +212,7 @@ do_vgs (void)
   int r;
 
   r = command (&out, &err,
-               str_lvm, "vgs", "-o", "vg_name",
"--noheadings", NULL);
+               str_lvm, "vgs", "--foreign", "-o",
"vg_name", "--noheadings", NULL);
   if (r == -1) {
     reply_with_error ("%s", err);
     free (out);
@@ -263,6 +263,7 @@ do_lvs (void)
   if (has_S > 0) {
     r = command (&out, &err,
                  str_lvm, "lvs",
+                 "--foreign",
                  "-o", "vg_name,lv_name",
                  "-S", "lv_role=public &&
lv_skip_activation!=1",
                  "--noheadings",
@@ -277,6 +278,7 @@ do_lvs (void)
   } else {
     r = command (&out, &err,
                  str_lvm, "lvs",
+                 "--foreign",
                  "-o", "lv_attr,vg_name,lv_name",
                  "--noheadings",
                  "--separator", ":", NULL);
@@ -718,7 +720,7 @@ get_lvm_field (const char *cmd, const char *field, const
char *device)
   char *out;
   CLEANUP_FREE char *err = NULL;
   int r = command (&out, &err,
-                   str_lvm, cmd,
+                   str_lvm, cmd, "--foreign",
                    "--unbuffered", "--noheadings",
"-o", field,
                    device, NULL);
   if (r == -1) {
@@ -755,7 +757,7 @@ get_lvm_fields (const char *cmd, const char *field, const
char *device)
   CLEANUP_FREE char *out = NULL, *err = NULL;
 
   int r = command (&out, &err,
-                   str_lvm, cmd,
+                   str_lvm, cmd, "--foreign",
                    "--unbuffered", "--noheadings",
"-o", field,
                    device, NULL);
   if (r == -1) {
diff --git a/generator/daemon.ml b/generator/daemon.ml
index 1d79126..9160beb 100644
--- a/generator/daemon.ml
+++ b/generator/daemon.ml
@@ -632,6 +632,7 @@ cleanup_free_mountable (mountable_t *mountable)
         pr "\n";
         pr "  r = command (&out, &err,\n";
         pr "	       \"lvm\", \"%ss\",\n" typ;
+        pr "	       \"--foreign\",\n";
         pr "	       \"-o\", lvm_%s_cols,
\"--unbuffered\", \"--noheadings\",\n" typ;
         pr "	       \"--nosuffix\", \"--separator\",
\"\\r\", \"--units\", \"b\", NULL);\n";
         pr "  if (r == -1) {\n";
-- 
2.7.4

Pino Toscano

2016-Jul-26 15:41 UTC

head link

[Libguestfs] [PATCH 5/5] appliance: run systemd-tmpfiles also for /run

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

Richard W.M. Jones

2016-Jul-26 15:53 UTC

head link

Re: [Libguestfs] [PATCH 2/5] daemon: lvm-filter: start lvmetad better

On Tue, Jul 26, 2016 at 05:41:26PM +0200, Pino Toscano
wrote:> 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.
Can we move the lvmetad start up to another function?  Either that or
change the name of the copy_lvm function to more accurately reflect
what it does now.

Rich.
>  appliance/init      |  1 -
>  daemon/lvm-filter.c | 13 ++++++++++++-
>  daemon/lvm.c        |  2 +-
>  3 files changed, 13 insertions(+), 3 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/lvm-filter.c b/daemon/lvm-filter.c
> index 8629aab..2add95a 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
> @@ -97,6 +98,16 @@ copy_lvm (void)
>    snprintf (env, sizeof env, "%s/lvm", lvm_system_dir);
>    setenv ("LVM_SYSTEM_DIR", env, 1);
>  
> +  /* Try to run lvmetad, without failing if it couldn't. */
> +  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");
> +
>    /* Set a handler to remove the temporary directory at exit. */
>    atexit (rm_lvm_system_dir);
>  }
> @@ -247,7 +258,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
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Richard W.M. Jones

2016-Jul-26 15:59 UTC

head link

Re: [Libguestfs] [PATCH 4/5] daemon: lvm: list PVs/VGs/LVs with --foreign

On Tue, Jul 26, 2016 at 05:41:28PM +0200, Pino Toscano
wrote:> The appliance has no LVM system ID set, which means that lvm commands
> will ignore VGs with a system ID set to anything.  Since we want to work
> with them, pass --foreign at least when listing them to see them.
> 
> See also lvmsystemid(7).
This is sort of a hack, if I'm understanding correctly.  Can we not
instead give the appliance a "system ID"?

Also, lvm(8) says:

       --foreign
              Cause the command to access foreign VGs, that would otherwise be
              skipped.  It can be used to report or display a VG that is owned
              by  another  host.   This  option can cause a command to perform
              poorly because lvmetad caching is not used and metadata is  read
              from disks.

which sounds bad.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Richard W.M. Jones

2016-Jul-26 16:00 UTC

head link

Re: [Libguestfs] [PATCH 5/5] appliance: run systemd-tmpfiles also for /run

On Tue, Jul 26, 2016 at 05:41:29PM +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 patches 1, 3 & 5.

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

Possibly Parallel Threads

Search for more apparently analagous threads

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

[Libguestfs] [PATCH 0/5] Improve LVM handling in the appliance

[Libguestfs] [PATCH 1/5] daemon: lvm-filter: set also global_filter

[Libguestfs] [PATCH 2/5] daemon: lvm-filter: start lvmetad better

[Libguestfs] [PATCH 3/5] daemon: lvm: improve filter for LVs with activationskip flag set

[Libguestfs] [PATCH 4/5] daemon: lvm: list PVs/VGs/LVs with --foreign

[Libguestfs] [PATCH 5/5] appliance: run systemd-tmpfiles also for /run

Re: [Libguestfs] [PATCH 2/5] daemon: lvm-filter: start lvmetad better

Re: [Libguestfs] [PATCH 4/5] daemon: lvm: list PVs/VGs/LVs with --foreign

Re: [Libguestfs] [PATCH 5/5] appliance: run systemd-tmpfiles also for /run

Possibly Parallel Threads