Implement a layer for starting and stopping of external devices.
The tpm-emulator is the only user of this layer.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 src/qemu/Makefile.inc.am  |   2 +
 src/qemu/qemu_extdevice.c | 300 ++++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_extdevice.h |  43 +++++++
 src/qemu/qemu_process.c   |  13 ++
 4 files changed, 358 insertions(+)
 create mode 100644 src/qemu/qemu_extdevice.c
 create mode 100644 src/qemu/qemu_extdevice.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 63e7c87..d16e880 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -19,6 +19,8 @@ QEMU_DRIVER_SOURCES = \
 	qemu/qemu_domain_address.h \
 	qemu/qemu_cgroup.c \
 	qemu/qemu_cgroup.h \
+	qemu/qemu_extdevice.c \
+	qemu/qemu_extdevice.h \
 	qemu/qemu_hostdev.c \
 	qemu/qemu_hostdev.h \
 	qemu/qemu_hotplug.c \
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
new file mode 100644
index 0000000..f3f337d
--- /dev/null
+++ b/src/qemu/qemu_extdevice.c
@@ -0,0 +1,300 @@
+/*
+ * qemu_extdevice.c: QEMU external devices support
+ *
+ * Copyright (C) 2014, 2018 IBM Corporation
+ *
+ * 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/>.
+ *
+ * Author: Stefan Berger <stefanb@linux.vnet.ibm.com>
+ */
+
+#include <config.h>
+
+#include "qemu_extdevice.h"
+#include "qemu_domain.h"
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virtpm.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT("qemu.qemu_extdevice")
+
+static int
+qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt,
+                        virCommandPtr cmd,
+                        const char *info)
+{
+    int ret = -1;
+    char *timestamp = NULL;
+    char *logline = NULL;
+    int logFD;
+
+    logFD = qemuDomainLogContextGetWriteFD(logCtxt);
+
+    if ((timestamp = virTimeStringNow()) == NULL)
+        goto cleanup;
+
+    if (virAsprintf(&logline, "%s: Starting external device: %s\n",
+                    timestamp, info) < 0)
+        goto cleanup;
+
+    if (safewrite(logFD, logline, strlen(logline)) < 0)
+        goto cleanup;
+
+    virCommandWriteArgLog(cmd, logFD);
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(timestamp);
+    VIR_FREE(logline);
+
+    return ret;
+}
+
+
+static int
+qemuExtTPMInitPaths(virQEMUDriverPtr driver,
+                    virDomainDefPtr def)
+{
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    int ret = 0;
+
+    switch (def->tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        ret = virTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir,
+                                      def->uuid);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        break;
+    }
+
+    virObjectUnref(cfg);
+
+    return ret;
+}
+
+
+static int
+qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
+                      virDomainDefPtr def)
+{
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    int ret = 0;
+    char *shortName = NULL;
+
+    switch (def->tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        shortName = virDomainDefGetShortName(def);
+        if (!shortName)
+            goto cleanup;
+
+        ret = virTPMEmulatorPrepareHost(def->tpm, cfg->swtpmLogDir,
+                                        def->name, cfg->swtpm_user,
+                                        cfg->swtpm_group,
+                                        cfg->swtpmStateDir, cfg->user,
+                                        shortName);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        break;
+    }
+
+cleanup:
+    VIR_FREE(shortName);
+    virObjectUnref(cfg);
+
+    return ret;
+}
+
+
+/*
+ * qemuExtTPMStartEmulator:
+ *
+ * @driver: QEMU driver
+ * @def: domain definition
+ * @logCtxt: log context
+ *
+ * Start the external TPM Emulator:
+ * - have the command line built
+ * - start the external TPM Emulator and sync with it before QEMU start
+ */
+static int
+qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
+                        virDomainDefPtr def,
+                        qemuDomainLogContextPtr logCtxt)
+{
+    int ret = -1;
+    virCommandPtr cmd = NULL;
+    int exitstatus;
+    char *errbuf = NULL;
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    virDomainTPMDefPtr tpm = def->tpm;
+    char *shortName = virDomainDefGetShortName(def);
+
+    if (!shortName)
+        return -1;
+
+    /* stop any left-over TPM emulator for this VM */
+    virTPMEmulatorStop(cfg->swtpmStateDir, shortName);
+
+    if (!(cmd = virTPMEmulatorBuildCommand(tpm, def->name, def->uuid,
+                                           driver->privileged,
+                                           cfg->swtpm_user,
+                                           cfg->swtpm_group)))
+        goto cleanup;
+
+    if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
+        goto cleanup;
+
+    virCommandSetErrorBuffer(cmd, &errbuf);
+
+    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
+        VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
+                  "stderr: %s\n", exitstatus, errbuf);
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not start 'swtpm'. exitstatus: %d, "
+                       "error: %s"), exitstatus, errbuf);
+        goto error;
+    }
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(shortName);
+    VIR_FREE(errbuf);
+    virCommandFree(cmd);
+
+    virObjectUnref(cfg);
+
+    return ret;
+
+ error:
+    VIR_FREE(tpm->data.emulator.source.data.nix.path);
+
+    goto cleanup;
+}
+
+static int
+qemuExtTPMStart(virQEMUDriverPtr driver,
+                virDomainDefPtr def,
+                qemuDomainLogContextPtr logCtxt)
+{
+    int ret = 0;
+    virDomainTPMDefPtr tpm = def->tpm;
+
+    switch (tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        ret = qemuExtTPMStartEmulator(driver, def, logCtxt);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        break;
+    }
+
+    return ret;
+}
+
+static void
+qemuExtTPMStop(virQEMUDriverPtr driver, virDomainDefPtr def)
+{
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    char *shortName = NULL;
+
+    switch (def->tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        shortName = virDomainDefGetShortName(def);
+        if (!shortName)
+            goto cleanup;
+
+        virTPMEmulatorStop(cfg->swtpmStateDir, shortName);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        break;
+    }
+
+cleanup:
+    VIR_FREE(shortName);
+    virObjectUnref(cfg);
+}
+
+/*
+ * qemuExtDevicesInitPaths:
+ *
+ * @driver: QEMU driver
+ * @def: domain definition
+ *
+ * Initialize paths of external devices so that it is known where state is
+ * stored and we can remove directories and files in case of domain XML
+ * changes.
+ */
+int
+qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
+                        virDomainDefPtr def)
+{
+    int ret = 0;
+
+    if (def->tpm)
+        ret = qemuExtTPMInitPaths(driver, def);
+
+    return ret;
+}
+
+/*
+ * qemuExtDevicesPrepareHost:
+ *
+ * @driver: QEMU driver
+ * @def: domain definition
+ *
+ * Prepare host storage paths for external devices.
+ */
+int
+qemuExtDevicesPrepareHost(virQEMUDriverPtr driver,
+                          virDomainDefPtr def)
+{
+    int ret = 0;
+
+    if (def->tpm)
+        ret = qemuExtTPMPrepareHost(driver, def);
+
+    return ret;
+}
+
+int
+qemuExtDevicesStart(virQEMUDriverPtr driver,
+                    virDomainDefPtr def,
+                    qemuDomainLogContextPtr logCtxt)
+{
+    int ret = 0;
+
+    if (def->tpm)
+        ret = qemuExtTPMStart(driver, def, logCtxt);
+
+    return ret;
+}
+
+void
+qemuExtDevicesStop(virQEMUDriverPtr driver,
+                   virDomainDefPtr def)
+{
+     if (def->tpm)
+         qemuExtTPMStop(driver, def);
+}
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
new file mode 100644
index 0000000..fd6b630
--- /dev/null
+++ b/src/qemu/qemu_extdevice.h
@@ -0,0 +1,43 @@
+/*
+ * qemu_extdevice.h: QEMU external devices support
+ *
+ * Copyright (C) 2014, 2018 IBM Corporation
+ *
+ * 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/>.
+ *
+ * Author: Stefan Berger <stefanb@linux.vnet.ibm.com>
+ */
+#ifndef __QEMU_EXTDEVICE_H__
+# define __QEMU_EXTDEVICE_H__
+
+# include "qemu_conf.h"
+# include "qemu_domain.h"
+
+int qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
+                            virDomainDefPtr def)
+    ATTRIBUTE_RETURN_CHECK;
+
+int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver,
+                              virDomainDefPtr def)
+    ATTRIBUTE_RETURN_CHECK;
+
+int qemuExtDevicesStart(virQEMUDriverPtr driver,
+                        virDomainDefPtr def,
+                        qemuDomainLogContextPtr logCtxt)
+    ATTRIBUTE_RETURN_CHECK;
+
+void qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainDefPtr def);
+
+#endif /* __QEMU_EXTDEVICE_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9233d26..2b07530 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -48,6 +48,7 @@
 #include "qemu_migration_params.h"
 #include "qemu_interface.h"
 #include "qemu_security.h"
