From 39a12e8336d314a1f1e6ed1abf15e4ff10f6f81e Mon Sep 17 00:00:00 2001

Message-Id: <39a12e8336d314a1f1e6ed1abf15e4ff10f6f81e@dist-git>

From: Michal Privoznik <mprivozn@redhat.com>

Date: Mon, 10 Jan 2022 12:08:08 +0100

Subject: [PATCH] lib: Fix calling of virNetworkUpdate() driver callback

The order in which virNetworkUpdate() accepts @section and

@command arguments is not the same as in which it passes them

onto networkUpdate() callback. Until recently, it did not really

matter, because calling the API on client side meant arguments

were encoded in reversed order (compared to the public API), but

then on the server it was fixed again - because the server

decoded RPC (still swapped), called public API (still swapped)

and in turn called the network driver callback (with reversing

the order - so magically fixing the order).

Long story short, if the public API is called even number of

times those swaps cancel each other out. The problem is when the

API is called an odd numbed of times - which happens with split

daemons and the right URI. There's one call in the client (e.g.

virsh net-update), the other in a hypervisor daemon (say

virtqemud) which ends up calling the API in the virnetworkd.

The fix is obvious - fix the order in which arguments are passed

to the callback.

But, to maintain compatibility with older, yet unfixed, daemons

new connection feature is introduced. The feature is detected

just before calling the callback and allows client to pass

arguments in correct order (talking to fixed daemon) or in

reversed order (talking to older daemon).

Unfortunately, older client talking to newer daemon can't be

fixed. Let's hope that it's less frequent scenario.

Fixes: 574b9bc66b6b10cc4cf50f299c3f0ff55f2cbefb

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870552

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

(cherry picked from commit b0f78d626a18bcecae3a4d165540ab88bfbfc9ee)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2038812

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Message-Id: <4601f7b2c8ef354e0f8c8020ecd1bb20b20d0f53.1641812574.git.mprivozn@redhat.com>

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/esx/esx_driver.c                |  3 +++

 src/libvirt-network.c               | 24 ++++++++++++++++++++++--

 src/libvirt_internal.h              |  5 +++++

 src/libxl/libxl_driver.c            |  1 +

 src/lxc/lxc_driver.c                |  1 +

 src/network/bridge_driver.c         |  2 ++

 src/openvz/openvz_driver.c          |  1 +

 src/qemu/qemu_driver.c              |  1 +

 src/remote/remote_daemon_dispatch.c |  1 +

 src/test/test_driver.c              |  1 +

 10 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c

index 0ede65279a..07ce7961b0 100644

--- a/src/esx/esx_driver.c

+++ b/src/esx/esx_driver.c

@@ -1059,6 +1059,9 @@ esxConnectSupportsFeature(virConnectPtr conn, int feature)

         return priv->vCenter &&

                supportsVMotion == esxVI_Boolean_True ? 1 : 0;

+    case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:

+        return 1;

+

     case VIR_DRV_FEATURE_FD_PASSING:

     case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION:

     case VIR_DRV_FEATURE_MIGRATION_DIRECT:

diff --git a/src/libvirt-network.c b/src/libvirt-network.c

index 09e24fb0a8..9edd30d2b7 100644

--- a/src/libvirt-network.c

+++ b/src/libvirt-network.c

