On 05/10/2018 05:57 PM, Stefan Berger wrote:
Implement functions for managing the storage of the external swtpm as
well
as starting and stopping it. Also implement functions to use swtpm_setup,
which simulates the manufacturing of a TPM, which includes creation of
certificates for the device.
Further, the external TPM needs storage on the host that we need to set
up before it can be run. We can clean up the host once the domain is
undefined.
This patch also implements a small layer for external device support that
calls into the TPM device layer if a domain has an attached TPM. This is
the layer we will wire up later on.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/qemu/Makefile.inc.am | 4 +
src/qemu/qemu_domain.c | 2 +
src/qemu/qemu_driver.c | 5 +
src/qemu/qemu_extdevice.c | 154 ++++++++++
src/qemu/qemu_extdevice.h | 53 ++++
src/qemu/qemu_migration.c | 3 +
src/qemu/qemu_process.c | 12 +
src/qemu/qemu_tpm.c | 753 ++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_tpm.h | 50 +++
9 files changed, 1036 insertions(+)
create mode 100644 src/qemu/qemu_extdevice.c
create mode 100644 src/qemu/qemu_extdevice.h
create mode 100644 src/qemu/qemu_tpm.c
create mode 100644 src/qemu/qemu_tpm.h
[...]
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 37876b8..9370de3 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,9 @@ qemuProcessLaunch(virConnectPtr conn,
goto cleanup;
logfile = qemuDomainLogContextGetWriteFD(logCtxt);
+ if (qemuExtDevicesStart(driver, vm->def, logCtxt) < 0)
+ goto cleanup;
+
VIR_DEBUG("Building emulator command line");
if (!(cmd = qemuBuildCommandLine(driver,
qemuDomainLogContextGetManager(logCtxt),
@@ -6194,6 +6202,8 @@ qemuProcessLaunch(virConnectPtr conn,
ret = 0;
cleanup:
+ if (ret)
if (ret < 0), right?
+ qemuExtDevicesStop(driver, vm->def);
qemuDomainSecretDestroy(vm);
virCommandFree(cmd);
virObjectUnref(logCtxt);
@@ -6614,6 +6624,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
qemuDomainCleanupRun(driver, vm);
+ qemuExtDevicesStop(driver, vm->def);
+
/* Stop autodestroy in case guest is restarted */
qemuProcessAutoDestroyRemove(driver, vm);
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
new file mode 100644
index 0000000..024d24d
--- /dev/null
+++ b/src/qemu/qemu_tpm.c
@@ -0,0 +1,753 @@
+/*
+ * qemu_tpm.c: QEMU TPM support
+ *
+ * Copyright (C) 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(a)linux.vnet.ibm.com>
+ */
+
[...]
+/*
+ * qemuTPMCreateEmulatorStoragePath
+ *
+ * @swtpmStorageDir: directory for swtpm persistent state
+ * @vmname: The name of the VM for which to create the storage
+ *
+ * Create the swtpm's storage path
+ */
+static char *
+qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir,
+ const char *vmname)
You changed at some point to use the uuidstr - probably should change
this too. Comment and argument
+{
+ char *path = NULL;
+
+ ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir,
vmname));
+
+ return path;
+}
+
+
[...]
+/*
+ * qemuTPMCreateEmulatorStorage
+ *
+ * @storagepath: directory for swtpm's pesistent state
persistent
+ * @created: a pointer to a bool that will be set to true if the
+ * storage was created because it did not exist yet
+ * @swtpm_user: The uid that needs to be able to access the directory
+ * @swtpm_group: The gid that needs to be able to access the directory
+ *
+ * Unless the storage path for the swtpm for the given VM
+ * already exists, create it and make it accessible for the given userid.
+ * Adapt ownership of the directory and all swtpm's state files there.
+ */
+static int
+qemuTPMCreateEmulatorStorage(const char *storagepath,
+ bool *created,
+ uid_t swtpm_user,
+ gid_t swtpm_group)
+{
+ int ret = -1;
+ char *swtpmStorageDir = qemuTPMGetTPMStorageDir(storagepath);
+
+ if (!swtpmStorageDir)
+ return -1;
+
+ if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0)
+ goto cleanup;
+
+ *created = false;
+
+ if (!virFileExists(storagepath))
+ *created = true;
+
+ if (virDirCreate(storagepath, 0700, swtpm_user, swtpm_group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not create directory %s as %u:%d"),
+ storagepath, swtpm_user, swtpm_group);
+ goto cleanup;
+ }
+
+ if (virFileChownFiles(storagepath, swtpm_user, swtpm_group) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(swtpmStorageDir);
+
+ return ret;
+}
+
+
[...]
+/*
+ * qemuTPMEmulatorPrepareHost:
+ *
+ * @tpm: tpm definition
+ * @logDir: directory where swtpm writes its logs into
+ * @vmname: name of the VM
+ * @swtpm_user: uid to run the swtpm with
+ * @swtpm_group: gid to run the swtpm with
+ * @swtpmStateDir: directory for swtpm's persistent state
+ * @qemu_user: uid that qemu will run with; we share the socket file with it
+ * @shortName: short and unique name of the domain
+ *
+ * Prepare the log directory for the swtpm and adjust ownership of it and the
+ * log file we will be using. Prepare the state directory where we will share
+ * the socket between tss and qemu users.
+ */
+static int
+qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
+ const char *logDir,
+ const char *vmname,
+ uid_t swtpm_user,
+ gid_t swtpm_group,
+ const char *swtpmStateDir,
+ uid_t qemu_user,
+ const char *shortName)
+{
+ int ret = -1;
+
+ if (qemuTPMEmulatorInit() < 0)
+ return -1;
+
+ /* create log dir ... */
+ if (virFileMakePathWithMode(logDir, 0730) < 0)
+ return -1;
+
+ /* ... and adjust ownership */
+ if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0)
+ goto cleanup;
Do we really need both? IOW: Why not just virDirCreate
+
+ /* create logfile name ... */
+ if (!tpm->data.emulator.logfile &&
+ virAsprintf(&tpm->data.emulator.logfile, "%s/%s-swtpm.log",
+ logDir, vmname) < 0)
+ goto cleanup;
+
+ /* ... and make sure it can be accessed by swtpm_user */
+ if (virFileExists(tpm->data.emulator.logfile) &&
+ chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) {
+ virReportSystemError(errno,
+ _("Could not chown on swtpm logfile %s"),
+ tpm->data.emulator.logfile);
+ goto cleanup;
+ }
+
+ /*
+ create our swtpm state dir ...
+ - QEMU user needs to be able to access the socket there
+ - swtpm group needs to be able to create files there
+ - in privileged mode 0570 would be enough, for non-privileged mode
+ we need 0770
+ */
+ if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0)
+ goto cleanup;
+
+ /* create the socket filename */
+ if (!tpm->data.emulator.source.data.nix.path &&
+ !(tpm->data.emulator.source.data.nix.path =
+ qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName)))
+ goto cleanup;
+ tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+
+ ret = 0;
+
+ cleanup:
+
+ return ret;
+}
[...]
+int
+qemuExtTPMInitPaths(virQEMUDriverPtr driver,
+ virDomainDefPtr def)
When I see qemuExt - I generally expect the code to be in
qemu_extdevice.c... Similar for anything qemuExtTPM. I understand why
the Ext is there (e.g. external), but not sure it's necessary.
FWIW: Although I didn't win my argument when last discussed, my feeling
is static functions should be something like "tpmEmulatorInitPaths";
whereas, external functions should be qemuTPMEmulatorInitPaths.
John
+{
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ int ret = 0;
+
+ switch (def->tpm->type) {
+ case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+ ret = qemuTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir,
+ def->uuid);
+ break;
+ case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+ case VIR_DOMAIN_TPM_TYPE_LAST:
+ break;
+ }
+
+ virObjectUnref(cfg);
+
+ return ret;
+}
+
+
[...]