From eaf2c9f89107b9f60cf8db2c919f78b987ff7177 Mon Sep 17 00:00:00 2001 From: Martin Kletzander Date: Fri, 21 Jul 2017 15:51:03 +0200 Subject: [PATCH] Move machineName generation from virsystemd into domain_conf 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 --- 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 b07b553ca0..34c8f45ed1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27030,3 +27030,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 e1586fdfba..239b218130 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 d984176787..fa2cd08fe3 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 1c42ab5cde..3369801870 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 7c1386e40c..d23969a585 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 5c4f31e546..b248cdfd6f 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 724297d128..05f1dec4c4 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 36762d4f67..7355527c1c 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 8549990dc9..e3b5c94ded 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 71567af034..4c9050aff0 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 757f5d95e0..87ab85fdb4 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 2912fc9be5..f274aee81a 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 2de1bf2de4..d833927678 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 5d9746f995..829b92d54d 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 93b0aae7e1..9aedb40b86 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 15642759e9..4f4f29bfac 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)) { -- GitLab