@@ -543,8 +543,28 @@ virNetworkUpdate(virNetworkPtr network,

     if (conn->networkDriver && conn->networkDriver->networkUpdate) {

         int ret;

-        ret = conn->networkDriver->networkUpdate(network, section, command,

-                                                 parentIndex, xml, flags);

+        int rc;

+

+        /* Since its introduction in v0.10.2-rc1~9 the @section and @command

+         * arguments were mistakenly swapped when passed to driver's callback.

+         * Detect if the other side is fixed already or not. */

+        rc = VIR_DRV_SUPPORTS_FEATURE(conn->driver, conn,

+                                      VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER);

+

+        VIR_DEBUG("Argument order feature detection returned: %d", rc);

+        if (rc < 0)

+            goto error;

+

+        if (rc == 0) {

+            /* Feature not supported, preserve swapped order */

+            ret = conn->networkDriver->networkUpdate(network, section, command,

+                                                     parentIndex, xml, flags);

+        } else {

+            /* Feature supported, correct order can be used */

+            ret = conn->networkDriver->networkUpdate(network, command, section,

+                                                     parentIndex, xml, flags);

+        }

+

         if (ret < 0)

             goto error;

         return ret;

diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h

index 4a74dbc2af..21b7243557 100644

--- a/src/libvirt_internal.h

+++ b/src/libvirt_internal.h

@@ -123,6 +123,11 @@ typedef enum {

      * Support for driver close callback rpc

      */

     VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK = 15,

+

+    /*

+     * Whether the virNetworkUpdate() API implementation passes arguments to

+     * the driver's callback in correct order. */

+    VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER = 16,

 } virDrvFeature;

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c

index 9269e9b475..827a58b2c6 100644

--- a/src/libxl/libxl_driver.c

+++ b/src/libxl/libxl_driver.c

@@ -5714,6 +5714,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature)

     case VIR_DRV_FEATURE_TYPED_PARAM_STRING:

     case VIR_DRV_FEATURE_MIGRATION_PARAMS:

     case VIR_DRV_FEATURE_MIGRATION_P2P:

+    case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:

         return 1;

     case VIR_DRV_FEATURE_FD_PASSING:

     case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION:

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c

index 853ddac8b9..8cf4dbab57 100644

--- a/src/lxc/lxc_driver.c

+++ b/src/lxc/lxc_driver.c

@@ -1699,6 +1699,7 @@ lxcConnectSupportsFeature(virConnectPtr conn, int feature)

     switch ((virDrvFeature) feature) {

     case VIR_DRV_FEATURE_TYPED_PARAM_STRING:

+    case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:

         return 1;

     case VIR_DRV_FEATURE_FD_PASSING:

     case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION:

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c

index 703348888a..cd6d09e164 100644

--- a/src/network/bridge_driver.c

+++ b/src/network/bridge_driver.c

@@ -968,6 +968,8 @@ networkConnectSupportsFeature(virConnectPtr conn, int feature)

         return -1;

     switch ((virDrvFeature) feature) {

+    case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:

+        return 1;

     case VIR_DRV_FEATURE_MIGRATION_V2:

     case VIR_DRV_FEATURE_MIGRATION_V3:

     case VIR_DRV_FEATURE_MIGRATION_P2P:

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c

index 62644f3129..22715c8e22 100644

--- a/src/openvz/openvz_driver.c

+++ b/src/openvz/openvz_driver.c

@@ -2007,6 +2007,7 @@ openvzConnectSupportsFeature(virConnectPtr conn G_GNUC_UNUSED, int feature)

     switch ((virDrvFeature) feature) {

     case VIR_DRV_FEATURE_MIGRATION_PARAMS:

     case VIR_DRV_FEATURE_MIGRATION_V3:

+    case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:

         return 1;

     case VIR_DRV_FEATURE_FD_PASSING:

     case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

index 3914d3ff68..73f6247f2e 100644

--- a/src/qemu/qemu_driver.c

+++ b/src/qemu/qemu_driver.c

@@ -1215,6 +1215,7 @@ qemuConnectSupportsFeature(virConnectPtr conn, int feature)

     case VIR_DRV_FEATURE_XML_MIGRATABLE:

     case VIR_DRV_FEATURE_MIGRATION_OFFLINE:

     case VIR_DRV_FEATURE_MIGRATION_PARAMS:

+    case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:

         return 1;

     case VIR_DRV_FEATURE_MIGRATION_DIRECT:

     case VIR_DRV_FEATURE_MIGRATION_V1:

diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c

index 9c294ddc39..b82548a999 100644

--- a/src/remote/remote_daemon_dispatch.c

+++ b/src/remote/remote_daemon_dispatch.c

@@ -5009,6 +5009,7 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr server G_GNUC_UN

     case VIR_DRV_FEATURE_XML_MIGRATABLE:

     case VIR_DRV_FEATURE_MIGRATION_OFFLINE:

     case VIR_DRV_FEATURE_MIGRATION_PARAMS:

+    case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:

     default:

         if ((supported = virConnectSupportsFeature(conn, args->feature)) < 0)

             goto cleanup;

diff --git a/src/test/test_driver.c b/src/test/test_driver.c

index 1908185743..d21fedbcf8 100644

--- a/src/test/test_driver.c

+++ b/src/test/test_driver.c

@@ -1588,6 +1588,7 @@ testConnectSupportsFeature(virConnectPtr conn G_GNUC_UNUSED,

 {

     switch ((virDrvFeature) feature) {

     case VIR_DRV_FEATURE_TYPED_PARAM_STRING:

+    case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:

         return 1;

     case VIR_DRV_FEATURE_MIGRATION_V2:

     case VIR_DRV_FEATURE_MIGRATION_V3:

-- 

2.34.1