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: 574b9bc66b
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>

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:

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;

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;
 
 

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:

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:

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:

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:

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:

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;

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: