Libguestfs - Mar 2022 - [PATCH p2v] ssh: Allow interop with virt-v2v 2.0

The code previously checked that the remote version of virt-v2v was
1.x and >= 1.28.  Since virt-v2v 2.0 has been released and is broadly
compatible, allow 2.0 as well.
---
 ssh.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ssh.c b/ssh.c
index ce0caaa..7ac43eb 100644
--- a/ssh.c
+++ b/ssh.c
@@ -1030,11 +1030,11 @@ add_output_driver (const char *name, size_t len)
 static int
 compatible_version (const char *v2v_version)
 {
-  unsigned v2v_minor;
+  unsigned v2v_major, v2v_minor;
 
-  /* The major version must always be 1. */
-  if (!STRPREFIX (v2v_version, "1.")) {
-    set_ssh_error ("virt-v2v major version is not 1 (\"%s\"),
"
+  /* The major version must always be 1 or 2. */
+  if (! (STRPREFIX (v2v_version, "1.") || STRPREFIX (v2v_version,
"2."))) {
+    set_ssh_error ("virt-v2v major version is not 1 or 2
(\"%s\"), "
                    "this version of virt-p2v is not compatible.",
                    v2v_version);
     return 0;
@@ -1045,13 +1045,13 @@ compatible_version (const char *v2v_version)
    * that we published during development, nor (b) using old virt-v2v.
    * We should remain compatible with any virt-v2v after 1.28.
    */
-  if (sscanf (v2v_version, "1.%u", &v2v_minor) != 1) {
+  if (sscanf (v2v_version, "%u.%u", &v2v_major, &v2v_minor)
!= 2) {
     set_ssh_internal_error ("cannot parse virt-v2v version string
(\"%s\")",
                             v2v_version);
     return 0;
   }
 
-  if (v2v_minor < 28) {
+  if (v2v_major == 1 && v2v_minor < 28) {
     set_ssh_error ("virt-v2v version is < 1.28 (\"%s\"),
"
                    "you must upgrade to virt-v2v >= 1.28 on "
                    "the conversion server.", v2v_version);
-- 
2.35.1

On 03/23/22 18:04, Richard W.M. Jones wrote:
> The code previously checked that the remote version of virt-v2v was
> 1.x and >= 1.28.  Since virt-v2v 2.0 has been released and is broadly
> compatible, allow 2.0 as well.
> ---
>  ssh.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/ssh.c b/ssh.c
> index ce0caaa..7ac43eb 100644
> --- a/ssh.c
> +++ b/ssh.c
> @@ -1030,11 +1030,11 @@ add_output_driver (const char *name, size_t len)
>  static int
>  compatible_version (const char *v2v_version)
>  {
> -  unsigned v2v_minor;
> +  unsigned v2v_major, v2v_minor;
>  
> -  /* The major version must always be 1. */
> -  if (!STRPREFIX (v2v_version, "1.")) {
> -    set_ssh_error ("virt-v2v major version is not 1
(\"%s\"), "
> +  /* The major version must always be 1 or 2. */
> +  if (! (STRPREFIX (v2v_version, "1.") || STRPREFIX
(v2v_version, "2."))) {
> +    set_ssh_error ("virt-v2v major version is not 1 or 2
(\"%s\"), "
>                     "this version of virt-p2v is not
compatible.",
>                     v2v_version);
>      return 0;
I have a personal preference for avoiding such "outermost" logical
negations; I always move negations as far inside as possible, using De
Morgan's Laws:

  !STRPREFIX (v2v_version, "1.") && !STRPREFIX (v2v_version,
"2.")

However, this is just a style question, and the logic is certainly correct.

Somewhat relatedly, regarding the English language error message, I
think we could slightly clarify it:

  virt-v2v major version is neither 1 nor 2

The rest looks OK to me.

With or without the msg update:

Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks
Laszlo
> @@ -1045,13 +1045,13 @@ compatible_version (const char *v2v_version)
>     * that we published during development, nor (b) using old virt-v2v.
>     * We should remain compatible with any virt-v2v after 1.28.
>     */
> -  if (sscanf (v2v_version, "1.%u", &v2v_minor) != 1) {
> +  if (sscanf (v2v_version, "%u.%u", &v2v_major,
&v2v_minor) != 2) {
>      set_ssh_internal_error ("cannot parse virt-v2v version string
(\"%s\")",
>                              v2v_version);
>      return 0;
>    }
>  
> -  if (v2v_minor < 28) {
> +  if (v2v_major == 1 && v2v_minor < 28) {
>      set_ssh_error ("virt-v2v version is < 1.28 (\"%s\"),
"
>                     "you must upgrade to virt-v2v >= 1.28 on "
>                     "the conversion server.", v2v_version);
>