+#include "qemu_extdevice.h"
 
 #include "cpu/cpu.h"
 #include "datatypes.h"
@@ -5872,6 +5873,10 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
     if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0)
         goto cleanup;
 
+    VIR_DEBUG("Preparing external devices");
+    if (qemuExtDevicesPrepareHost(driver, vm->def) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     virObjectUnref(cfg);
@@ -5955,6 +5960,10 @@ qemuProcessLaunch(virConnectPtr conn,
         goto cleanup;
     logfile = qemuDomainLogContextGetWriteFD(logCtxt);
 
+    if (qemuExtDevicesInitPaths(driver, vm->def) < 0 ||
+        qemuExtDevicesStart(driver, vm->def, logCtxt) < 0)
+        goto cleanup;
+
     VIR_DEBUG("Building emulator command line");
     if (!(cmd = qemuBuildCommandLine(driver,
                                      qemuDomainLogContextGetManager(logCtxt),
@@ -6194,6 +6203,8 @@ qemuProcessLaunch(virConnectPtr conn,
     ret = 0;
 
  cleanup:
+    if (ret)
+        qemuExtDevicesStop(driver, vm->def);
     qemuDomainSecretDestroy(vm);
     virCommandFree(cmd);
     virObjectUnref(logCtxt);
@@ -6614,6 +6625,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
     qemuDomainCleanupRun(driver, vm);
 
+    qemuExtDevicesStop(driver, vm->def);
+
     /* Stop autodestroy in case guest is restarted */
     qemuProcessAutoDestroyRemove(driver, vm);
 
-- 
2.5.5

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

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> Implement a layer for starting and stopping of external devices.
> The tpm-emulator is the only user of this layer.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  src/qemu/Makefile.inc.am  |   2 +
>  src/qemu/qemu_extdevice.c | 300 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_extdevice.h |  43 +++++++
>  src/qemu/qemu_process.c   |  13 ++
>  4 files changed, 358 insertions(+)
>  create mode 100644 src/qemu/qemu_extdevice.c
>  create mode 100644 src/qemu/qemu_extdevice.h
> 
> diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
> index 63e7c87..d16e880 100644
> --- a/src/qemu/Makefile.inc.am
> +++ b/src/qemu/Makefile.inc.am
> @@ -19,6 +19,8 @@ QEMU_DRIVER_SOURCES = \
>  	qemu/qemu_domain_address.h \
>  	qemu/qemu_cgroup.c \
>  	qemu/qemu_cgroup.h \
> +	qemu/qemu_extdevice.c \
> +	qemu/qemu_extdevice.h \
>  	qemu/qemu_hostdev.c \
>  	qemu/qemu_hostdev.h \
>  	qemu/qemu_hotplug.c \
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> new file mode 100644
> index 0000000..f3f337d
> --- /dev/null
> +++ b/src/qemu/qemu_extdevice.c
> @@ -0,0 +1,300 @@
> +/*
> + * qemu_extdevice.c: QEMU external devices support
> + *
> + * Copyright (C) 2014, 2018 IBM Corporation
> + *
> + * 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/>.
> + *
> + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com>
> + */
> +
> +#include <config.h>
> +
> +#include "qemu_extdevice.h"
> +#include "qemu_domain.h"
> +
> +#include "viralloc.h"
> +#include "virlog.h"
> +#include "virstring.h"
> +#include "virtime.h"
> +#include "virtpm.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_QEMU
> +
> +VIR_LOG_INIT("qemu.qemu_extdevice")
> +
> +static int
> +qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt,
> +                        virCommandPtr cmd,
> +                        const char *info)
> +{
> +    int ret = -1;
> +    char *timestamp = NULL;
> +    char *logline = NULL;
> +    int logFD;
> +
> +    logFD = qemuDomainLogContextGetWriteFD(logCtxt);
> +
> +    if ((timestamp = virTimeStringNow()) == NULL)
> +        goto cleanup;
> +
> +    if (virAsprintf(&logline, "%s: Starting external device: %s\n",
> +                    timestamp, info) < 0)
> +        goto cleanup;
> +
> +    if (safewrite(logFD, logline, strlen(logline)) < 0)
> +        goto cleanup;
> +
> +    virCommandWriteArgLog(cmd, logFD);
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(timestamp);
> +    VIR_FREE(logline);
> +
> +    return ret;
> +}
> +
> +
> +static int
> +qemuExtTPMInitPaths(virQEMUDriverPtr driver,
> +                    virDomainDefPtr def)
> +{
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    int ret = 0;
> +
> +    switch (def->tpm->type) {
> +    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> +        ret = virTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir,
> +                                      def->uuid);
> +        break;
> +    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +    case VIR_DOMAIN_TPM_TYPE_LAST:
> +        break;
> +    }
> +
> +    virObjectUnref(cfg);
> +
> +    return ret;
> +}
> +
> +
> +static int
> +qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
> +                      virDomainDefPtr def)
> +{
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    int ret = 0;
> +    char *shortName = NULL;
> +
> +    switch (def->tpm->type) {
> +    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> +        shortName = virDomainDefGetShortName(def);
> +        if (!shortName)
> +            goto cleanup;
> +
> +        ret = virTPMEmulatorPrepareHost(def->tpm, cfg->swtpmLogDir,
> +                                        def->name, cfg->swtpm_user,
> +                                        cfg->swtpm_group,
> +                                        cfg->swtpmStateDir, cfg->user,
> +                                        shortName);
> +        break;
> +    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +    case VIR_DOMAIN_TPM_TYPE_LAST:
> +        break;
> +    }
> +
> +cleanup:
> +    VIR_FREE(shortName);
> +    virObjectUnref(cfg);
> +
> +    return ret;
> +}
> +
> +
> +/*
> + * qemuExtTPMStartEmulator:
> + *
> + * @driver: QEMU driver
> + * @def: domain definition
> + * @logCtxt: log context
> + *
> + * Start the external TPM Emulator:
> + * - have the command line built
> + * - start the external TPM Emulator and sync with it before QEMU start
> + */
> +static int
> +qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
> +                        virDomainDefPtr def,
> +                        qemuDomainLogContextPtr logCtxt)
> +{
> +    int ret = -1;
> +    virCommandPtr cmd = NULL;
> +    int exitstatus;
> +    char *errbuf = NULL;
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    virDomainTPMDefPtr tpm = def->tpm;
> +    char *shortName = virDomainDefGetShortName(def);
> +
> +    if (!shortName)
> +        return -1;

Leaking @cfg (repeats often)

> +
> +    /* stop any left-over TPM emulator for this VM */
> +    virTPMEmulatorStop(cfg->swtpmStateDir, shortName);
> +
> +    if (!(cmd = virTPMEmulatorBuildCommand(tpm, def->name, def->uuid,
> +                                           driver->privileged,
> +                                           cfg->swtpm_user,
> +                                           cfg->swtpm_group)))
> +        goto cleanup;
> +
> +    if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
> +        goto cleanup;
> +
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +
> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> +        VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
> +                  "stderr: %s\n", exitstatus, errbuf);
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not start 'swtpm'. exitstatus: %d, "
> +                       "error: %s"), exitstatus, errbuf);
> +        goto error;
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(shortName);
> +    VIR_FREE(errbuf);
> +    virCommandFree(cmd);
> +
> +    virObjectUnref(cfg);
> +
> +    return ret;
> +
> + error:
> +    VIR_FREE(tpm->data.emulator.source.data.nix.path);

