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>

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f2395abd74e6e8b24f04421752e49eb58a8aad13..c82e9921d7b2b16a1f2d01d7516145b67ed18eb8 100644 (file)
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -1036,6 +1036,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 b84389f7624b9f63f3f376bda7fbcc2b694eccad..145487d5992b73d7ae5b1fa2dc8ef6095b616bc0 100644 (file)
--- 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 2bf7744bd6f1305bacdcaad8af3b22f6c0dc7ee0..f4e592922d0537567a0ece1193545a779090092e 100644 (file)
--- a/src/libvirt_internal.h
+++ b/src/libvirt_internal.h
@@ -126,6 +126,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 23ef55cf37882e51ae018b4d2f09d68e9bed7a91..0928d2756302f4c210046fdbb3a6dfafd868c195 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5743,6 +5743,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 3fc15ff2ecec7c17787ff0767cef2c8df37fad11..c613019c7f50b00395c104e8186a5215dd3f6c16 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1651,6 +1651,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 7c9e36c6253c2ac0a72b32e4b53acd7f92114273..14db9ffc825a4938dbeb74776d4fc2f5535178e4 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -951,6 +951,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 e898af85abe30cb27306dd3635e560ee1d30d565..9f65dff5d11bf2c31443f4147104685057313465 100644 (file)
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1992,6 +1992,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 b31be76f91ca96a330d33a8d060e18cccadbceb8..3e9fc88ca15f98d012e665014e26129d222e4f26 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1238,6 +1238,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 d9181c0ca6d7337eabfb43b0e711ef12b51ed038..42c03d6cfcbebcfbc33ae61389589205465656ed 100644 (file)
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -4994,6 +4994,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 0c2ebddd228c5048641a7a3c063ed43e45fa195f..3531011a812e379d94f916502fb1b73bb4acc373 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1590,6 +1590,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: