[libvirt] [PATCH 4/4] Move machineName generation from virsystemd into domain_conf
Martin Kletzander
mkletzan at redhat.com
Mon Jul 24 14:09:34 UTC 2017
It is more related to a domain as we might use it even when there is
no systemd and it does not use any dbus/systemd functions. In order
not to use code from conf/ in util/ pass machineName in cgroups code
as a parameter. That also fixes a leak of machineName in the lxc
driver and cleans up and de-duplicates some code.
Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
---
src/conf/domain_conf.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 5 ++++
src/libvirt_private.syms | 2 +-
src/lxc/lxc_cgroup.c | 5 +---
src/lxc/lxc_domain.c | 19 +++++++++++++++
src/lxc/lxc_domain.h | 3 +++
src/lxc/lxc_process.c | 25 +++++++++----------
src/qemu/qemu_cgroup.c | 24 ++++---------------
src/qemu/qemu_domain.c | 21 ++++++++++++++++
src/qemu/qemu_domain.h | 3 +++
src/qemu/qemu_process.c | 6 +++++
src/util/vircgroup.c | 15 ++++--------
src/util/vircgroup.h | 14 +++++------
src/util/virsystemd.c | 62 ------------------------------------------------
src/util/virsystemd.h | 5 ----
tests/virsystemdtest.c | 4 ++--
16 files changed, 153 insertions(+), 122 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2231b195b9c4..98e2e7292912 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -27007,3 +27007,65 @@ virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk,
return 0;
}
+
+#define HOSTNAME_CHARS \
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-"
+
+static void
+virDomainMachineNameAppendValid(virBufferPtr buf,
+ const char *name)
+{
+ bool skip_dot = false;
+
+ for (; *name; name++) {
+ if (virBufferError(buf))
+ break;
+ if (strlen(virBufferCurrentContent(buf)) >= 64)
+ break;
+
+ if (*name == '.') {
+ if (!skip_dot)
+ virBufferAddChar(buf, *name);
+ skip_dot = true;
+ continue;
+ }
+
+ skip_dot = false;
+
+ if (!strchr(HOSTNAME_CHARS, *name))
+ continue;
+
+ virBufferAddChar(buf, *name);
+ }
+}
+
+#undef HOSTNAME_CHARS
+
+char *
+virDomainGenerateMachineName(const char *drivername,
+ int id,
+ const char *name,
+ bool privileged)
+{
+ char *machinename = NULL;
+ char *username = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (privileged) {
+ virBufferAsprintf(&buf, "%s-", drivername);
+ } else {
+ if (!(username = virGetUserName(geteuid())))
+ goto cleanup;
+
+ virBufferAsprintf(&buf, "%s-%s-", username, drivername);
+ }
+
+ virBufferAsprintf(&buf, "%d-", id);
+ virDomainMachineNameAppendValid(&buf, name);
+
+ machinename = virBufferContentAndReset(&buf);
+ cleanup:
+ VIR_FREE(username);
+
+ return machinename;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index dfc51208a029..e422e01e71ed 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3341,4 +3341,9 @@ virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def,
int virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk,
virDomainBlockIoTuneInfo *info);
+char *
+virDomainGenerateMachineName(const char *drivername,
+ int id,
+ const char *name,
+ bool privileged);
#endif /* __DOMAIN_CONF_H */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 87753758f9cc..c5c048d0189f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -340,6 +340,7 @@ virDomainFSTypeFromString;
virDomainFSTypeToString;
virDomainFSWrpolicyTypeFromString;
virDomainFSWrpolicyTypeToString;
+virDomainGenerateMachineName;
virDomainGetFilesystemForTarget;
virDomainGraphicsAuthConnectedTypeFromString;
virDomainGraphicsAuthConnectedTypeToString;
@@ -2711,7 +2712,6 @@ virSystemdCanSuspend;
virSystemdCreateMachine;
virSystemdGetMachineNameByPID;
virSystemdHasMachinedResetCachedValue;
-virSystemdMakeMachineName;
virSystemdMakeScopeName;
virSystemdMakeSliceName;
virSystemdNotifyStartup;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 1c42ab5cde9d..336980187035 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -485,10 +485,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
int *nicindexes)
{
virCgroupPtr cgroup = NULL;
- char *machineName = virSystemdMakeMachineName("lxc",
- def->id,
- def->name,
- true);
+ char *machineName = virLXCDomainGetMachineName(def, 0);
if (!machineName)
goto cleanup;
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index 7c1386e40c82..d23969a585f4 100644
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -31,6 +31,7 @@
#include "virutil.h"
#include "virfile.h"
#include "virtime.h"
+#include "virsystemd.h"
#define VIR_FROM_THIS VIR_FROM_LXC
#define LXC_NAMESPACE_HREF "http://libvirt.org/schemas/domain/lxc/1.0"
@@ -397,3 +398,21 @@ virDomainDefParserConfig virLXCDriverDomainDefParserConfig = {
.domainPostParseCallback = virLXCDomainDefPostParse,
.devicesPostParseCallback = virLXCDomainDeviceDefPostParse,
};
+
+
+char *
+virLXCDomainGetMachineName(virDomainDefPtr def, pid_t pid)
+{
+ char *ret = NULL;
+
+ if (pid) {
+ ret = virSystemdGetMachineNameByPID(pid);
+ if (!ret)
+ virResetLastError();
+ }
+
+ if (!ret)
+ ret = virDomainGenerateMachineName("lxc", def->id, def->name, true);
+
+ return ret;
+}
diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
index 5c4f31e54625..b248cdfd6f34 100644
--- a/src/lxc/lxc_domain.h
+++ b/src/lxc/lxc_domain.h
@@ -107,4 +107,7 @@ virLXCDomainObjEndJob(virLXCDriverPtr driver,
virDomainObjPtr obj);
+char *
+virLXCDomainGetMachineName(virDomainDefPtr def, pid_t pid);
+
#endif /* __LXC_DOMAIN_H__ */
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 724297d128d0..05f1dec4c421 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -234,6 +234,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
* the bug we are working around here.
*/
virCgroupTerminateMachine(priv->machineName);
+ VIR_FREE(priv->machineName);
/* The "release" hook cleans up additional resources */
if (virHookPresent(VIR_HOOK_DRIVER_LXC)) {
@@ -1494,13 +1495,17 @@ int virLXCProcessStart(virConnectPtr conn,
goto cleanup;
}
+ priv->machineName = virLXCDomainGetMachineName(vm->def, vm->pid);
+ if (!priv->machineName)
+ goto cleanup;
+
/* We know the cgroup must exist by this synchronization
* point so lets detect that first, since it gives us a
* more reliable way to kill everything off if something
* goes wrong from here onwards ... */
if (virCgroupNewDetectMachine(vm->def->name, "lxc",
- vm->def->id, true,
- vm->pid, -1, &priv->cgroup) < 0)
+ vm->pid, -1, priv->machineName,
+ &priv->cgroup) < 0)
goto cleanup;
if (!priv->cgroup) {
@@ -1510,11 +1515,6 @@ int virLXCProcessStart(virConnectPtr conn,
goto cleanup;
}
- /* Get the machine name so we can properly delete it through
- * systemd later */
- if (!(priv->machineName = virSystemdGetMachineNameByPID(vm->pid)))
- virResetLastError();
-
/* And we can get the first monitor connection now too */
if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm))) {
/* Intentionally overwrite the real monitor error message,
@@ -1666,8 +1666,12 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm)))
goto error;
- if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->def->id, true,
- vm->pid, -1, &priv->cgroup) < 0)
+ priv->machineName = virLXCDomainGetMachineName(vm->def, vm->pid);
+ if (!priv->machineName)
+ goto cleanup;
+
+ if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid, -1,
+ priv->machineName, &priv->cgroup) < 0)
goto error;
if (!priv->cgroup) {
@@ -1677,9 +1681,6 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
goto error;
}
- if (!(priv->machineName = virSystemdGetMachineNameByPID(vm->pid)))
- virResetLastError();
-
if (virLXCUpdateActiveUSBHostdevs(driver, vm->def) < 0)
goto error;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 36762d4f676e..7355527c1cc5 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -852,17 +852,6 @@ qemuInitCgroup(virQEMUDriverPtr driver,
goto cleanup;
}
- /*
- * We need to do this because of systemd-machined, because
- * CreateMachine requires the name to be a valid hostname.
- */
- priv->machineName = virSystemdMakeMachineName("qemu",
- vm->def->id,
- vm->def->name,
- virQEMUDriverIsPrivileged(driver));
- if (!priv->machineName)
- goto cleanup;
-
if (virCgroupNewMachine(priv->machineName,
"qemu",
vm->def->uuid,
@@ -978,21 +967,20 @@ qemuConnectCgroup(virQEMUDriverPtr driver,
if (!virCgroupAvailable())
goto done;
+ priv->machineName = qemuDomainGetMachineName(vm);
+ if (!priv->machineName)
+ goto cleanup;
+
virCgroupFree(&priv->cgroup);
if (virCgroupNewDetectMachine(vm->def->name,
"qemu",
- vm->def->id,
- virQEMUDriverIsPrivileged(driver),
vm->pid,
cfg->cgroupControllers,
+ priv->machineName,
&priv->cgroup) < 0)
goto cleanup;
- priv->machineName = virSystemdGetMachineNameByPID(vm->pid);
- if (!priv->machineName)
- virResetLastError();
-
qemuRestoreCgroupState(vm);
done:
@@ -1164,8 +1152,6 @@ qemuRemoveCgroup(virDomainObjPtr vm)
VIR_DEBUG("Failed to terminate cgroup for %s", vm->def->name);
}
- VIR_FREE(priv->machineName);
-
return virCgroupRemove(priv->cgroup);
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0b7c45280321..b71ca168ce18 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -49,6 +49,7 @@
#include "viratomic.h"
#include "virprocess.h"
#include "vircrypto.h"
+#include "virsystemd.h"
#include "secret_util.h"
#include "logging/log_manager.h"
#include "locking/domain_lock.h"
@@ -9568,3 +9569,23 @@ qemuDomainUpdateCPU(virDomainObjPtr vm,
return 0;
}
+
+char *
+qemuDomainGetMachineName(virDomainObjPtr vm)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virQEMUDriverPtr driver = priv->driver;
+ char *ret = NULL;
+
+ if (vm->pid) {
+ ret = virSystemdGetMachineNameByPID(vm->pid);
+ if (!ret)
+ virResetLastError();
+ }
+
+ if (!ret)
+ ret = virDomainGenerateMachineName("qemu", vm->def->id, vm->def->name,
+ virQEMUDriverIsPrivileged(driver));
+
+ return ret;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 71567af034f5..4c9050aff000 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -934,4 +934,7 @@ qemuDomainUpdateCPU(virDomainObjPtr vm,
virCPUDefPtr cpu,
virCPUDefPtr *origCPU);
+char *
+qemuDomainGetMachineName(virDomainObjPtr vm);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 757f5d95e0b7..87ab85fdb476 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5242,6 +5242,10 @@ qemuProcessPrepareDomain(virConnectPtr conn,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
+ priv->machineName = qemuDomainGetMachineName(vm);
+ if (!priv->machineName)
+ goto cleanup;
+
if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) {
/* If you are using a SecurityDriver with dynamic labelling,
then generate a security label for isolation */
@@ -6307,6 +6311,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
}
}
+ VIR_FREE(priv->machineName);
+
vm->taint = 0;
vm->pid = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 2912fc9be539..f274aee81ab9 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -252,17 +252,14 @@ static bool
virCgroupValidateMachineGroup(virCgroupPtr group,
const char *name,
const char *drivername,
- int id,
- bool privileged,
- bool stripEmulatorSuffix)
+ bool stripEmulatorSuffix,
+ char *machinename)
{
size_t i;
bool valid = false;
char *partname = NULL;
char *scopename_old = NULL;
char *scopename_new = NULL;
- char *machinename = virSystemdMakeMachineName(drivername, id,
- name, privileged);
char *partmachinename = NULL;
if (virAsprintf(&partname, "%s.libvirt-%s",
@@ -1539,10 +1536,9 @@ virCgroupNewDetect(pid_t pid,
int
virCgroupNewDetectMachine(const char *name,
const char *drivername,
- int id,
- bool privileged,
pid_t pid,
int controllers,
+ char *machinename,
virCgroupPtr *group)
{
if (virCgroupNewDetect(pid, controllers, group) < 0) {
@@ -1552,7 +1548,7 @@ virCgroupNewDetectMachine(const char *name,
}
if (!virCgroupValidateMachineGroup(*group, name, drivername,
- id, privileged, true)) {
+ true, machinename)) {
VIR_DEBUG("Failed to validate machine name for '%s' driver '%s'",
name, drivername);
virCgroupFree(group);
@@ -4208,10 +4204,9 @@ virCgroupNewDetect(pid_t pid ATTRIBUTE_UNUSED,
int
virCgroupNewDetectMachine(const char *name ATTRIBUTE_UNUSED,
const char *drivername ATTRIBUTE_UNUSED,
- int id ATTRIBUTE_UNUSED,
- bool privileged ATTRIBUTE_UNUSED,
pid_t pid ATTRIBUTE_UNUSED,
int controllers ATTRIBUTE_UNUSED,
+ char *machinename ATTRIBUTE_UNUSED,
virCgroupPtr *group ATTRIBUTE_UNUSED)
{
virReportSystemError(ENXIO, "%s",
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 2de1bf2de4ce..d83392767864 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -94,13 +94,13 @@ int virCgroupNewDetect(pid_t pid,
int controllers,
virCgroupPtr *group);
-int virCgroupNewDetectMachine(const char *name,
- const char *drivername,
- int id,
- bool privileged,
- pid_t pid,
- int controllers,
- virCgroupPtr *group)
+int
+virCgroupNewDetectMachine(const char *name,
+ const char *drivername,
+ pid_t pid,
+ int controllers,
+ char *machinename,
+ virCgroupPtr *group)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int virCgroupNewMachine(const char *name,
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 5d9746f9955f..829b92d54d94 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -125,68 +125,6 @@ char *virSystemdMakeSliceName(const char *partition)
return virBufferContentAndReset(&buf);
}
-#define HOSTNAME_CHARS \
- "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-"
-
-static void
-virSystemdAppendValidMachineName(virBufferPtr buf,
- const char *name)
-{
- bool skip_dot = false;
-
- for (; *name; name++) {
- if (virBufferError(buf))
- break;
- if (strlen(virBufferCurrentContent(buf)) >= 64)
- break;
-
- if (*name == '.') {
- if (!skip_dot)
- virBufferAddChar(buf, *name);
- skip_dot = true;
- continue;
- }
-
- skip_dot = false;
-
- if (!strchr(HOSTNAME_CHARS, *name))
- continue;
-
- virBufferAddChar(buf, *name);
- }
-}
-
-#undef HOSTNAME_CHARS
-
-char *
-virSystemdMakeMachineName(const char *drivername,
- int id,
- const char *name,
- bool privileged)
-{
- char *machinename = NULL;
- char *username = NULL;
- virBuffer buf = VIR_BUFFER_INITIALIZER;
-
- if (privileged) {
- virBufferAsprintf(&buf, "%s-", drivername);
- } else {
- if (!(username = virGetUserName(geteuid())))
- goto cleanup;
-
- virBufferAsprintf(&buf, "%s-%s-", username, drivername);
- }
-
- virBufferAsprintf(&buf, "%d-", id);
- virSystemdAppendValidMachineName(&buf, name);
-
- machinename = virBufferContentAndReset(&buf);
- cleanup:
- VIR_FREE(username);
-
- return machinename;
-}
-
static int virSystemdHasMachinedCachedValue = -1;
/* Reset the cache from tests for testing the underlying dbus calls
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
index 93b0aae7e15f..9aedb40b86f1 100644
--- a/src/util/virsystemd.h
+++ b/src/util/virsystemd.h
@@ -29,11 +29,6 @@ char *virSystemdMakeScopeName(const char *name,
bool legacy_behaviour);
char *virSystemdMakeSliceName(const char *partition);
-char *virSystemdMakeMachineName(const char *drivername,
- int id,
- const char *name,
- bool privileged);
-
int virSystemdCreateMachine(const char *name,
const char *drivername,
const unsigned char *uuid,
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 15642759e94f..4f4f29bfacde 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -413,8 +413,8 @@ testMachineName(const void *opaque)
int ret = -1;
char *actual = NULL;
- if (!(actual = virSystemdMakeMachineName("qemu", data->id,
- data->name, true)))
+ if (!(actual = virDomainGenerateMachineName("qemu", data->id,
+ data->name, true)))
goto cleanup;
if (STRNEQ(actual, data->expected)) {
--
2.13.3
More information about the libvir-list
mailing list