Still not clear why VIR_FREE here since virDomainTPMDefFree does it.

> +
> +    goto cleanup;
> +}
> +

Right about here you went back to single blank line between functions.

> +static int
> +qemuExtTPMStart(virQEMUDriverPtr driver,
> +                virDomainDefPtr def,
> +                qemuDomainLogContextPtr logCtxt)
> +{
> +    int ret = 0;
> +    virDomainTPMDefPtr tpm = def->tpm;
> +
> +    switch (tpm->type) {
> +    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> +        ret = qemuExtTPMStartEmulator(driver, def, logCtxt);
> +        break;
> +    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +    case VIR_DOMAIN_TPM_TYPE_LAST:
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static void
> +qemuExtTPMStop(virQEMUDriverPtr driver, virDomainDefPtr def)
> +{
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    char *shortName = NULL;
> +
> +    switch (def->tpm->type) {
> +    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> +        shortName = virDomainDefGetShortName(def);
> +        if (!shortName)
> +            goto cleanup;
> +
> +        virTPMEmulatorStop(cfg->swtpmStateDir, shortName);
> +        break;
> +    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +    case VIR_DOMAIN_TPM_TYPE_LAST:
> +        break;
> +    }
> +
> +cleanup:
> +    VIR_FREE(shortName);
> +    virObjectUnref(cfg);
> +}
> +
> +/*
> + * qemuExtDevicesInitPaths:
> + *
> + * @driver: QEMU driver
> + * @def: domain definition
> + *
> + * Initialize paths of external devices so that it is known where state is
> + * stored and we can remove directories and files in case of domain XML
> + * changes.
> + */
> +int
> +qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
> +                        virDomainDefPtr def)
> +{
> +    int ret = 0;
> +
> +    if (def->tpm)
> +        ret = qemuExtTPMInitPaths(driver, def);
> +
> +    return ret;
> +}
> +
> +/*
> + * qemuExtDevicesPrepareHost:
> + *
> + * @driver: QEMU driver
> + * @def: domain definition
> + *
> + * Prepare host storage paths for external devices.
> + */
> +int
> +qemuExtDevicesPrepareHost(virQEMUDriverPtr driver,
> +                          virDomainDefPtr def)
> +{
> +    int ret = 0;
> +
> +    if (def->tpm)
> +        ret = qemuExtTPMPrepareHost(driver, def);
> +
> +    return ret;
> +}
> +
> +int
> +qemuExtDevicesStart(virQEMUDriverPtr driver,
> +                    virDomainDefPtr def,
> +                    qemuDomainLogContextPtr logCtxt)
> +{
> +    int ret = 0;
> +
> +    if (def->tpm)
> +        ret = qemuExtTPMStart(driver, def, logCtxt);
> +
> +    return ret;
> +}
> +
> +void
> +qemuExtDevicesStop(virQEMUDriverPtr driver,
> +                   virDomainDefPtr def)
> +{
> +     if (def->tpm)
> +         qemuExtTPMStop(driver, def);
> +}
> diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
> new file mode 100644
> index 0000000..fd6b630
> --- /dev/null
> +++ b/src/qemu/qemu_extdevice.h
> @@ -0,0 +1,43 @@
> +/*
> + * qemu_extdevice.h: QEMU external devices support
> + *
> + * Copyright (C) 2014, 2018 IBM Corporation
> + *
> + * 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/>.
> + *
> + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com>
> + */
> +#ifndef __QEMU_EXTDEVICE_H__
> +# define __QEMU_EXTDEVICE_H__
> +
> +# include "qemu_conf.h"
> +# include "qemu_domain.h"
> +
> +int qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
> +                            virDomainDefPtr def)
> +    ATTRIBUTE_RETURN_CHECK;
> +
> +int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver,
> +                              virDomainDefPtr def)
> +    ATTRIBUTE_RETURN_CHECK;
> +
> +int qemuExtDevicesStart(virQEMUDriverPtr driver,
> +                        virDomainDefPtr def,
> +                        qemuDomainLogContextPtr logCtxt)
> +    ATTRIBUTE_RETURN_CHECK;
> +
> +void qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainDefPtr def);
> +
> +#endif /* __QEMU_EXTDEVICE_H__ */
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 9233d26..2b07530 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -48,6 +48,7 @@
>  #include "qemu_migration_params.h"
>  #include "qemu_interface.h"
>  #include "qemu_security.h"
> +#include "qemu_extdevice.h"
>  
>  #include "cpu/cpu.h"
>  #include "datatypes.h"
> @@ -5872,6 +5873,10 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
>      if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0)
>          goto cleanup;
>  
> +    VIR_DEBUG("Preparing external devices");
> +    if (qemuExtDevicesPrepareHost(driver, vm->def) < 0)
> +        goto cleanup;
> +
>      ret = 0;
>   cleanup:
>      virObjectUnref(cfg);
> @@ -5955,6 +5960,10 @@ qemuProcessLaunch(virConnectPtr conn,
>          goto cleanup;
>      logfile = qemuDomainLogContextGetWriteFD(logCtxt);
>  
> +    if (qemuExtDevicesInitPaths(driver, vm->def) < 0 ||
> +        qemuExtDevicesStart(driver, vm->def, logCtxt) < 0)
> +        goto cleanup;
> +
>      VIR_DEBUG("Building emulator command line");
>      if (!(cmd = qemuBuildCommandLine(driver,
>                                       qemuDomainLogContextGetManager(logCtxt),
> @@ -6194,6 +6203,8 @@ qemuProcessLaunch(virConnectPtr conn,
>      ret = 0;
>  
>   cleanup:
> +    if (ret)
> +        qemuExtDevicesStop(driver, vm->def);
>      qemuDomainSecretDestroy(vm);
>      virCommandFree(cmd);
>      virObjectUnref(logCtxt);
> @@ -6614,6 +6625,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>  
>      qemuDomainCleanupRun(driver, vm);
>  
> +    qemuExtDevicesStop(driver, vm->def);
> +
>      /* Stop autodestroy in case guest is restarted */
>      qemuProcessAutoDestroyRemove(driver, vm);
>  
> 

Anything need to be done during qemuProcessReconnect?

John

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

On 05/08/2018 04:50 PM, John Ferlan wrote:
>
> On 05/04/2018 04:21 PM, Stefan Berger wrote:
> +
>> + cleanup:
>> +    VIR_FREE(shortName);
>> +    VIR_FREE(errbuf);
>> +    virCommandFree(cmd);
>> +
>> +    virObjectUnref(cfg);
>> +
>> +    return ret;
>> +
>> + error:
>> +    VIR_FREE(tpm->data.emulator.source.data.nix.path);
> Still not clear why VIR_FREE here since virDomainTPMDefFree does it.

Removed.

>
>> +
>> +    goto cleanup;
>> +}
>> +
> Right about here you went back to single blank line between functions.

Until you now told me that this is the new rule, I was sometimes looking 
around in the file what others had used there before. ok, in this file I 
was inconsistent.@@ -6194,6 +6203,8 @@ qemuProcessLaunch(virConnectPtr 
conn,
>>       ret = 0;
>>   
>>    cleanup:
>> +    if (ret)
>> +        qemuExtDevicesStop(driver, vm->def);
>>       qemuDomainSecretDestroy(vm);
>>       virCommandFree(cmd);
>>       virObjectUnref(logCtxt);
>> @@ -6614,6 +6625,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>>   
>>       qemuDomainCleanupRun(driver, vm);
>>   
>> +    qemuExtDevicesStop(driver, vm->def);
>> +
>>       /* Stop autodestroy in case guest is restarted */
>>       qemuProcessAutoDestroyRemove(driver, vm);
>>   
>>
> Anything need to be done during qemuProcessReconnect?

In all the scenarios I have tried so far I haven't come across having to 
add something to this function. If this is triggered by restart of 
libvirtd, I haven't seen a failure. I could have missed something, though.

   Stefan
>
> John
>

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