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 <mkletzan@redhat.com>

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;
+}

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 */

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;

src/lxc/lxc_cgroup.c

@@ -485,10 +485,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
                                 int *nicindexes)
 {
     virCgroupPtr cgroup = NULL;
-    char *machineName = virSystemdMakeMachineName("lxc",
+    char *machineName = virLXCDomainGetMachineName(def, 0);
-                                                  def->id,
-                                                  def->name,
-                                                  true);
 
     if (!machineName)
         goto cleanup;

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;
+}

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__ */

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->machineName,
-                                  vm->pid, -1, &priv->cgroup) < 0)
+                                  &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,
+        priv->machineName = virLXCDomainGetMachineName(vm->def, vm->pid);
-                                      vm->pid, -1, &priv->cgroup) < 0)
+        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;
 

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);
 }
 

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;
+}

src/qemu/qemu_domain.h

@@ -934,4 +934,7 @@ qemuDomainUpdateCPU(virDomainObjPtr vm,
                     virCPUDefPtr cpu,
                     virCPUDefPtr *origCPU);
 
+char *
+qemuDomainGetMachineName(virDomainObjPtr vm);
+
 #endif /* __QEMU_DOMAIN_H__ */

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);

src/util/vircgroup.c

@@ -252,17 +252,14 @@ static bool
 virCgroupValidateMachineGroup(virCgroupPtr group,
                               const char *name,
                               const char *drivername,
-                              int id,
+                              bool stripEmulatorSuffix,
-                              bool privileged,
+                              char *machinename)
-                              bool stripEmulatorSuffix)
 {
     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",

src/util/vircgroup.h

@@ -94,13 +94,13 @@ int virCgroupNewDetect(pid_t pid,
                        int controllers,
                        virCgroupPtr *group);
 
-int virCgroupNewDetectMachine(const char *name,
+int
-                              const char *drivername,
+virCgroupNewDetectMachine(const char *name,
-                              int id,
+                          const char *drivername,
-                              bool privileged,
+                          pid_t pid,
-                              pid_t pid,
+                          int controllers,
-                              int controllers,
+                          char *machinename,
-                              virCgroupPtr *group)
+                          virCgroupPtr *group)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 int virCgroupNewMachine(const char *name,

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

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,

tests/virsystemdtest.c

@@ -413,8 +413,8 @@ testMachineName(const void *opaque)
     int ret = -1;
     char *actual = NULL;
 
-    if (!(actual = virSystemdMakeMachineName("qemu", data->id,
+    if (!(actual = virDomainGenerateMachineName("qemu", data->id,
-                                             data->name, true)))
+                                                data->name, true)))
         goto cleanup;
 
     if (STRNEQ(actual, data->expected)) {