2:01 p.m.
Ján Tomko (4):
qemu: tpm: remove unused pidfile variable
qemu: tpm: use g_autofree
qemu: tpm: use g_autoptr where applicable
qemu: tpm: remove unused labels
src/qemu/qemu_tpm.c | 130 +++++++++++++++-----------------------------
1 file changed, 44 insertions(+), 86 deletions(-)
--
2.19.2
2:01 p.m.
New subject: [libvirt PATCH 1/4] qemu: tpm: remove unused pidfile variable
qemuExtTPMSetupCgroup declares pidfile but never uses it.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_tpm.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 6741373583..385f4c74f7 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -913,7 +913,6 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
virCgroupPtr cgroup)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- char *pidfile = NULL;
char *shortName = NULL;
int ret = -1, rc;
pid_t pid;
@@ -940,7 +939,6 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
ret = 0;
cleanup:
- VIR_FREE(pidfile);
VIR_FREE(shortName);
virObjectUnref(cfg);
--
2.19.2
2:01 p.m.
New subject: [libvirt PATCH 2/4] qemu: tpm: use g_autofree
Mark eligible declarations as g_autofree and remove
the corresponding VIR_FREE calls.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_tpm.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 385f4c74f7..cfe7eb1846 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -126,7 +126,7 @@ qemuTPMCreateEmulatorStorage(const char *storagepath,
gid_t swtpm_group)
{
int ret = -1;
- char *swtpmStorageDir = g_path_get_dirname(storagepath);
+ g_autofree char *swtpmStorageDir = g_path_get_dirname(storagepath);
if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0)
goto cleanup;
@@ -150,8 +150,6 @@ qemuTPMCreateEmulatorStorage(const char *storagepath,
ret = 0;
cleanup:
- VIR_FREE(swtpmStorageDir);
-
return ret;
}
@@ -220,14 +218,12 @@ qemuTPMEmulatorCreatePidFilename(const char *swtpmStateDir,
const char *shortName)
{
char *pidfile = NULL;
- char *devicename = NULL;
+ g_autofree char *devicename = NULL;
devicename = g_strdup_printf("%s-swtpm", shortName);
pidfile = virPidFileBuildPath(swtpmStateDir, devicename);
- VIR_FREE(devicename);
-
return pidfile;
}
@@ -433,7 +429,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
int exitstatus;
int ret = -1;
char uuid[VIR_UUID_STRING_BUFLEN];
- char *vmid = NULL;
+ g_autofree char *vmid = NULL;
g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
VIR_AUTOCLOSE pwdfile_fd = -1;
@@ -516,7 +512,6 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
ret = 0;
cleanup:
- VIR_FREE(vmid);
virCommandFree(cmd);
return ret;
@@ -554,7 +549,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
{
virCommandPtr cmd = NULL;
bool created = false;
- char *pidfile;
+ g_autofree char *pidfile = NULL;
g_autofree char *swtpm = virTPMGetSwtpm();
VIR_AUTOCLOSE pwdfile_fd = -1;
VIR_AUTOCLOSE migpwdfile_fd = -1;
@@ -615,7 +610,6 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
virCommandAddArg(cmd, "--pid");
virCommandAddArgFormat(cmd, "file=%s", pidfile);
- VIR_FREE(pidfile);
if (tpm->data.emulator.hassecretuuid) {
if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) {
@@ -669,8 +663,8 @@ qemuTPMEmulatorStop(const char *swtpmStateDir,
const char *shortName)
{
virCommandPtr cmd;
- char *pathname;
- char *errbuf = NULL;
+ g_autofree char *pathname = NULL;
+ g_autofree char *errbuf = NULL;
g_autofree char *swtpm_ioctl = virTPMGetSwtpmIoctl();
if (!swtpm_ioctl)
@@ -701,8 +695,7 @@ qemuTPMEmulatorStop(const char *swtpmStateDir,
unlink(pathname);
cleanup:
- VIR_FREE(pathname);
- VIR_FREE(errbuf);
+ return;
}
@@ -735,7 +728,7 @@ qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
int ret = 0;
- char *shortName = NULL;
+ g_autofree char *shortName = NULL;
switch (def->tpm->type) {
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
@@ -755,7 +748,6 @@ qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
}
cleanup:
- VIR_FREE(shortName);
virObjectUnref(cfg);
return ret;
@@ -885,7 +877,7 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- char *shortName = NULL;
+ g_autofree char *shortName = NULL;
switch (vm->def->tpm->type) {
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
@@ -902,7 +894,6 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
}
cleanup:
- VIR_FREE(shortName);
virObjectUnref(cfg);
}
@@ -913,7 +904,7 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
virCgroupPtr cgroup)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- char *shortName = NULL;
+ g_autofree char *shortName = NULL;
int ret = -1, rc;
pid_t pid;
@@ -939,7 +930,6 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
ret = 0;
cleanup:
- VIR_FREE(shortName);
virObjectUnref(cfg);
return ret;
--
2.19.2
2:01 p.m.
New subject: [libvirt PATCH 3/4] qemu: tpm: use g_autoptr where applicable
This requires stealing one cmd pointer before returning it.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_tpm.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index cfe7eb1846..f9cea2ffde 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -425,7 +425,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
const unsigned char *secretuuid,
bool incomingMigration)
{
- virCommandPtr cmd = NULL;
+ g_autoptr(virCommand) cmd = NULL;
int exitstatus;
int ret = -1;
char uuid[VIR_UUID_STRING_BUFLEN];
@@ -512,8 +512,6 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
ret = 0;
cleanup:
- virCommandFree(cmd);
-
return ret;
}
@@ -547,7 +545,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
const char *shortName,
bool incomingMigration)
{
- virCommandPtr cmd = NULL;
+ g_autoptr(virCommand) cmd = NULL;
bool created = false;
g_autofree char *pidfile = NULL;
g_autofree char *swtpm = virTPMGetSwtpm();
@@ -639,14 +637,12 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
migpwdfile_fd = -1;
}
- return cmd;
+ return g_steal_pointer(&cmd);
error:
if (created)
qemuTPMDeleteEmulatorStorage(tpm);
- virCommandFree(cmd);
-
return NULL;
}
@@ -703,7 +699,7 @@ int
qemuExtTPMInitPaths(virQEMUDriverPtr driver,
virDomainDefPtr def)
{
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
int ret = 0;
switch (def->tpm->type) {
@@ -716,8 +712,6 @@ qemuExtTPMInitPaths(virQEMUDriverPtr driver,
break;
}
- virObjectUnref(cfg);
-
return ret;
}
@@ -726,7 +720,7 @@ int
qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
virDomainDefPtr def)
{
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
int ret = 0;
g_autofree char *shortName = NULL;
@@ -748,8 +742,6 @@ qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
}
cleanup:
- virObjectUnref(cfg);
-
return ret;
}
@@ -876,7 +868,7 @@ void
qemuExtTPMStop(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *shortName = NULL;
switch (vm->def->tpm->type) {
@@ -894,7 +886,7 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
}
cleanup:
- virObjectUnref(cfg);
+ return;
}
@@ -903,7 +895,7 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
virDomainDefPtr def,
virCgroupPtr cgroup)
{
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *shortName = NULL;
int ret = -1, rc;
pid_t pid;
@@ -930,7 +922,5 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
ret = 0;
cleanup:
- virObjectUnref(cfg);
-
return ret;
}
--
2.19.2
2:01 p.m.
New subject: [libvirt PATCH 4/4] qemu: tpm: remove unused labels
Remove all the labels that are now redundant.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_tpm.c | 74 +++++++++++++++++----------------------------
1 file changed, 27 insertions(+), 47 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index f9cea2ffde..f3562d7dfe 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -125,11 +125,10 @@ qemuTPMCreateEmulatorStorage(const char *storagepath,
uid_t swtpm_user,
gid_t swtpm_group)
{
- int ret = -1;
g_autofree char *swtpmStorageDir = g_path_get_dirname(storagepath);
if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0)
- goto cleanup;
+ return -1;
*created = false;
@@ -141,16 +140,13 @@ qemuTPMCreateEmulatorStorage(const char *storagepath,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not create directory %s as %u:%d"),
storagepath, swtpm_user, swtpm_group);
- goto cleanup;
+ return -1;
}
if (virFileChownFiles(storagepath, swtpm_user, swtpm_group) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- return ret;
+ return 0;
}
@@ -427,7 +423,6 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
{
g_autoptr(virCommand) cmd = NULL;
int exitstatus;
- int ret = -1;
char uuid[VIR_UUID_STRING_BUFLEN];
g_autofree char *vmid = NULL;
g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
@@ -444,7 +439,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
cmd = virCommandNew(swtpm_setup);
if (!cmd)
- goto cleanup;
+ return -1;
virUUIDFormat(vmuuid, uuid);
vmid = g_strdup_printf("%s:%s", vmname, uuid);
@@ -469,10 +464,10 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
_("%s does not support passing a passphrase using a file "
"descriptor"), swtpm_setup);
- goto cleanup;
+ return -1;
}
if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0)
- goto cleanup;
+ return -1;
virCommandAddArg(cmd, "--pwdfile-fd");
virCommandAddArgFormat(cmd, "%d", pwdfile_fd);
@@ -506,13 +501,10 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
_("Could not run '%s'. exitstatus: %d; "
"Check error log '%s' for details."),
swtpm_setup, exitstatus, logfile);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- return ret;
+ return 0;
}
@@ -673,11 +665,11 @@ qemuTPMEmulatorStop(const char *swtpmStateDir,
return;
if (!virFileExists(pathname))
- goto cleanup;
+ return;
cmd = virCommandNew(swtpm_ioctl);
if (!cmd)
- goto cleanup;
+ return;
virCommandAddArgList(cmd, "--unix", pathname, "-s", NULL);
@@ -689,9 +681,6 @@ qemuTPMEmulatorStop(const char *swtpmStateDir,
/* clean up the socket */
unlink(pathname);
-
- cleanup:
- return;
}
@@ -700,19 +689,17 @@ qemuExtTPMInitPaths(virQEMUDriverPtr driver,
virDomainDefPtr def)
{
g_autoptr(virQEMUDriverConfig) 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;
+ return qemuTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir,
+ def->uuid);
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
case VIR_DOMAIN_TPM_TYPE_LAST:
break;
}
- return ret;
+ return 0;
}
@@ -721,28 +708,25 @@ qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
virDomainDefPtr def)
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
- int ret = 0;
g_autofree char *shortName = NULL;
switch (def->tpm->type) {
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
shortName = virDomainDefGetShortName(def);
if (!shortName)
- goto cleanup;
+ return -1;
- ret = qemuTPMEmulatorPrepareHost(def->tpm, cfg->swtpmLogDir,
- def->name, cfg->swtpm_user,
- cfg->swtpm_group,
- cfg->swtpmStateDir, cfg->user,
- shortName);
- break;
+ return qemuTPMEmulatorPrepareHost(def->tpm, cfg->swtpmLogDir,
+ def->name, cfg->swtpm_user,
+ cfg->swtpm_group,
+ cfg->swtpmStateDir, cfg->user,
+ shortName);
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
case VIR_DOMAIN_TPM_TYPE_LAST:
break;
}
- cleanup:
- return ret;
+ return 0;
}
@@ -875,7 +859,7 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
shortName = virDomainDefGetShortName(vm->def);
if (!shortName)
- goto cleanup;
+ return;
qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName);
qemuSecurityCleanupTPMEmulator(driver, vm);
@@ -885,7 +869,6 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
break;
}
- cleanup:
return;
}
@@ -897,30 +880,27 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *shortName = NULL;
- int ret = -1, rc;
+ int rc;
pid_t pid;
switch (def->tpm->type) {
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
shortName = virDomainDefGetShortName(def);
if (!shortName)
- goto cleanup;
+ return -1;
rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid);
if (rc < 0 || (rc == 0 && pid == (pid_t)-1)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not get process id of swtpm"));
- goto cleanup;
+ return -1;
}
if (virCgroupAddProcess(cgroup, pid) < 0)
- goto cleanup;
+ return -1;
break;
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
case VIR_DOMAIN_TPM_TYPE_LAST:
break;
}
- ret = 0;
-
- cleanup:
- return ret;
+ return 0;
}
--
2.19.2
2:12 a.m.
On Sun, Feb 02, 2020 at 21:01:43 +0100, Ján Tomko wrote:
Ján Tomko (4):
qemu: tpm: remove unused pidfile variable
qemu: tpm: use g_autofree
qemu: tpm: use g_autoptr where applicable
I don't understand why we split g_autofree and g_autoptr. Specifically
as g_autoptr patches usually combine changes to distinct types.
qemu: tpm: remove unused labels
Nevertheless:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
12:41 p.m.
On Mon, Feb 03, 2020 at 09:12:57AM +0100, Peter Krempa wrote:
On Sun, Feb 02, 2020 at 21:01:43 +0100, Ján Tomko wrote:
> Ján Tomko (4):
> qemu: tpm: remove unused pidfile variable
> qemu: tpm: use g_autofree
> qemu: tpm: use g_autoptr where applicable
I don't understand why we split g_autofree and g_autoptr. Specifically
as g_autoptr patches usually combine changes to distinct types.
Usually the g_autoptr changes are fewer than the mechanical g_autofree
changes, which made it worth it for large files.
That might not be the case here, but it's easier to squash
patches together than to separate them :)
Jano
> qemu: tpm: remove unused labels
Nevertheless:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
1784
days inactive
1785
days old
6 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa