Libguestfs - May 2023 - [libguestfs-common PATCH v2 2/2] options/keys: introduce unescape_device_mapper_lvm()

Libguestfs uses the

  /dev/VG/LV

format internally, for identifying LVM logical volumes, but the user might
want to specify the

  /dev/mapper/VG-LV ID

format with the "--key ID:..." options.

Introduce unescape_device_mapper_lvm() for turning

  /dev/mapper/VG-LV

key IDs into

  /dev/VG/LV

ones, unescaping doubled hyphens to single hyphens in both VG and LV in
the process.

Call unescape_device_mapper_lvm() from key_store_import_key(). That is,
translate the ID as soon as the "--key" option is processed -- let the
keystore only know about the usual /dev/VG/LV format, for later matching.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2168506
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - rewrite without regular expressions [Rich, Laszlo]
    
    - restructure the commit message
    
    v1:
    
    regcomp() must definitely allocate memory dynamically, and we
    (intentionally) never free that -- we never call regfree(). I assume
    valgrind will catch this as a leak, so we might have to extend
    "valgrind-suppressions" in each dependent superproject. However,
I'm
    unable to run "make check-valgrind" successfully in e.g. virt-v2v
even
    before these patches; see also
    <https://listman.redhat.com/archives/libguestfs/2023-May/031496.html>.

 options/keys.c | 100 ++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/options/keys.c b/options/keys.c
index bc7459749276..52b27369016a 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -260,6 +260,105 @@ key_store_add_from_selector (struct key_store *ks, const
char *selector)
   return key_store_import_key (ks, &key);
 }
 
+/* Turn /dev/mapper/VG-LV into /dev/VG/LV, in-place. */
+static void
+unescape_device_mapper_lvm (char *id)
+{
+  static const char dev[] = "/dev/", dev_mapper[] =
"/dev/mapper/";
+  const char *input_start;
+  char *output;
+  enum { M_SCAN, M_FILL, M_DONE } mode;
+
+  if (!STRPREFIX (id, dev_mapper))
+    return;
+
+  /* Start parsing "VG-LV" from "id" after
"/dev/mapper/". */
+  input_start = id + (sizeof dev_mapper - 1);
+
+  /* Start writing the unescaped "VG/LV" output after
"/dev/". */
+  output = id + (sizeof dev - 1);
+
+  for (mode = M_SCAN; mode < M_DONE; ++mode) {
+    char c;
+    const char *input = input_start;
+    const char *hyphen_buffered = NULL;
+    bool single_hyphen_seen = false;
+
+    do {
+      c = *input;
+
+      switch (c) {
+      case '-':
+        if (hyphen_buffered == NULL)
+          /* This hyphen may start an escaped hyphen, or it could be the
+           * separator in VG-LV.
+           */
+          hyphen_buffered = input;
+        else {
+          /* This hyphen completes an escaped hyphen; unescape it. */
+          if (mode == M_FILL)
+            *output++ = '-';
+          hyphen_buffered = NULL;
+        }
+        break;
+
+      case '/':
+        /* Slash characters are forbidden in VG-LV anywhere. If there's
any,
+         * we'll find it in the first (i.e., scanning) phase, before we
output
+         * anything back to "id".
+         */
+        assert (mode == M_SCAN);
+        return;
+
+      default:
+        /* Encountered a non-slash, non-hyphen character -- which also may be
+         * the terminating NUL.
+         */
+        if (hyphen_buffered != NULL) {
+          /* The non-hyphen character comes after a buffered hyphen, so the
+           * buffered hyphen is supposed to be the single hyphen that separates
+           * VG from LV in VG-LV. There are three requirements for this
+           * separator: (a) it must be unique (we must not have seen another
+           * such separator earlier), (b) it must not be at the start of VG-LV
+           * (because VG would be empty that way), (c) it must not be at the
end
+           * of VG-LV (because LV would be empty that way). Should any of these
+           * be violated, we'll catch that during the first (i.e.,
scanning)
+           * phase, before modifying "id".
+           */
+          if (single_hyphen_seen || hyphen_buffered == input_start ||
+              c == '\0') {
+            assert (mode == M_SCAN);
+            return;
+          }
+
+          /* Translate the separator hyphen to a slash character. */
+          if (mode == M_FILL)
+            *output++ = '/';
+          hyphen_buffered = NULL;
+          single_hyphen_seen = true;
+        }
+
+        /* Output the non-hyphen character (including the terminating NUL)
+         * regardless of whether there was a buffered hyphen separator (which,
+         * by now, we'll have attempted to translate and flush).
+         */
+        if (mode == M_FILL)
+          *output++ = c;
+      }
+
+      ++input;
+    } while (c != '\0');
+
+    /* We must have seen the VG-LV separator. If that's not the case,
we'll
+     * catch it before modifying "id".
+     */
+    if (!single_hyphen_seen) {
+      assert (mode == M_SCAN);
+      return;
+    }
+  }
+}
+
 struct key_store *
 key_store_import_key (struct key_store *ks, struct key_store_key *key)
 {
@@ -278,6 +377,7 @@ key_store_import_key (struct key_store *ks, struct
key_store_key *key)
     error (EXIT_FAILURE, errno, "realloc");
 
   ks->keys = new_keys;
+  unescape_device_mapper_lvm (key->id);
   ks->keys[ks->nr_keys] = *key;
   ++ks->nr_keys;

On Thu, May 18, 2023 at 03:09:42PM +0200, Laszlo Ersek
wrote:
> Libguestfs uses the
> 
>   /dev/VG/LV
> 
> format internally, for identifying LVM logical volumes, but the user might
> want to specify the
> 
>   /dev/mapper/VG-LV ID
> 
> format with the "--key ID:..." options.
> 
> Introduce unescape_device_mapper_lvm() for turning
> 
>   /dev/mapper/VG-LV
> 
> key IDs into
> 
>   /dev/VG/LV
> 
> ones, unescaping doubled hyphens to single hyphens in both VG and LV in
> the process.
> 
> Call unescape_device_mapper_lvm() from key_store_import_key(). That is,
> translate the ID as soon as the "--key" option is processed --
let the
> keystore only know about the usual /dev/VG/LV format, for later matching.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2168506
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
> 
> Notes:
>     v2:
>     
>     - rewrite without regular expressions [Rich, Laszlo]
>     
>     - restructure the commit message
>     
>     v1:
>     
>     regcomp() must definitely allocate memory dynamically, and we
>     (intentionally) never free that -- we never call regfree(). I assume
>     valgrind will catch this as a leak, so we might have to extend
>     "valgrind-suppressions" in each dependent superproject.
However, I'm
>     unable to run "make check-valgrind" successfully in e.g.
virt-v2v even
>     before these patches; see also
>    
<https://listman.redhat.com/archives/libguestfs/2023-May/031496.html>.
> 
>  options/keys.c | 100 ++++++++++++++++++++
>  1 file changed, 100 insertions(+)
> 
> diff --git a/options/keys.c b/options/keys.c
> index bc7459749276..52b27369016a 100644
> --- a/options/keys.c
> +++ b/options/keys.c
> @@ -260,6 +260,105 @@ key_store_add_from_selector (struct key_store *ks,
const char *selector)
>    return key_store_import_key (ks, &key);
>  }
>  
> +/* Turn /dev/mapper/VG-LV into /dev/VG/LV, in-place. */
> +static void
> +unescape_device_mapper_lvm (char *id)
> +{
> +  static const char dev[] = "/dev/", dev_mapper[] =
"/dev/mapper/";
> +  const char *input_start;
> +  char *output;
> +  enum { M_SCAN, M_FILL, M_DONE } mode;
> +
> +  if (!STRPREFIX (id, dev_mapper))
> +    return;
> +
> +  /* Start parsing "VG-LV" from "id" after
"/dev/mapper/". */
> +  input_start = id + (sizeof dev_mapper - 1);
> +
> +  /* Start writing the unescaped "VG/LV" output after
"/dev/". */
> +  output = id + (sizeof dev - 1);
> +
> +  for (mode = M_SCAN; mode < M_DONE; ++mode) {
> +    char c;
> +    const char *input = input_start;
> +    const char *hyphen_buffered = NULL;
> +    bool single_hyphen_seen = false;
> +
> +    do {
> +      c = *input;
> +
> +      switch (c) {
> +      case '-':
> +        if (hyphen_buffered == NULL)
> +          /* This hyphen may start an escaped hyphen, or it could be the
> +           * separator in VG-LV.
> +           */
> +          hyphen_buffered = input;
> +        else {
> +          /* This hyphen completes an escaped hyphen; unescape it. */
> +          if (mode == M_FILL)
> +            *output++ = '-';
> +          hyphen_buffered = NULL;
> +        }
> +        break;
> +
> +      case '/':
> +        /* Slash characters are forbidden in VG-LV anywhere. If
there's any,
> +         * we'll find it in the first (i.e., scanning) phase, before
we output
> +         * anything back to "id".
> +         */
> +        assert (mode == M_SCAN);
> +        return;
> +
> +      default:
> +        /* Encountered a non-slash, non-hyphen character -- which also may
be
> +         * the terminating NUL.
> +         */
> +        if (hyphen_buffered != NULL) {
> +          /* The non-hyphen character comes after a buffered hyphen, so
the
> +           * buffered hyphen is supposed to be the single hyphen that
separates
> +           * VG from LV in VG-LV. There are three requirements for this
> +           * separator: (a) it must be unique (we must not have seen
another
> +           * such separator earlier), (b) it must not be at the start of
VG-LV
> +           * (because VG would be empty that way), (c) it must not be at
the end
> +           * of VG-LV (because LV would be empty that way). Should any of
these
> +           * be violated, we'll catch that during the first (i.e.,
scanning)
> +           * phase, before modifying "id".
> +           */
> +          if (single_hyphen_seen || hyphen_buffered == input_start ||
> +              c == '\0') {
> +            assert (mode == M_SCAN);
> +            return;
> +          }
> +
> +          /* Translate the separator hyphen to a slash character. */
> +          if (mode == M_FILL)
> +            *output++ = '/';
> +          hyphen_buffered = NULL;
> +          single_hyphen_seen = true;
> +        }
> +
> +        /* Output the non-hyphen character (including the terminating NUL)
> +         * regardless of whether there was a buffered hyphen separator
(which,
> +         * by now, we'll have attempted to translate and flush).
> +         */
> +        if (mode == M_FILL)
> +          *output++ = c;
> +      }
> +
> +      ++input;
> +    } while (c != '\0');
> +
> +    /* We must have seen the VG-LV separator. If that's not the case,
we'll
> +     * catch it before modifying "id".
> +     */
> +    if (!single_hyphen_seen) {
> +      assert (mode == M_SCAN);
> +      return;
> +    }
> +  }
> +}
So this code can never return an error?  eg if the input was "A-B-C",
I think it would translate the string to "A/B-C" which is an output
but the input seems like it was wrong.  Or is it that only the VG is
escaped?  (I don't imagine there is some specification for /dev/mapper
device names.)

Rich.
>  struct key_store *
>  key_store_import_key (struct key_store *ks, struct key_store_key *key)
>  {
> @@ -278,6 +377,7 @@ key_store_import_key (struct key_store *ks, struct
key_store_key *key)
>      error (EXIT_FAILURE, errno, "realloc");
>  
>    ks->keys = new_keys;
> +  unescape_device_mapper_lvm (key->id);
>    ks->keys[ks->nr_keys] = *key;
>    ++ks->nr_keys;
>  
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://listman.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-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