Laszlo Ersek
2022-Jul-06 10:32 UTC
[Libguestfs] [v2v PATCH] convert/convert_linux: complete the remapping of NVMe devices
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
Richard W.M. Jones
2022-Jul-06 11:51 UTC
[Libguestfs] [v2v PATCH] convert/convert_linux: complete the remapping of NVMe devices
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.g30247aa5d201Reviewed-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