Pino Toscano
2016-Jan-27 18:06 UTC
[Libguestfs] [PATCH] lvm: support lvm2 older than 2.02.107
lvm2 2.02.107 adds the -S/--select option used in lvs to filter out only public LVs (see RHBZ#1278878). To make this work again with versions of lvm2 older than that, only on old versions filter out thin layouts and compose the resulting device strings ourselves. The filtering done is much simplier than what "-S lv_role=public" will do, but should be good enough for our need. --- daemon/lvm.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 140 insertions(+), 10 deletions(-) diff --git a/daemon/lvm.c b/daemon/lvm.c index 979cf63..604e106 100644 --- a/daemon/lvm.c +++ b/daemon/lvm.c @@ -26,6 +26,7 @@ #include <fcntl.h> #include <sys/stat.h> #include <dirent.h> +#include <assert.h> #include "daemon.h" #include "c-ctype.h" @@ -103,6 +104,85 @@ convert_lvm_output (char *out, const char *prefix) return ret.argv; } +/* Filter a colon-separated output of + * lvs -o lv_attr,vg_name,lv_name + * removing thin layouts, and building the device path as we expect it. + * + * This is used only when lvm has no -S. + */ +static char ** +filter_convert_old_lvs_output (char *out) +{ + char *p, *pend; + DECLARE_STRINGSBUF (ret); + + p = out; + while (p) { + size_t len; + char *saveptr; + char *lv_attr, *vg_name, *lv_name; + + pend = strchr (p, '\n'); /* Get the next line of output. */ + if (pend) { + *pend = '\0'; + pend++; + } + + while (*p && c_isspace (*p)) /* Skip any leading whitespace. */ + p++; + + /* Sigh, skip trailing whitespace too. "pvs", I'm looking at you. */ + len = strlen (p)-1; + while (*p && c_isspace (p[len])) + p[len--] = '\0'; + + if (!*p) { /* Empty line? Skip it. */ + skip_line: + p = pend; + continue; + } + + lv_attr = strtok_r (p, ":", &saveptr); + if (!lv_attr) + goto skip_line; + + vg_name = strtok_r (NULL, ":", &saveptr); + if (!vg_name) + goto skip_line; + + lv_name = strtok_r (NULL, ":", &saveptr); + if (!lv_name) + goto skip_line; + + /* Ignore thin layouts (RHBZ#1278878). */ + if (lv_attr[0] == 't') + goto skip_line; + + /* Ignore "unknown device" message (RHBZ#1054761). */ + if (STRNEQ (p, "unknown device")) { + char buf[256]; + + snprintf (buf, sizeof buf, "/dev/%s/%s", vg_name, lv_name); + if (add_string (&ret, buf) == -1) { + free (out); + return NULL; + } + } + + p = pend; + } + + free (out); + + if (ret.size > 0) + sort_strings (ret.argv, ret.size); + + if (end_stringsbuf (&ret) == -1) + return NULL; + + return ret.argv; +} + char ** do_pvs (void) { @@ -139,26 +219,76 @@ do_vgs (void) return convert_lvm_output (out, NULL); } +/* Check whether lvs has -S to filter its output. + * It is available only in lvm2 >= 2.02.107. + */ +static int +test_lvs_has_S_opt (void) +{ + static int result = -1; + if (result != -1) + return result; + + CLEANUP_FREE char *out = NULL; + CLEANUP_FREE char *err = NULL; + + int r = command (&out, &err, str_lvm, "lvs", "--help", NULL); + if (r == -1) { + reply_with_error ("lvm lvs --help: %s", err); + return -1; + } + + if (strstr (out, "-S") == NULL) + result = 0; + else + result = 1; + + return result; +} + char ** do_lvs (void) { char *out; CLEANUP_FREE char *err = NULL; int r; + int has_S = test_lvs_has_S_opt (); - r = command (&out, &err, - str_lvm, "lvs", - "-o", "vg_name,lv_name", - "-S", "lv_role=public", - "--noheadings", - "--separator", "/", NULL); - if (r == -1) { - reply_with_error ("%s", err); - free (out); + if (has_S < 0) return NULL; + + if (has_S > 0) { + r = command (&out, &err, + str_lvm, "lvs", + "-o", "vg_name,lv_name", + "-S", "lv_role=public", + "--noheadings", + "--separator", "/", NULL); + if (r == -1) { + reply_with_error ("%s", err); + free (out); + return NULL; + } + + return convert_lvm_output (out, "/dev/"); + } else { + r = command (&out, &err, + str_lvm, "lvs", + "-o", "lv_attr,vg_name,lv_name", + "--noheadings", + "--separator", ":", NULL); + if (r == -1) { + reply_with_error ("%s", err); + free (out); + return NULL; + } + + return filter_convert_old_lvs_output (out); } - return convert_lvm_output (out, "/dev/"); + /*NOTREACHED*/ + assert (false); + return NULL; } /* These were so complex to implement that I ended up auto-generating -- 2.5.0
Richard W.M. Jones
2016-Jan-28 13:47 UTC
Re: [Libguestfs] [PATCH] lvm: support lvm2 older than 2.02.107
On Wed, Jan 27, 2016 at 07:06:46PM +0100, Pino Toscano wrote:> char ** > do_lvs (void) > { > char *out; > CLEANUP_FREE char *err = NULL; > int r; > + int has_S = test_lvs_has_S_opt (); > + if (has_S < 0) > return NULL;I think this doesn't do the right thing on the second failing call to test_lvs_has_S_opt, because it won't return a protocol error?> + /*NOTREACHED*/ > + assert (false); > + return NULL; > }What's this bit for? It is indeed NOTREACHED, but it seems as if the 3 lines could just be removed. Rest looks fine, so ACK with those fixed. Rich. -- 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/
Pino Toscano
2016-Jan-28 13:53 UTC
Re: [Libguestfs] [PATCH] lvm: support lvm2 older than 2.02.107
On Thursday 28 January 2016 13:47:30 Richard W.M. Jones wrote:> On Wed, Jan 27, 2016 at 07:06:46PM +0100, Pino Toscano wrote: > > char ** > > do_lvs (void) > > { > > char *out; > > CLEANUP_FREE char *err = NULL; > > int r; > > + int has_S = test_lvs_has_S_opt (); > > + if (has_S < 0) > > return NULL; > > I think this doesn't do the right thing on the second failing call > to test_lvs_has_S_opt, because it won't return a protocol error?test_lvs_has_S_opt caches the result only if != -1, so if executing "lvm lvs --help" fails then an error is replied and nothing is cached; next time the command will be tried again.> > + /*NOTREACHED*/ > > + assert (false); > > + return NULL; > > } > > What's this bit for? It is indeed NOTREACHED, but it seems as if > the 3 lines could just be removed.It was a simple extra safety belt, since IIRC past versions of GCC could not recognize such case was indeed not reachable. I guess this can be added in case it is misreported. -- Pino Toscano
Pino Toscano
2016-Jan-28 15:11 UTC
[Libguestfs] [PATCH v2] lvm: support lvm2 older than 2.02.107
lvm2 2.02.107 adds the -S/--select option used in lvs to filter out only public LVs (see RHBZ#1278878). To make this work again with versions of lvm2 older than that, only on old versions filter out thin layouts and compose the resulting device strings ourselves. The filtering done is much simplier than what "-S lv_role=public" will do, but should be good enough for our need. --- daemon/lvm.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 136 insertions(+), 11 deletions(-) diff --git a/daemon/lvm.c b/daemon/lvm.c index 979cf63..8bef4d5 100644 --- a/daemon/lvm.c +++ b/daemon/lvm.c @@ -103,6 +103,85 @@ convert_lvm_output (char *out, const char *prefix) return ret.argv; } +/* Filter a colon-separated output of + * lvs -o lv_attr,vg_name,lv_name + * removing thin layouts, and building the device path as we expect it. + * + * This is used only when lvm has no -S. + */ +static char ** +filter_convert_old_lvs_output (char *out) +{ + char *p, *pend; + DECLARE_STRINGSBUF (ret); + + p = out; + while (p) { + size_t len; + char *saveptr; + char *lv_attr, *vg_name, *lv_name; + + pend = strchr (p, '\n'); /* Get the next line of output. */ + if (pend) { + *pend = '\0'; + pend++; + } + + while (*p && c_isspace (*p)) /* Skip any leading whitespace. */ + p++; + + /* Sigh, skip trailing whitespace too. "pvs", I'm looking at you. */ + len = strlen (p)-1; + while (*p && c_isspace (p[len])) + p[len--] = '\0'; + + if (!*p) { /* Empty line? Skip it. */ + skip_line: + p = pend; + continue; + } + + lv_attr = strtok_r (p, ":", &saveptr); + if (!lv_attr) + goto skip_line; + + vg_name = strtok_r (NULL, ":", &saveptr); + if (!vg_name) + goto skip_line; + + lv_name = strtok_r (NULL, ":", &saveptr); + if (!lv_name) + goto skip_line; + + /* Ignore thin layouts (RHBZ#1278878). */ + if (lv_attr[0] == 't') + goto skip_line; + + /* Ignore "unknown device" message (RHBZ#1054761). */ + if (STRNEQ (p, "unknown device")) { + char buf[256]; + + snprintf (buf, sizeof buf, "/dev/%s/%s", vg_name, lv_name); + if (add_string (&ret, buf) == -1) { + free (out); + return NULL; + } + } + + p = pend; + } + + free (out); + + if (ret.size > 0) + sort_strings (ret.argv, ret.size); + + if (end_stringsbuf (&ret) == -1) + return NULL; + + return ret.argv; +} + char ** do_pvs (void) { @@ -139,26 +218,72 @@ do_vgs (void) return convert_lvm_output (out, NULL); } +/* Check whether lvs has -S to filter its output. + * It is available only in lvm2 >= 2.02.107. + */ +static int +test_lvs_has_S_opt (void) +{ + static int result = -1; + if (result != -1) + return result; + + CLEANUP_FREE char *out = NULL; + CLEANUP_FREE char *err = NULL; + + int r = command (&out, &err, str_lvm, "lvs", "--help", NULL); + if (r == -1) { + reply_with_error ("lvm lvs --help: %s", err); + return -1; + } + + if (strstr (out, "-S") == NULL) + result = 0; + else + result = 1; + + return result; +} + char ** do_lvs (void) { char *out; CLEANUP_FREE char *err = NULL; int r; + int has_S = test_lvs_has_S_opt (); - r = command (&out, &err, - str_lvm, "lvs", - "-o", "vg_name,lv_name", - "-S", "lv_role=public", - "--noheadings", - "--separator", "/", NULL); - if (r == -1) { - reply_with_error ("%s", err); - free (out); + if (has_S < 0) return NULL; - } - return convert_lvm_output (out, "/dev/"); + if (has_S > 0) { + r = command (&out, &err, + str_lvm, "lvs", + "-o", "vg_name,lv_name", + "-S", "lv_role=public", + "--noheadings", + "--separator", "/", NULL); + if (r == -1) { + reply_with_error ("%s", err); + free (out); + return NULL; + } + + return convert_lvm_output (out, "/dev/"); + } else { + r = command (&out, &err, + str_lvm, "lvs", + "-o", "lv_attr,vg_name,lv_name", + "--noheadings", + "--separator", ":", NULL); + if (r == -1) { + reply_with_error ("%s", err); + free (out); + return NULL; + } + + return filter_convert_old_lvs_output (out); + } } /* These were so complex to implement that I ended up auto-generating -- 2.5.0
Richard W.M. Jones
2016-Jan-28 15:41 UTC
Re: [Libguestfs] [PATCH v2] lvm: support lvm2 older than 2.02.107
On Thu, Jan 28, 2016 at 04:11:30PM +0100, Pino Toscano wrote:> lvm2 2.02.107 adds the -S/--select option used in lvs to filter out only > public LVs (see RHBZ#1278878). To make this work again with versions > of lvm2 older than that, only on old versions filter out thin layouts > and compose the resulting device strings ourselves. > > The filtering done is much simplier than what "-S lv_role=public" will > do, but should be good enough for our need. > --- > daemon/lvm.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 136 insertions(+), 11 deletions(-) > > diff --git a/daemon/lvm.c b/daemon/lvm.c > index 979cf63..8bef4d5 100644 > --- a/daemon/lvm.c > +++ b/daemon/lvm.c > @@ -103,6 +103,85 @@ convert_lvm_output (char *out, const char *prefix) > return ret.argv; > } > > +/* Filter a colon-separated output of > + * lvs -o lv_attr,vg_name,lv_name > + * removing thin layouts, and building the device path as we expect it. > + * > + * This is used only when lvm has no -S. > + */ > +static char ** > +filter_convert_old_lvs_output (char *out) > +{ > + char *p, *pend; > + DECLARE_STRINGSBUF (ret); > + > + p = out; > + while (p) { > + size_t len; > + char *saveptr; > + char *lv_attr, *vg_name, *lv_name; > + > + pend = strchr (p, '\n'); /* Get the next line of output. */ > + if (pend) { > + *pend = '\0'; > + pend++; > + } > + > + while (*p && c_isspace (*p)) /* Skip any leading whitespace. */ > + p++; > + > + /* Sigh, skip trailing whitespace too. "pvs", I'm looking at you. */ > + len = strlen (p)-1; > + while (*p && c_isspace (p[len])) > + p[len--] = '\0'; > + > + if (!*p) { /* Empty line? Skip it. */ > + skip_line: > + p = pend; > + continue; > + } > + > + lv_attr = strtok_r (p, ":", &saveptr); > + if (!lv_attr) > + goto skip_line; > + > + vg_name = strtok_r (NULL, ":", &saveptr); > + if (!vg_name) > + goto skip_line; > + > + lv_name = strtok_r (NULL, ":", &saveptr); > + if (!lv_name) > + goto skip_line; > + > + /* Ignore thin layouts (RHBZ#1278878). */ > + if (lv_attr[0] == 't') > + goto skip_line; > + > + /* Ignore "unknown device" message (RHBZ#1054761). */ > + if (STRNEQ (p, "unknown device")) { > + char buf[256]; > + > + snprintf (buf, sizeof buf, "/dev/%s/%s", vg_name, lv_name); > + if (add_string (&ret, buf) == -1) { > + free (out); > + return NULL; > + } > + } > + > + p = pend; > + } > + > + free (out); > + > + if (ret.size > 0) > + sort_strings (ret.argv, ret.size); > + > + if (end_stringsbuf (&ret) == -1) > + return NULL; > + > + return ret.argv; > +} > + > char ** > do_pvs (void) > { > @@ -139,26 +218,72 @@ do_vgs (void) > return convert_lvm_output (out, NULL); > } > > +/* Check whether lvs has -S to filter its output. > + * It is available only in lvm2 >= 2.02.107. > + */ > +static int > +test_lvs_has_S_opt (void) > +{ > + static int result = -1; > + if (result != -1) > + return result; > + > + CLEANUP_FREE char *out = NULL; > + CLEANUP_FREE char *err = NULL; > + > + int r = command (&out, &err, str_lvm, "lvs", "--help", NULL); > + if (r == -1) { > + reply_with_error ("lvm lvs --help: %s", err); > + return -1; > + } > + > + if (strstr (out, "-S") == NULL) > + result = 0; > + else > + result = 1; > + > + return result; > +} > + > char ** > do_lvs (void) > { > char *out; > CLEANUP_FREE char *err = NULL; > int r; > + int has_S = test_lvs_has_S_opt (); > > - r = command (&out, &err, > - str_lvm, "lvs", > - "-o", "vg_name,lv_name", > - "-S", "lv_role=public", > - "--noheadings", > - "--separator", "/", NULL); > - if (r == -1) { > - reply_with_error ("%s", err); > - free (out); > + if (has_S < 0) > return NULL; > - } > > - return convert_lvm_output (out, "/dev/"); > + if (has_S > 0) { > + r = command (&out, &err, > + str_lvm, "lvs", > + "-o", "vg_name,lv_name", > + "-S", "lv_role=public", > + "--noheadings", > + "--separator", "/", NULL); > + if (r == -1) { > + reply_with_error ("%s", err); > + free (out); > + return NULL; > + } > + > + return convert_lvm_output (out, "/dev/"); > + } else { > + r = command (&out, &err, > + str_lvm, "lvs", > + "-o", "lv_attr,vg_name,lv_name", > + "--noheadings", > + "--separator", ":", NULL); > + if (r == -1) { > + reply_with_error ("%s", err); > + free (out); > + return NULL; > + } > + > + return filter_convert_old_lvs_output (out); > + } > }ACK. 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
Seemingly Similar Threads
- [PATCH] lvm: support lvm2 older than 2.02.107
- [PATCH 14/27] daemon: Reimplement ‘lvs’ API in OCaml.
- [PATCH] daemon: lvm: Ignore LVs with the activationskip flag set (RHBZ#1306666).
- [PATCH 4/5] daemon: lvm: list PVs/VGs/LVs with --foreign
- [PATCH] daemon: lvm: Only return public LVs from guestfs_lvs API (RHBZ#1278878).