Re: [PATCH v2 09/23] qemu: add dbus-vmstate

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/8/19 4:55 PM, marcandre.lureau@xxxxxxxxxx wrote:
From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>

Add dbusVMStates to keep a list of dbus-vmstate objects needed for
migration. They are populated on the command line during start or
qemuDBusVMStateAdd/Remove() will hotplug them as needed.

Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
---
  src/qemu/Makefile.inc.am |  2 +
  src/qemu/qemu_alias.c    | 17 ++++++++
  src/qemu/qemu_alias.h    |  3 ++
  src/qemu/qemu_command.c  | 83 +++++++++++++++++++++++++++++++++++
  src/qemu/qemu_command.h  |  3 ++
  src/qemu/qemu_dbus.c     | 94 ++++++++++++++++++++++++++++++++++++++++
  src/qemu/qemu_dbus.h     | 42 ++++++++++++++++++
  src/qemu/qemu_domain.c   | 14 ++++++
  src/qemu/qemu_domain.h   |  2 +
  src/qemu/qemu_hotplug.c  | 75 ++++++++++++++++++++++++++++++++
  src/qemu/qemu_hotplug.h  | 11 +++++
  11 files changed, 346 insertions(+)
  create mode 100644 src/qemu/qemu_dbus.c
  create mode 100644 src/qemu/qemu_dbus.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 254ba07dc0..94dd0e56ff 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -13,6 +13,8 @@ QEMU_DRIVER_SOURCES = \
  	qemu/qemu_capabilities.h \
  	qemu/qemu_command.c \
  	qemu/qemu_command.h \
+	qemu/qemu_dbus.c \
+	qemu/qemu_dbus.h \
  	qemu/qemu_domain.c \
  	qemu/qemu_domain.h \
  	qemu/qemu_domain_address.c \
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 585cc972ba..fc5246bc7f 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -843,3 +843,20 @@ qemuDomainGetUnmanagedPRAlias(const char *parentalias)
return ret;
  }
+
+char *
+qemuAliasDBusVMStateFromId(const char *id)
+{
+    char *ret;
+    int i;

This needs to be size_t rather than int.

+
+    if (virAsprintf(&ret, "dbus-vms-%s", id) < 0)
+        return NULL;
+
+    for (i = 0; ret[i]; i++) {
+        if (ret[i] == ':')
+            ret[i] = '_';
+    }
+
+    return ret;
+}
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index aaac09a1d1..ae2fce16bc 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -95,3 +95,6 @@ char *qemuAliasChardevFromDevAlias(const char *devAlias)
  const char *qemuDomainGetManagedPRAlias(void);
char *qemuDomainGetUnmanagedPRAlias(const char *parentalias);
+
+char *qemuAliasDBusVMStateFromId(const char *id)
+    ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 71a36ff63a..4357aa2fe1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -27,6 +27,7 @@
  #include "qemu_interface.h"
  #include "qemu_alias.h"
  #include "qemu_security.h"
+#include "qemu_dbus.h"
  #include "qemu_block.h"
  #include "cpu/cpu.h"
  #include "dirname.h"
@@ -10386,6 +10387,85 @@ qemuBuildManagedPRCommandLine(virCommandPtr cmd,
  }
