[RFC] dbus-vmstate: Connect to the dbus only during the migration phase

[Priyankar Jain]

Today, dbus-vmstate maintains a constant connection to the dbus. This is
problematic for a number of reasons:
1. If dbus-vmstate is attached during power-on, then the device holds
   the unused connection for a long period of time until migration
   is triggered, thus unnecessarily occupying dbus.
2. Similarly, if the dbus is restarted in the time period between VM
   power-on (dbus-vmstate initialisation) and migration, then the
   migration will fail. The only way to recover would be by
   re-initialising the dbus-vmstate object.
3. If dbus is not available during VM power-on, then currently dbus-vmstate
   initialisation fails, causing power-on to fail.
4. For a system with large number of VMs, having multiple QEMUs connected to
   the same dbus can lead to a DoS for new connections.

To resolve these issues, this change makes dbus-vmstate connect to the dbus
only during the migration phase, inside dbus_vmstate_pre_save() and
dbus_vmstate_post_load(). The change also adds a helper, 
clear_dbus_connection(),
and calls it in the appropriate places to ensure dbus-vmstate closes the
connection properly post migration.

Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com>
Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
 backends/dbus-vmstate.c | 58 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 10 deletions(-)

diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
index bd050e8..6bff11e 100644
--- a/backends/dbus-vmstate.c
+++ b/backends/dbus-vmstate.c
@@ -80,6 +80,20 @@ get_id_list_set(DBusVMState *self)
     return g_steal_pointer(&set);
 }
 
+static void
+clear_dbus_connection(DBusVMState *self)
+{
+    g_autoptr(GError) err = NULL;
+    if (self->bus) {
+        g_dbus_connection_close_sync(self->bus, NULL, &err);
+        if (err) {
+            warn_report("%s: Failed to close the Dbus connection: %s",
+                        __func__, err->message);
+        }
+        g_clear_object(&self->bus);
+    }
+}
+
 static GHashTable *
 dbus_get_proxies(DBusVMState *self, GError **err)
 {
@@ -195,9 +209,21 @@ static int dbus_vmstate_post_load(void *opaque, int 
version_id)
 
     trace_dbus_vmstate_post_load(version_id);
 
+    self->bus = g_dbus_connection_new_for_address_sync(self->dbus_addr,
+                    G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT |
+                    G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
+                    NULL, NULL, &err);
+    if (err) {
+        error_report("%s: Failed to connect to DBus: '%s'",
+                     __func__, err->message);
+        clear_dbus_connection(self);
+        return -1;
+    }
+
     proxies = dbus_get_proxies(self, &err);
     if (!proxies) {
         error_report("%s: Failed to get proxies: %s", __func__, err->message);
+        clear_dbus_connection(self);
         return -1;
     }
 
@@ -223,6 +249,7 @@ static int dbus_vmstate_post_load(void *opaque, int 
version_id)
         if (len >= 256) {
             error_report("%s: Invalid DBus vmstate proxy name %u",
                          __func__, len);
+            clear_dbus_connection(self);
             return -1;
         }
         if (!g_input_stream_read_all(G_INPUT_STREAM(s), id, len,
@@ -237,6 +264,7 @@ static int dbus_vmstate_post_load(void *opaque, int 
version_id)
         proxy = g_hash_table_lookup(proxies, id);
         if (!proxy) {
             error_report("%s: Failed to find proxy Id '%s'", __func__, id);
+            clear_dbus_connection(self);
             return -1;
         }
 
@@ -246,6 +274,7 @@ static int dbus_vmstate_post_load(void *opaque, int 
version_id)
 
         if (len > DBUS_VMSTATE_SIZE_LIMIT || len > avail) {
             error_report("%s: Invalid vmstate size: %u", __func__, len);
+            clear_dbus_connection(self);
             return -1;
         }
 
@@ -254,6 +283,7 @@ static int dbus_vmstate_post_load(void *opaque, int 
version_id)
                                                     NULL),
                 len) < 0) {
             error_report("%s: Failed to restore Id '%s'", __func__, id);
+            clear_dbus_connection(self);
             return -1;
         }
 
@@ -264,10 +294,12 @@ static int dbus_vmstate_post_load(void *opaque, int 
version_id)
         nelem -= 1;
     }
 
+    clear_dbus_connection(self);
     return 0;
 
 error:
     error_report("%s: Failed to read from stream: %s", __func__, err->message);
+    clear_dbus_connection(self);
     return -1;
 }
 
@@ -327,9 +359,21 @@ static int dbus_vmstate_pre_save(void *opaque)
 
     trace_dbus_vmstate_pre_save();
 
+    self->bus = g_dbus_connection_new_for_address_sync(self->dbus_addr,
+                    G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT |
+                    G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
+                    NULL, NULL, &err);
+    if (err) {
+        error_report("%s: Failed to connect to DBus: '%s'",
+                     __func__, err->message);
+        clear_dbus_connection(self);
+        return -1;
+    }
+
     proxies = dbus_get_proxies(self, &err);
     if (!proxies) {
         error_report("%s: Failed to get proxies: %s", __func__, err->message);
+        clear_dbus_connection(self);
         return -1;
     }
 
@@ -341,6 +385,7 @@ static int dbus_vmstate_pre_save(void *opaque)
                                          NULL, &err)) {
         error_report("%s: Failed to write to stream: %s",
                      __func__, err->message);
+        clear_dbus_connection(self);
         return -1;
     }
 
@@ -349,11 +394,13 @@ static int dbus_vmstate_pre_save(void *opaque)
     if (g_memory_output_stream_get_size(G_MEMORY_OUTPUT_STREAM(m))
         > UINT32_MAX) {
         error_report("%s: DBus vmstate buffer is too large", __func__);
+        clear_dbus_connection(self);
         return -1;
     }
 
     if (!g_output_stream_close(G_OUTPUT_STREAM(m), NULL, &err)) {
         error_report("%s: Failed to close stream: %s", __func__, err->message);
+        clear_dbus_connection(self);
         return -1;
     }
 
@@ -363,6 +410,7 @@ static int dbus_vmstate_pre_save(void *opaque)
     self->data =
         g_memory_output_stream_steal_data(G_MEMORY_OUTPUT_STREAM(m));
 
+    clear_dbus_connection(self);
     return 0;
 }
 
@@ -382,7 +430,6 @@ static void
 dbus_vmstate_complete(UserCreatable *uc, Error **errp)
 {
     DBusVMState *self = DBUS_VMSTATE(uc);
-    g_autoptr(GError) err = NULL;
 
     if (!object_resolve_path_type("", TYPE_DBUS_VMSTATE, NULL)) {
         error_setg(errp, "There is already an instance of %s",
@@ -395,15 +442,6 @@ dbus_vmstate_complete(UserCreatable *uc, Error **errp)
         return;
     }
 
-    self->bus = g_dbus_connection_new_for_address_sync(self->dbus_addr,
-                    G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT |
-                    G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
-                    NULL, NULL, &err);
-    if (err) {
-        error_setg(errp, "failed to connect to DBus: '%s'", err->message);
-        return;
-    }
-
     if (vmstate_register(VMSTATE_IF(self), VMSTATE_INSTANCE_ID_ANY,
                          &dbus_vmstate, self) < 0) {
         error_setg(errp, "Failed to register vmstate");
-- 
1.8.3.1