[Libguestfs] [v2v PATCH] convert/convert_linux: complete the remapping of NVMe devices

[Richard W.M. Jones]

On Wed, Jul 06, 2022 at 12:32:15PM +0200, Laszlo Ersek wrote:
> In commit 75872bf282d7 ("input: -i vmx: Add support for NVMe devices",
> 2022-04-08), we missed that pathnames such as
> 
>   /dev/nvme0n1[p1]
> 
> would not match our "rex_device_cciss" and "rex_device" regular
> expressions.
> 
> As a consequence, we don't remap such pathnames now in the boot config
> files with Augeas.
> 
> Add a new regex and associated mapping logic for this kind of pathname.
> 
> Notes:
> 
> (1) "rex_device_cciss" could be extended internally with an alternative
> pattern:
> 
>   ^/dev/(cciss/c\\d+d\\d+|nvme\\d+n1)(?:p(\\d+))?$
>                          ^^^^^^^^^^^
> 
> but Rich suggested we should add a separate, complete regexp for
> maintainability.
> 
> (2) Even with a separate regexp, we could reuse the existent CCISS pattern
> handler:
> 
>   if PCRE.matches rex_device_cciss value ||
>      PCRE.matches rex_device_nvme value then (
>     let device = PCRE.sub 1
>     and part = try PCRE.sub 2 with Not_found -> "" in
>     "/dev/" ^ replace device ^ part
>   )
> 
> Namely, although "PCRE.matches" creates/updates global state, and
> "PCRE.sub" reads that state, the "||" operator in OCaml has short-circuit
> behavior, and both regexps have the same structure.
> 
> But, using the same maintainability argument, let's keep the handler logic
> for NVMe detached.
> 
> Fixes: 75872bf282d7f2322110caca70963717b43806b1
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2101665
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  convert/convert_linux.ml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
> index 59d143bdda4b..a66ff1e45a57 100644
> --- a/convert/convert_linux.ml
> +++ b/convert/convert_linux.ml
> @@ -1199,6 +1199,7 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ =
>      (* Map device names for each entry. *)
>      let rex_resume = PCRE.compile "^resume=(/dev/[-a-z\\d/_]+)(.*)$"
>      and rex_device_cciss = PCRE.compile "^/dev/(cciss/c\\d+d\\d+)(?:p(\\d+))?$"
> +    and rex_device_nvme = PCRE.compile "^/dev/(nvme\\d+n1)(?:p(\\d+))?$"
>      and rex_device = PCRE.compile "^/dev/([a-z]+)(\\d*)?$" in
>  
>      let rec replace_if_device path value =
> @@ -1221,6 +1222,11 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ =
>          and part = try PCRE.sub 2 with Not_found -> "" in
>          "/dev/" ^ replace device ^ part
>        )
> +      else if PCRE.matches rex_device_nvme value then (
> +        let device = PCRE.sub 1
> +        and part = try PCRE.sub 2 with Not_found -> "" in
> +        "/dev/" ^ replace device ^ part
> +      )
>        else if PCRE.matches rex_device value then (
>          let device = PCRE.sub 1
>          and part = try PCRE.sub 2 with Not_found -> "" in
> -- 
> 2.19.1.3.g30247aa5d201

Reviewed-by: Richard W.M. Jones <rjones at redhat.com>

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html