+static virJSONValuePtr
+qemuBuildDBusVMStateInfoPropsInternal(const char *alias,
+                                      const char *addr)
+{
+    virJSONValuePtr ret = NULL;
+
+    if (qemuMonitorCreateObjectProps(&ret,
+                                     "dbus-vmstate", alias,
+                                     "s:addr", addr, NULL) < 0)
+        return NULL;
+
+    return ret;
+}
+
+
+virJSONValuePtr
+qemuBuildDBusVMStateInfoProps(const char *id,
+                              const char *addr)
+{
+    VIR_AUTOFREE(char *) alias = qemuAliasDBusVMStateFromId(id);
+
+    if (!alias)
+        return NULL;
+
+    return qemuBuildDBusVMStateInfoPropsInternal(alias, addr);
+}
+
+
+typedef struct qemuBuildDBusVMStateCommandLineData {
+    virCommandPtr cmd;
+} qemuBuildDBusVMStateCommandLineData;
+
+
+static int
+qemuBuildDBusVMStateCommandLineEach(void *payload,
+                                    const void *id,
+                                    void *user_data)
+{
+    qemuBuildDBusVMStateCommandLineData *data = user_data;
+    qemuDBusVMStatePtr vms = payload;
+    VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+    VIR_AUTOPTR(virJSONValue) props = NULL;
+
+    if (!(props = qemuBuildDBusVMStateInfoProps(id, vms->addr)))
+        return -1;
+
+    if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0)
+        return -1;
+
+    virCommandAddArg(data->cmd, "-object");
+    virCommandAddArgBuffer(data->cmd, &buf);
+
+    return 0;
+}
+
+static int
+qemuBuildDBusVMStateCommandLine(virCommandPtr cmd,
+                                qemuDomainObjPrivatePtr priv)
+{
+    qemuBuildDBusVMStateCommandLineData data = {
+        .cmd = cmd,
+    };
+
+    if (virHashSize(priv->dbusVMStates) == 0)
+        return 0;
+
+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("dbus-vmstate object is not supported by this QEMU binary"));
+        return 0;
+    }
+
+    if (virHashForEach(priv->dbusVMStates, qemuBuildDBusVMStateCommandLineEach, &data) < 0)
+        return -1;
+
+    return 0;
+}
+
+
  /**
   * qemuBuildCommandLineValidate:
   *
@@ -10630,6 +10710,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
      if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0)
          goto error;
+ if (qemuBuildDBusVMStateCommandLine(cmd, priv) < 0)
+        goto error;
+
      if (qemuBuildManagedPRCommandLine(cmd, def, priv) < 0)
          goto error;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 7e2dc5a60a..3a957c52fc 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -62,6 +62,9 @@ virJSONValuePtr qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivatePtr priv)
  int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
                               virJSONValuePtr *propsret);
+virJSONValuePtr qemuBuildDBusVMStateInfoProps(const char *id,
+                                              const char *addr);
+
  /* Generate the object properties for a tls-creds-x509 */
  int qemuBuildTLSx509BackendProps(const char *tlspath,
                                   bool isListen,
diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
new file mode 100644
index 0000000000..76cd3bd346
--- /dev/null
+++ b/src/qemu/qemu_dbus.c
@@ -0,0 +1,94 @@
+/*
+ * qemu_dbus.c: QEMU DBus-related helpers
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#include "qemu_extdevice.h"
+#include "qemu_dbus.h"
+#include "qemu_hotplug.h"
+#include "qemu_security.h"
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virpidfile.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT("qemu.dbus");
+
+
+qemuDBusVMStatePtr
+qemuDBusVMStateNew(const char *id, const char *addr)
+{
+    VIR_AUTOPTR(qemuDBusVMState) self;

This needs to be initialized to NULL to avoid freeing some random memory.

+
+    if (VIR_ALLOC(self) < 0)
+        return NULL;
+
+    if (VIR_STRDUP(self->id, id) < 0)
+        return NULL;
+
+    if (VIR_STRDUP(self->addr, addr) < 0)
+        return NULL;
+
+    VIR_RETURN_PTR(self);
+}
+
+
+void
+qemuDBusVMStateFree(qemuDBusVMStatePtr self)
+{

Well, we want our free functions to accept NULL. Not only because it simplifies the code in some cases it also ensures we don't SIGSEGV. For instance, if qemuDBusVMStateNew() fails rigt at the VIR_ALLOC(self) line, this free function is called with @self == NULL.

+    VIR_FREE(self->id);
+    VIR_FREE(self->addr);
+    VIR_FREE(self);
+}
+
+
+int
+qemuDBusVMStateAdd(virQEMUDriverPtr driver, virDomainObjPtr vm,
+                   const char *id, const char *addr, bool hot)
+{
+    qemuDBusVMStatePtr d = qemuDBusVMStateNew(id, addr);
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+
+    if (virHashAddEntry(QEMU_DOMAIN_PRIVATE(vm)->dbusVMStates, id, d) < 0) {

No need for this macro, it returns @priv in fact.

+        qemuDBusVMStateFree(d);
+        return -1;
+    }
+
+    if (hot && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE) &&
+        qemuDomainAttachDBusVMState(driver, vm, id, addr, QEMU_ASYNC_JOB_NONE) < 0)
+        return -1;
+
+    return 0;
+}
+
+
+void
+qemuDBusVMStateRemove(virQEMUDriverPtr driver, virDomainObjPtr vm,
+                      const char *id, bool hot)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+
+    if (virHashRemoveEntry(QEMU_DOMAIN_PRIVATE(vm)->dbusVMStates, id) < 0 ||
+        (hot && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE) &&
+         qemuDomainDetachDBusVMState(driver, vm, id, QEMU_ASYNC_JOB_NONE) < 0))
+        VIR_ERROR("Failed to remove vmstate id");

This needs to be in _("Failed ...") so that the message can be translated. Also, we might want to log the domain name at least to help sysadmins/users understand which domain the error relates to.

+}
diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h
new file mode 100644
index 0000000000..68ef6d1abf
--- /dev/null
+++ b/src/qemu/qemu_dbus.h
@@ -0,0 +1,42 @@
+/*
+ * qemu_dbus.c: QEMU DBus-related helpers

qemu_dbus.h

+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "qemu_conf.h"
+#include "qemu_domain.h"
+
+typedef struct _qemuDBusVMState qemuDBusVMState;
+typedef qemuDBusVMState *qemuDBusVMStatePtr;
+struct _qemuDBusVMState {
+    char *id;
+    char *addr;
+};
+
+
+qemuDBusVMStatePtr qemuDBusVMStateNew(const char *id, const char *addr);
+
+void qemuDBusVMStateFree(qemuDBusVMStatePtr self);
+
+int qemuDBusVMStateAdd(virQEMUDriverPtr driver, virDomainObjPtr vm,
+                       const char *id, const char *addr, bool hot);
+
+void qemuDBusVMStateRemove(virQEMUDriverPtr driver, virDomainObjPtr vm,
+                           const char *id, bool hot);
+
+VIR_DEFINE_AUTOPTR_FUNC(qemuDBusVMState, qemuDBusVMStateFree);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0555caa6ab..806dbfd1f8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -26,6 +26,7 @@
  #include "qemu_block.h"
  #include "qemu_cgroup.h"
  #include "qemu_command.h"
+#include "qemu_dbus.h"
  #include "qemu_process.h"
  #include "qemu_capabilities.h"
  #include "qemu_migration.h"
@@ -1961,6 +1962,14 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
  }
+static void
+dbusVMStateHashFree(void *opaque,
+                    const void *name ATTRIBUTE_UNUSED)
+{
+    qemuDBusVMStateFree(opaque);
+}
+
+
  static void *
  qemuDomainObjPrivateAlloc(void *opaque)
  {
@@ -1981,6 +1990,9 @@ qemuDomainObjPrivateAlloc(void *opaque)
      if (!(priv->blockjobs = virHashCreate(5, virObjectFreeHashData)))
          goto error;
+ if (!(priv->dbusVMStates = virHashCreate(5, dbusVMStateHashFree)))
+        goto error;
+
      priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
      priv->driver = opaque;
@@ -2052,6 +2064,7 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
      qemuDomainObjResetAsyncJob(priv);
virHashRemoveAll(priv->blockjobs);
+    virHashRemoveAll(priv->dbusVMStates);
  }
@@ -2084,6 +2097,7 @@ qemuDomainObjPrivateFree(void *data)
      qemuDomainMasterKeyFree(priv);
virHashFree(priv->blockjobs);
+    virHashFree(priv->dbusVMStates);
VIR_FREE(priv);
  }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index b76d3cace9..851fb98f42 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -392,6 +392,8 @@ struct _qemuDomainObjPrivate {
/* running block jobs */
      virHashTablePtr blockjobs;
+
+    virHashTablePtr dbusVMStates;
  };
#define QEMU_DOMAIN_PRIVATE(vm) \
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d8be63b71c..028921fb47 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -417,6 +417,81 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver,
  }
+/**
+ * qemuDomainAttachDBusVMState:
+ * @driver: QEMU driver object
+ * @vm: domain object
+ * @id
+ * @addr
+ * @asyncJob: asynchronous job identifier
+ *
+ * Add dbus-vmstate object.
+ *
+ * Returns: 0 on success, -1 on error.
+ */
+int
+qemuDomainAttachDBusVMState(virQEMUDriverPtr driver,
+                            virDomainObjPtr vm,
+                            const char *id,
+                            const char *addr,
+                            qemuDomainAsyncJob asyncJob)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    VIR_AUTOPTR(virJSONValue) props = NULL;
+    int ret;
+
+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("dbus-vmstate object is not supported by this QEMU binary"));
+        return -1;
+    }
+
+    if (!(props = qemuBuildDBusVMStateInfoProps(id, addr)))
+        return -1;
+
+    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+        return -1;
+
+    ret = qemuMonitorAddObject(priv->mon, &props, NULL);
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        return -1;
+
+    return ret;
+}
+
+
+/**
+ * qemuDomainDetachDBusVMState:
+ * @driver: QEMU driver object
+ * @vm: domain object
+ * @asyncJob: asynchronous job identifier
+ *
+ * Remove dbus-vmstate object from @vm.
+ *
+ * Returns: 0 on success, -1 on error.
+ */
+int
+qemuDomainDetachDBusVMState(virQEMUDriverPtr driver,
+                            virDomainObjPtr vm,
+                            const char *id,
+                            qemuDomainAsyncJob asyncJob)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    int ret;
+
+    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+        return -1;
+
+    ret = qemuMonitorDelObject(priv->mon, qemuAliasDBusVMStateFromId(id));

The retval of qemuAliasDBusVMStateFromId() is leaked here.

+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        return -1;
+
+    return ret;
+}
+

With all those small problems fixed:

Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux