d71922
From fdceac0494012ade9dddac2afe2965538f26206e Mon Sep 17 00:00:00 2001
d71922
Message-Id: <fdceac0494012ade9dddac2afe2965538f26206e@dist-git>
d71922
From: Martin Kletzander <mkletzan@redhat.com>
d71922
Date: Tue, 16 Feb 2016 11:55:09 +0100
d71922
Subject: [PATCH] systemd: Modernize machine naming
d71922
d71922
https://bugzilla.redhat.com/show_bug.cgi?id=1308494
d71922
d71922
So, systemd-machined has this philosophy that machine names are like
d71922
hostnames and hence should follow the same rules.  But we always allowed
d71922
international characters in domain names.  Thus we need to modify the
d71922
machine name we are passing to systemd.
d71922
d71922
In order to change some machine names that we will be passing to systemd,
d71922
we also need to call TerminateMachine at the end of a lifetime of a
d71922
domain.  Even for domains that were started with older libvirt.  That
d71922
can be achieved thanks to virSystemdGetMachineNameByPID().  And because
d71922
we can change machine names, we can get rid of the inconsistent and
d71922
pointless escaping of domain names when creating machine names.
d71922
d71922
So this patch modifies the naming in the following way.  It creates the
d71922
name as <drivername>-<id>-<name> where invalid hostname characters are
d71922
stripped out of the name and if the resulting name is longer, it
d71922
truncates it to 64 characters.  That way we can start domains we
d71922
couldn't start before.  Well, at least on systemd.
d71922
d71922
To make it work all together, the machineName (which is needed only with
d71922
systemd) is saved in domain's private data.  That way the generation is
d71922
moved to the driver and we don't need to pass various unnecessary
d71922
arguments to cgroup functions.
d71922
d71922
The only thing this complicates a bit is the scope generation when
d71922
validating a cgroup where we must check both old and new naming, so a
d71922
slight modification was needed there.
d71922
d71922
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1282846
d71922
d71922
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
d71922
(cherry picked from commit c3bd0019c0e3f080dbf0d4bd08245ffb2daa2765)
d71922
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
d71922
d71922
Conflicts:
d71922
	src/qemu/qemu_process.c - fe422b673b4b8 (context)
d71922
d71922
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
d71922
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
d71922
---
d71922
 src/lxc/lxc_cgroup.c    | 13 +++++++--
d71922
 src/lxc/lxc_domain.h    |  1 +
d71922
 src/lxc/lxc_process.c   | 20 +++++++++----
d71922
 src/qemu/qemu_cgroup.c  | 28 +++++++++++++-----
d71922
 src/qemu/qemu_cgroup.h  |  2 +-
d71922
 src/qemu/qemu_domain.h  |  1 +
d71922
 src/qemu/qemu_process.c |  4 +--
d71922
 src/util/vircgroup.c    | 59 ++++++++++++++++++++++++--------------
d71922
 src/util/vircgroup.h    | 12 ++++----
d71922
 src/util/virsystemd.c   | 75 +++++++++++++++++++++++++++++++++----------------
d71922
 src/util/virsystemd.h   | 13 ++++-----
d71922
 tests/virsystemdtest.c  | 67 +++++++++++++++++++++++++------------------
d71922
 12 files changed, 191 insertions(+), 104 deletions(-)
d71922
d71922
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
d71922
index ad254e4..3148946 100644
d71922
--- a/src/lxc/lxc_cgroup.c
d71922
+++ b/src/lxc/lxc_cgroup.c
d71922
@@ -29,6 +29,7 @@
d71922
 #include "viralloc.h"
d71922
 #include "vircgroup.h"
d71922
 #include "virstring.h"
d71922
+#include "virsystemd.h"
d71922
 
d71922
 #define VIR_FROM_THIS VIR_FROM_LXC
d71922
 
d71922
@@ -483,6 +484,13 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
d71922
                                 int *nicindexes)
d71922
 {
d71922
     virCgroupPtr cgroup = NULL;
d71922
+    char *machineName = virSystemdMakeMachineName("lxc",
d71922
+                                                  def->id,
d71922
+                                                  def->name,
d71922
+                                                  true);
d71922
+
d71922
+    if (!machineName)
d71922
+        goto cleanup;
d71922
 
d71922
     if (def->resource->partition[0] != '/') {
d71922
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
d71922
@@ -491,9 +499,8 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
d71922
         goto cleanup;
d71922
     }
d71922
 
d71922
-    if (virCgroupNewMachine(def->name,
d71922
+    if (virCgroupNewMachine(machineName,
d71922
                             "lxc",
d71922
-                            true,
d71922
                             def->uuid,
d71922
                             NULL,
d71922
                             initpid,
d71922
@@ -517,6 +524,8 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
d71922
     }
d71922
 
d71922
  cleanup:
d71922
+    VIR_FREE(machineName);
d71922
+
d71922
     return cgroup;
d71922
 }
d71922
 
d71922
diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
d71922
index 751aece..a9e319c 100644
d71922
--- a/src/lxc/lxc_domain.h
d71922
+++ b/src/lxc/lxc_domain.h
d71922
@@ -39,6 +39,7 @@ struct _virLXCDomainObjPrivate {
d71922
     pid_t initpid;
d71922
 
d71922
     virCgroupPtr cgroup;
d71922
+    char *machineName;
d71922
 };
d71922
 
d71922
 extern virDomainXMLPrivateDataCallbacks virLXCDriverPrivateDataCallbacks;
d71922
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
d71922
index 87ee484..2eaad7d 100644
d71922
--- a/src/lxc/lxc_process.c
d71922
+++ b/src/lxc/lxc_process.c
d71922
@@ -216,8 +216,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
d71922
      * properly. See https://bugs.freedesktop.org/show_bug.cgi?id=68370 for
d71922
      * the bug we are working around here.
d71922
      */
d71922
-    virSystemdTerminateMachine(vm->def->name, "lxc", true);
d71922
-
d71922
+    virCgroupTerminateMachine(priv->machineName);
d71922
 
d71922
     /* The "release" hook cleans up additional resources */
d71922
     if (virHookPresent(VIR_HOOK_DRIVER_LXC)) {
d71922
@@ -1318,8 +1317,9 @@ int virLXCProcessStart(virConnectPtr conn,
d71922
      * point so lets detect that first, since it gives us a
d71922
      * more reliable way to kill everything off if something
d71922
      * goes wrong from here onwards ... */
d71922
-    if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid,
d71922
-                                  -1, &priv->cgroup) < 0)
d71922
+    if (virCgroupNewDetectMachine(vm->def->name, "lxc",
d71922
+                                  vm->def->id, true,
d71922
+                                  vm->pid, -1, &priv->cgroup) < 0)
d71922
         goto cleanup;
d71922
 
d71922
     if (!priv->cgroup) {
d71922
@@ -1329,6 +1329,11 @@ int virLXCProcessStart(virConnectPtr conn,
d71922
         goto cleanup;
d71922
     }
d71922
 
d71922
+    /* Get the machine name so we can properly delete it through
d71922
+     * systemd later */
d71922
+    if (!(priv->machineName = virSystemdGetMachineNameByPID(vm->pid)))
d71922
+        virResetLastError();
d71922
+
d71922
     /* And we can get the first monitor connection now too */
d71922
     if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm))) {
d71922
         /* Intentionally overwrite the real monitor error message,
d71922
@@ -1501,8 +1506,8 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
d71922
         if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm)))
d71922
             goto error;
d71922
 
d71922
-        if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid,
d71922
-                                      -1, &priv->cgroup) < 0)
d71922
+        if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->def->id, true,
d71922
+                                      vm->pid, -1, &priv->cgroup) < 0)
d71922
             goto error;
d71922
 
d71922
         if (!priv->cgroup) {
d71922
@@ -1512,6 +1517,9 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
d71922
             goto error;
d71922
         }
d71922
 
d71922
+        if (!(priv->machineName = virSystemdGetMachineNameByPID(vm->pid)))
d71922
+            virResetLastError();
d71922
+
d71922
         if (virLXCUpdateActiveUSBHostdevs(driver, vm->def) < 0)
d71922
             goto error;
d71922
 
d71922
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
d71922
index 0da6c02..448183f 100644
d71922
--- a/src/qemu/qemu_cgroup.c
d71922
+++ b/src/qemu/qemu_cgroup.c
d71922
@@ -36,6 +36,7 @@
d71922
 #include "virfile.h"
d71922
 #include "virtypedparam.h"
d71922
 #include "virnuma.h"
d71922
+#include "virsystemd.h"
d71922
 
d71922
 #define VIR_FROM_THIS VIR_FROM_QEMU
d71922
 
d71922
@@ -753,9 +754,19 @@ qemuInitCgroup(virQEMUDriverPtr driver,
d71922
         goto cleanup;
d71922
     }
d71922
 
d71922
-    if (virCgroupNewMachine(vm->def->name,
d71922
+    /*
d71922
+     * We need to do this because of systemd-machined, because
d71922
+     * CreateMachine requires the name to be a valid hostname.
d71922
+     */
d71922
+    priv->machineName = virSystemdMakeMachineName("qemu",
d71922
+                                                  vm->def->id,
d71922
+                                                  vm->def->name,
d71922
+                                                  virQEMUDriverIsPrivileged(driver));
d71922
+    if (!priv->machineName)
d71922
+        goto cleanup;
d71922
+
d71922
+    if (virCgroupNewMachine(priv->machineName,
d71922
                             "qemu",
d71922
-                            true,
d71922
                             vm->def->uuid,
d71922
                             NULL,
d71922
                             vm->pid,
d71922
@@ -864,11 +875,17 @@ qemuConnectCgroup(virQEMUDriverPtr driver,
d71922
 
d71922
     if (virCgroupNewDetectMachine(vm->def->name,
d71922
                                   "qemu",
d71922
+                                  vm->def->id,
d71922
+                                  virQEMUDriverIsPrivileged(driver),
d71922
                                   vm->pid,
d71922
                                   cfg->cgroupControllers,
d71922
                                   &priv->cgroup) < 0)
d71922
         goto cleanup;
d71922
 
d71922
+    priv->machineName = virSystemdGetMachineNameByPID(vm->pid);
d71922
+    if (!priv->machineName)
d71922
+        virResetLastError();
d71922
+
d71922
     qemuRestoreCgroupState(vm);
d71922
 
d71922
  done:
d71922
@@ -1250,17 +1267,14 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
d71922
 }
d71922
 
d71922
 int
d71922
-qemuRemoveCgroup(virQEMUDriverPtr driver,
d71922
-                 virDomainObjPtr vm)
d71922
+qemuRemoveCgroup(virDomainObjPtr vm)
d71922
 {
d71922
     qemuDomainObjPrivatePtr priv = vm->privateData;
d71922
 
d71922
     if (priv->cgroup == NULL)
d71922
         return 0; /* Not supported, so claim success */
d71922
 
d71922
-    if (virCgroupTerminateMachine(vm->def->name,
d71922
-                                  "qemu",
d71922
-                                  virQEMUDriverIsPrivileged(driver)) < 0) {
d71922
+    if (virCgroupTerminateMachine(priv->machineName) < 0) {
d71922
         if (!virCgroupNewIgnoreError())
d71922
             VIR_DEBUG("Failed to terminate cgroup for %s", vm->def->name);
d71922
     }
d71922
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
d71922
index 711a6de..3fc313f 100644
d71922
--- a/src/qemu/qemu_cgroup.h
d71922
+++ b/src/qemu/qemu_cgroup.h
d71922
@@ -56,7 +56,7 @@ int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask);
d71922
 int qemuSetupCgroupForVcpu(virDomainObjPtr vm);
d71922
 int qemuSetupCgroupForIOThreads(virDomainObjPtr vm);
d71922
 int qemuSetupCgroupForEmulator(virDomainObjPtr vm);
d71922
-int qemuRemoveCgroup(virQEMUDriverPtr driver, virDomainObjPtr vm);
d71922
+int qemuRemoveCgroup(virDomainObjPtr vm);
d71922
 int qemuAddToCgroup(virDomainObjPtr vm);
d71922
 
d71922
 #endif /* __QEMU_CGROUP_H__ */
d71922
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
d71922
index 32f713b..8d4e099 100644
d71922
--- a/src/qemu/qemu_domain.h
d71922
+++ b/src/qemu/qemu_domain.h
d71922
@@ -202,6 +202,7 @@ struct _qemuDomainObjPrivate {
d71922
 
d71922
     bool signalIOError; /* true if the domain condition should be signalled on
d71922
                            I/O error */
d71922
+    char *machineName;
d71922
 };
d71922
 
d71922
 # define QEMU_DOMAIN_DISK_PRIVATE(disk)	\
d71922
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
d71922
index ed01bf4..1d06b6d 100644
d71922
--- a/src/qemu/qemu_process.c
d71922
+++ b/src/qemu/qemu_process.c
d71922
@@ -4540,7 +4540,7 @@ int qemuProcessStart(virConnectPtr conn,
d71922
     /* Ensure no historical cgroup for this VM is lying around bogus
d71922
      * settings */
d71922
     VIR_DEBUG("Ensuring no historical cgroup is lying around");
d71922
-    qemuRemoveCgroup(driver, vm);
d71922
+    qemuRemoveCgroup(vm);
d71922
 
d71922
     for (i = 0; i < vm->def->ngraphics; ++i) {
d71922
         virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
d71922
@@ -5390,7 +5390,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
d71922
     }
d71922
 
d71922
  retry:
d71922
-    if ((ret = qemuRemoveCgroup(driver, vm)) < 0) {
d71922
+    if ((ret = qemuRemoveCgroup(vm)) < 0) {
d71922
         if (ret == -EBUSY && (retries++ < 5)) {
d71922
             usleep(200*1000);
d71922
             goto retry;
d71922
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
d71922
index c94512a..b43edb2 100644
d71922
--- a/src/util/vircgroup.c
d71922
+++ b/src/util/vircgroup.c
d71922
@@ -243,12 +243,17 @@ static bool
d71922
 virCgroupValidateMachineGroup(virCgroupPtr group,
d71922
                               const char *name,
d71922
                               const char *drivername,
d71922
+                              int id,
d71922
+                              bool privileged,
d71922
                               bool stripEmulatorSuffix)
d71922
 {
d71922
     size_t i;
d71922
     bool valid = false;
d71922
-    char *partname;
d71922
-    char *scopename;
d71922
+    char *partname = NULL;
d71922
+    char *scopename_old = NULL;
d71922
+    char *scopename_new = NULL;
d71922
+    char *machinename = virSystemdMakeMachineName(drivername, id,
d71922
+                                                  name, privileged);
d71922
 
d71922
     if (virAsprintf(&partname, "%s.libvirt-%s",
d71922
                     name, drivername) < 0)
d71922
@@ -257,10 +262,21 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
d71922
     if (virCgroupPartitionEscape(&partname) < 0)
d71922
         goto cleanup;
d71922
 
d71922
-    if (!(scopename = virSystemdMakeScopeName(name, drivername)))
d71922
+    if (!(scopename_old = virSystemdMakeScopeName(name, drivername, true)))
d71922
         goto cleanup;
d71922
 
d71922
-    if (virCgroupPartitionEscape(&scopename) < 0)
d71922
+    /* We should keep trying even if this failed */
d71922
+    if (!machinename)
d71922
+        virResetLastError();
d71922
+    else if (!(scopename_new = virSystemdMakeScopeName(machinename,
d71922
+                                                       drivername, false)))
d71922
+        goto cleanup;
d71922
+
d71922
+    if (virCgroupPartitionEscape(&scopename_old) < 0)
d71922
+        goto cleanup;
d71922
+
d71922
+    if (scopename_new &&
d71922
+        virCgroupPartitionEscape(&scopename_new) < 0)
d71922
         goto cleanup;
d71922
 
d71922
     for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
d71922
@@ -290,12 +306,15 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
d71922
         tmp++;
d71922
 
d71922
         if (STRNEQ(tmp, name) &&
d71922
+            STRNEQ_NULLABLE(tmp, machinename) &&
d71922
             STRNEQ(tmp, partname) &&
d71922
-            STRNEQ(tmp, scopename)) {
d71922
+            STRNEQ(tmp, scopename_old) &&
d71922
+            STRNEQ_NULLABLE(tmp, scopename_new)) {
d71922
             VIR_DEBUG("Name '%s' for controller '%s' does not match "
d71922
-                      "'%s', '%s' or '%s'",
d71922
+                      "'%s', '%s', '%s', '%s' or '%s'",
d71922
                       tmp, virCgroupControllerTypeToString(i),
d71922
-                      name, partname, scopename);
d71922
+                      name, NULLSTR(machinename), partname,
d71922
+                      scopename_old, NULLSTR(scopename_new));
d71922
             goto cleanup;
d71922
         }
d71922
     }
d71922
@@ -304,7 +323,9 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
d71922
 
d71922
  cleanup:
d71922
     VIR_FREE(partname);
d71922
-    VIR_FREE(scopename);
d71922
+    VIR_FREE(scopename_old);
d71922
+    VIR_FREE(scopename_new);
d71922
+    VIR_FREE(machinename);
d71922
     return valid;
d71922
 }
d71922
 
d71922
@@ -1555,6 +1576,8 @@ virCgroupNewDetect(pid_t pid,
d71922
 int
d71922
 virCgroupNewDetectMachine(const char *name,
d71922
                           const char *drivername,
d71922
+                          int id,
d71922
+                          bool privileged,
d71922
                           pid_t pid,
d71922
                           int controllers,
d71922
                           virCgroupPtr *group)
d71922
@@ -1565,7 +1588,8 @@ virCgroupNewDetectMachine(const char *name,
d71922
         return -1;
d71922
     }
d71922
 
d71922
-    if (!virCgroupValidateMachineGroup(*group, name, drivername, true)) {
d71922
+    if (!virCgroupValidateMachineGroup(*group, name, drivername,
d71922
+                                       id, privileged, true)) {
d71922
         VIR_DEBUG("Failed to validate machine name for '%s' driver '%s'",
d71922
                   name, drivername);
d71922
         virCgroupFree(group);
d71922
@@ -1582,7 +1606,6 @@ virCgroupNewDetectMachine(const char *name,
d71922
 static int
d71922
 virCgroupNewMachineSystemd(const char *name,
d71922
                            const char *drivername,
d71922
-                           bool privileged,
d71922
                            const unsigned char *uuid,
d71922
                            const char *rootdir,
d71922
                            pid_t pidleader,
d71922
@@ -1602,7 +1625,6 @@ virCgroupNewMachineSystemd(const char *name,
d71922
     VIR_DEBUG("Trying to setup machine '%s' via systemd", name);
d71922
     if ((rv = virSystemdCreateMachine(name,
d71922
                                       drivername,
d71922
-                                      privileged,
d71922
                                       uuid,
d71922
                                       rootdir,
d71922
                                       pidleader,
d71922
@@ -1690,11 +1712,9 @@ virCgroupNewMachineSystemd(const char *name,
d71922
 /*
d71922
  * Returns 0 on success, -1 on fatal error
d71922
  */
d71922
-int virCgroupTerminateMachine(const char *name,
d71922
-                              const char *drivername,
d71922
-                              bool privileged)
d71922
+int virCgroupTerminateMachine(const char *name)
d71922
 {
d71922
-    return virSystemdTerminateMachine(name, drivername, privileged);
d71922
+    return virSystemdTerminateMachine(name);
d71922
 }
d71922
 
d71922
 
d71922
@@ -1749,7 +1769,6 @@ virCgroupNewMachineManual(const char *name,
d71922
 int
d71922
 virCgroupNewMachine(const char *name,
d71922
                     const char *drivername,
d71922
-                    bool privileged,
d71922
                     const unsigned char *uuid,
d71922
                     const char *rootdir,
d71922
                     pid_t pidleader,
d71922
@@ -1766,7 +1785,6 @@ virCgroupNewMachine(const char *name,
d71922
 
d71922
     if ((rv = virCgroupNewMachineSystemd(name,
d71922
                                          drivername,
d71922
-                                         privileged,
d71922
                                          uuid,
d71922
                                          rootdir,
d71922
                                          pidleader,
d71922
@@ -4220,6 +4238,8 @@ virCgroupNewDetect(pid_t pid ATTRIBUTE_UNUSED,
d71922
 int
d71922
 virCgroupNewDetectMachine(const char *name ATTRIBUTE_UNUSED,
d71922
                           const char *drivername ATTRIBUTE_UNUSED,
d71922
+                          int id ATTRIBUTE_UNUSED,
d71922
+                          bool privileged ATTRIBUTE_UNUSED,
d71922
                           pid_t pid ATTRIBUTE_UNUSED,
d71922
                           int controllers ATTRIBUTE_UNUSED,
d71922
                           virCgroupPtr *group ATTRIBUTE_UNUSED)
d71922
@@ -4230,9 +4250,7 @@ virCgroupNewDetectMachine(const char *name ATTRIBUTE_UNUSED,
d71922
 }
d71922
 
d71922
 
d71922
-int virCgroupTerminateMachine(const char *name ATTRIBUTE_UNUSED,
d71922
-                              const char *drivername ATTRIBUTE_UNUSED,
d71922
-                              bool privileged ATTRIBUTE_UNUSED)
d71922
+int virCgroupTerminateMachine(const char *name ATTRIBUTE_UNUSED)
d71922
 {
d71922
     virReportSystemError(ENXIO, "%s",
d71922
                          _("Control groups not supported on this platform"));
d71922
@@ -4243,7 +4261,6 @@ int virCgroupTerminateMachine(const char *name ATTRIBUTE_UNUSED,
d71922
 int
d71922
 virCgroupNewMachine(const char *name ATTRIBUTE_UNUSED,
d71922
                     const char *drivername ATTRIBUTE_UNUSED,
d71922
-                    bool privileged ATTRIBUTE_UNUSED,
d71922
                     const unsigned char *uuid ATTRIBUTE_UNUSED,
d71922
                     const char *rootdir ATTRIBUTE_UNUSED,
d71922
                     pid_t pidleader ATTRIBUTE_UNUSED,
d71922
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
d71922
index 63a9e1c..6570cb3 100644
d71922
--- a/src/util/vircgroup.h
d71922
+++ b/src/util/vircgroup.h
d71922
@@ -91,13 +91,15 @@ int virCgroupNewDetect(pid_t pid,
d71922
 
d71922
 int virCgroupNewDetectMachine(const char *name,
d71922
                               const char *drivername,
d71922
+                              int id,
d71922
+                              bool privileged,
d71922
                               pid_t pid,
d71922
                               int controllers,
d71922
-                              virCgroupPtr *group);
d71922
+                              virCgroupPtr *group)
d71922
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
d71922
 
d71922
 int virCgroupNewMachine(const char *name,
d71922
                         const char *drivername,
d71922
-                        bool privileged,
d71922
                         const unsigned char *uuid,
d71922
                         const char *rootdir,
d71922
                         pid_t pidleader,
d71922
@@ -110,10 +112,8 @@ int virCgroupNewMachine(const char *name,
d71922
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
d71922
     ATTRIBUTE_NONNULL(4);
d71922
 
d71922
-int virCgroupTerminateMachine(const char *name,
d71922
-                              const char *drivername,
d71922
-                              bool privileged)
d71922
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
d71922
+int virCgroupTerminateMachine(const char *name)
d71922
+    ATTRIBUTE_NONNULL(1);
d71922
 
d71922
 bool virCgroupNewIgnoreError(void);
d71922
 
d71922
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
d71922
index daaa37c..0f4e1f0 100644
d71922
--- a/src/util/virsystemd.c
d71922
+++ b/src/util/virsystemd.c
d71922
@@ -27,6 +27,7 @@
d71922
 
d71922
 #include "virsystemd.h"
d71922
 #include "viratomic.h"
d71922
+#include "virbuffer.h"
d71922
 #include "virdbus.h"
d71922
 #include "virstring.h"
d71922
 #include "viralloc.h"
d71922
@@ -78,15 +79,17 @@ static void virSystemdEscapeName(virBufferPtr buf,
d71922
 #undef VALID_CHARS
d71922
 }
d71922
 
d71922
-
d71922
 char *virSystemdMakeScopeName(const char *name,
d71922
-                              const char *drivername)
d71922
+                              const char *drivername,
d71922
+                              bool legacy_behaviour)
d71922
 {
d71922
     virBuffer buf = VIR_BUFFER_INITIALIZER;
d71922
 
d71922
     virBufferAddLit(&buf, "machine-");
d71922
-    virSystemdEscapeName(&buf, drivername);
d71922
-    virBufferAddLit(&buf, "\\x2d");
d71922
+    if (legacy_behaviour) {
d71922
+        virSystemdEscapeName(&buf, drivername);
d71922
+        virBufferAddLit(&buf, "\\x2d");
d71922
+    }
d71922
     virSystemdEscapeName(&buf, name);
d71922
     virBufferAddLit(&buf, ".scope");
d71922
 
d71922
@@ -113,10 +116,42 @@ char *virSystemdMakeSliceName(const char *partition)
d71922
     return virBufferContentAndReset(&buf;;
d71922
 }
d71922
 
d71922
+#define HOSTNAME_CHARS                                                  \
d71922
+    "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-"
d71922
 
d71922
-char *virSystemdMakeMachineName(const char *name,
d71922
-                                const char *drivername,
d71922
-                                bool privileged)
d71922
+static void
d71922
+virSystemdAppendValidMachineName(virBufferPtr buf,
d71922
+                                 const char *name)
d71922
+{
d71922
+    bool skip_dot = false;
d71922
+
d71922
+    for (; *name; name++) {
d71922
+        if (strlen(virBufferCurrentContent(buf)) >= 64)
d71922
+            break;
d71922
+
d71922
+        if (*name == '.') {
d71922
+            if (!skip_dot)
d71922
+                virBufferAddChar(buf, *name);
d71922
+            skip_dot = true;
d71922
+            continue;
d71922
+        }
d71922
+
d71922
+        skip_dot = false;
d71922
+
d71922
+        if (!strchr(HOSTNAME_CHARS, *name))
d71922
+            continue;
d71922
+
d71922
+        virBufferAddChar(buf, *name);
d71922
+    }
d71922
+}
d71922
+
d71922
+#undef HOSTNAME_CHARS
d71922
+
d71922
+char *
d71922
+virSystemdMakeMachineName(const char *drivername,
d71922
+                          int id,
d71922
+                          const char *name,
d71922
+                          bool privileged)
d71922
 {
d71922
     char *machinename = NULL;
d71922
     char *username = NULL;
d71922
@@ -131,7 +166,8 @@ char *virSystemdMakeMachineName(const char *name,
d71922
         virBufferAsprintf(&buf, "%s-%s-", username, drivername);
d71922
     }
d71922
 
d71922
-    virSystemdEscapeName(&buf, name);
d71922
+    virBufferAsprintf(&buf, "%d-", id);
d71922
+    virSystemdAppendValidMachineName(&buf, name);
d71922
 
d71922
     machinename = virBufferContentAndReset(&buf;;
d71922
  cleanup:
d71922
@@ -212,7 +248,6 @@ virSystemdGetMachineNameByPID(pid_t pid)
d71922
  */
d71922
 int virSystemdCreateMachine(const char *name,
d71922
                             const char *drivername,
d71922
-                            bool privileged,
d71922
                             const unsigned char *uuid,
d71922
                             const char *rootdir,
d71922
                             pid_t pidleader,
d71922
@@ -223,7 +258,6 @@ int virSystemdCreateMachine(const char *name,
d71922
 {
d71922
     int ret;
d71922
     DBusConnection *conn;
d71922
-    char *machinename = NULL;
d71922
     char *creatorname = NULL;
d71922
     char *slicename = NULL;
d71922
     static int hasCreateWithNetwork = 1;
d71922
@@ -239,8 +273,6 @@ int virSystemdCreateMachine(const char *name,
d71922
         return -1;
d71922
 
d71922
     ret = -1;
d71922
-    if (!(machinename = virSystemdMakeMachineName(name, drivername, privileged)))
d71922
-        goto cleanup;
d71922
 
d71922
     if (virAsprintf(&creatorname, "libvirt-%s", drivername) < 0)
d71922
         goto cleanup;
d71922
@@ -318,7 +350,7 @@ int virSystemdCreateMachine(const char *name,
d71922
                               "org.freedesktop.machine1.Manager",
d71922
                               "CreateMachineWithNetwork",
d71922
                               "sayssusa&ia(sv)",
d71922
-                              machinename,
d71922
+                              name,
d71922
                               16,
d71922
                               uuid[0], uuid[1], uuid[2], uuid[3],
d71922
                               uuid[4], uuid[5], uuid[6], uuid[7],
d71922
@@ -360,7 +392,7 @@ int virSystemdCreateMachine(const char *name,
d71922
                               "org.freedesktop.machine1.Manager",
d71922
                               "CreateMachine",
d71922
                               "sayssusa(sv)",
d71922
-                              machinename,
d71922
+                              name,
d71922
                               16,
d71922
                               uuid[0], uuid[1], uuid[2], uuid[3],
d71922
                               uuid[4], uuid[5], uuid[6], uuid[7],
d71922
@@ -381,20 +413,19 @@ int virSystemdCreateMachine(const char *name,
d71922
 
d71922
  cleanup:
d71922
     VIR_FREE(creatorname);
d71922
-    VIR_FREE(machinename);
d71922
     VIR_FREE(slicename);
d71922
     return ret;
d71922
 }
d71922
 
d71922
-int virSystemdTerminateMachine(const char *name,
d71922
-                               const char *drivername,
d71922
-                               bool privileged)
d71922
+int virSystemdTerminateMachine(const char *name)
d71922
 {
d71922
     int ret;
d71922
     DBusConnection *conn;
d71922
-    char *machinename = NULL;
d71922
     virError error;
d71922
 
d71922
+    if (!name)
d71922
+        return 0;
d71922
+
d71922
     memset(&error, 0, sizeof(error));
d71922
 
d71922
     ret = virDBusIsServiceEnabled("org.freedesktop.machine1");
d71922
@@ -409,9 +440,6 @@ int virSystemdTerminateMachine(const char *name,
d71922
     if (!(conn = virDBusGetSystemBus()))
d71922
         goto cleanup;
d71922
 
d71922
-    if (!(machinename = virSystemdMakeMachineName(name, drivername, privileged)))
d71922
-        goto cleanup;
d71922
-
d71922
     /*
d71922
      * The systemd DBus API we're invoking has the
d71922
      * following signature
d71922
@@ -431,7 +459,7 @@ int virSystemdTerminateMachine(const char *name,
d71922
                           "org.freedesktop.machine1.Manager",
d71922
                           "TerminateMachine",
d71922
                           "s",
d71922
-                          machinename) < 0)
d71922
+                          name) < 0)
d71922
         goto cleanup;
d71922
 
d71922
     if (error.code == VIR_ERR_ERROR &&
d71922
@@ -446,7 +474,6 @@ int virSystemdTerminateMachine(const char *name,
d71922
  cleanup:
d71922
     virResetError(&error);
d71922
 
d71922
-    VIR_FREE(machinename);
d71922
     return ret;
d71922
 }
d71922
 
d71922
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
d71922
index a13a4c0..93b0aae 100644
d71922
--- a/src/util/virsystemd.h
d71922
+++ b/src/util/virsystemd.h
d71922
@@ -25,16 +25,17 @@
d71922
 # include "internal.h"
d71922
 
d71922
 char *virSystemdMakeScopeName(const char *name,
d71922
-                              const char *drivername);
d71922
+                              const char *drivername,
d71922
+                              bool legacy_behaviour);
d71922
 char *virSystemdMakeSliceName(const char *partition);
d71922
 
d71922
-char *virSystemdMakeMachineName(const char *name,
d71922
-                                const char *drivername,
d71922
+char *virSystemdMakeMachineName(const char *drivername,
d71922
+                                int id,
d71922
+                                const char *name,
d71922
                                 bool privileged);
d71922
 
d71922
 int virSystemdCreateMachine(const char *name,
d71922
                             const char *drivername,
d71922
-                            bool privileged,
d71922
                             const unsigned char *uuid,
d71922
                             const char *rootdir,
d71922
                             pid_t pidleader,
d71922
@@ -43,9 +44,7 @@ int virSystemdCreateMachine(const char *name,
d71922
                             int *nicindexes,
d71922
                             const char *partition);
d71922
 
d71922
-int virSystemdTerminateMachine(const char *name,
d71922
-                               const char *drivername,
d71922
-                               bool privileged);
d71922
+int virSystemdTerminateMachine(const char *name);
d71922
 
d71922
 void virSystemdNotifyStartup(void);
d71922
 
d71922
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
d71922
index 3a3cd99..46452dd 100644
d71922
--- a/tests/virsystemdtest.c
d71922
+++ b/tests/virsystemdtest.c
d71922
@@ -166,7 +166,6 @@ static int testCreateContainer(const void *opaque ATTRIBUTE_UNUSED)
d71922
     };
d71922
     if (virSystemdCreateMachine("demo",
d71922
                                 "lxc",
d71922
-                                true,
d71922
                                 uuid,
d71922
                                 "/proc/123/root",
d71922
                                 123,
d71922
@@ -182,9 +181,7 @@ static int testCreateContainer(const void *opaque ATTRIBUTE_UNUSED)
d71922
 
d71922
 static int testTerminateContainer(const void *opaque ATTRIBUTE_UNUSED)
d71922
 {
d71922
-    if (virSystemdTerminateMachine("demo",
d71922
-                                   "lxc",
d71922
-                                   true) < 0) {
d71922
+    if (virSystemdTerminateMachine("lxc-demo") < 0) {
d71922
         fprintf(stderr, "%s", "Failed to terminate LXC machine\n");
d71922
         return -1;
d71922
     }
d71922
@@ -202,7 +199,6 @@ static int testCreateMachine(const void *opaque ATTRIBUTE_UNUSED)
d71922
     };
d71922
     if (virSystemdCreateMachine("demo",
d71922
                                 "qemu",
d71922
-                                false,
d71922
                                 uuid,
d71922
                                 NULL,
d71922
                                 123,
d71922
@@ -218,9 +214,7 @@ static int testCreateMachine(const void *opaque ATTRIBUTE_UNUSED)
d71922
 
d71922
 static int testTerminateMachine(const void *opaque ATTRIBUTE_UNUSED)
d71922
 {
d71922
-    if (virSystemdTerminateMachine("demo",
d71922
-                                   "qemu",
d71922
-                                   false) < 0) {
d71922
+    if (virSystemdTerminateMachine("test-qemu-demo") < 0) {
d71922
         fprintf(stderr, "%s", "Failed to terminate KVM machine\n");
d71922
         return -1;
d71922
     }
d71922
@@ -242,7 +236,6 @@ static int testCreateNoSystemd(const void *opaque ATTRIBUTE_UNUSED)
d71922
 
d71922
     if ((rv = virSystemdCreateMachine("demo",
d71922
                                       "qemu",
d71922
-                                      true,
d71922
                                       uuid,
d71922
                                       NULL,
d71922
                                       123,
d71922
@@ -277,7 +270,6 @@ static int testCreateSystemdNotRunning(const void *opaque ATTRIBUTE_UNUSED)
d71922
 
d71922
     if ((rv = virSystemdCreateMachine("demo",
d71922
                                       "qemu",
d71922
-                                      true,
d71922
                                       uuid,
d71922
                                       NULL,
d71922
                                       123,
d71922
@@ -312,7 +304,6 @@ static int testCreateBadSystemd(const void *opaque ATTRIBUTE_UNUSED)
d71922
 
d71922
     if ((rv = virSystemdCreateMachine("demo",
d71922
                                       "qemu",
d71922
-                                      true,
d71922
                                       uuid,
d71922
                                       NULL,
d71922
                                       123,
d71922
@@ -348,7 +339,6 @@ static int testCreateNetwork(const void *opaque ATTRIBUTE_UNUSED)
d71922
     size_t nnicindexes = ARRAY_CARDINALITY(nicindexes);
d71922
     if (virSystemdCreateMachine("demo",
d71922
                                 "lxc",
d71922
-                                true,
d71922
                                 uuid,
d71922
                                 "/proc/123/root",
d71922
                                 123,
d71922
@@ -385,6 +375,8 @@ testGetMachineName(const void *opaque ATTRIBUTE_UNUSED)
d71922
 struct testNameData {
d71922
     const char *name;
d71922
     const char *expected;
d71922
+    int id;
d71922
+    bool legacy;
d71922
 };
d71922
 
d71922
 static int
d71922
@@ -394,7 +386,7 @@ testScopeName(const void *opaque)
d71922
     int ret = -1;
d71922
     char *actual = NULL;
d71922
 
d71922
-    if (!(actual = virSystemdMakeScopeName(data->name, "lxc")))
d71922
+    if (!(actual = virSystemdMakeScopeName(data->name, "lxc", data->legacy)))
d71922
         goto cleanup;
d71922
 
d71922
     if (STRNEQ(actual, data->expected)) {
d71922
@@ -417,7 +409,8 @@ testMachineName(const void *opaque)
d71922
     int ret = -1;
d71922
     char *actual = NULL;
d71922
 
d71922
-    if (!(actual = virSystemdMakeMachineName(data->name, "qemu", true)))
d71922
+    if (!(actual = virSystemdMakeMachineName("qemu", data->id,
d71922
+                                             data->name, true)))
d71922
         goto cleanup;
d71922
 
d71922
     if (STRNEQ(actual, data->expected)) {
d71922
@@ -518,6 +511,12 @@ mymain(void)
d71922
 {
d71922
     int ret = 0;
d71922
 
d71922
+    unsigned char uuid[VIR_UUID_BUFLEN];
d71922
+
d71922
+    /* The one we use in tests quite often */
d71922
+    if (virUUIDParse("c7a5fdbd-edaf-9455-926a-d65c16db1809", uuid) < 0)
d71922
+        return EXIT_FAILURE;
d71922
+
d71922
     if (virtTestRun("Test create container ", testCreateContainer, NULL) < 0)
d71922
         ret = -1;
d71922
     if (virtTestRun("Test terminate container ", testTerminateContainer, NULL) < 0)
d71922
@@ -538,35 +537,47 @@ mymain(void)
d71922
     if (virtTestRun("Test getting machine name ", testGetMachineName, NULL) < 0)
d71922
         ret = -1;
d71922
 
d71922
-# define TEST_SCOPE(name, unitname)                                     \
d71922
+# define TEST_SCOPE(_name, unitname, _legacy)                           \
d71922
     do {                                                                \
d71922
         struct testNameData data = {                                    \
d71922
-            name, unitname                                              \
d71922
+            .name = _name, .expected = unitname, .legacy = _legacy,     \
d71922
         };                                                              \
d71922
         if (virtTestRun("Test scopename", testScopeName, &data) < 0)    \
d71922
             ret = -1;                                                   \
d71922
     } while (0)
d71922
 
d71922
-    TEST_SCOPE("demo", "machine-lxc\\x2ddemo.scope");
d71922
-    TEST_SCOPE("demo-name", "machine-lxc\\x2ddemo\\x2dname.scope");
d71922
-    TEST_SCOPE("demo!name", "machine-lxc\\x2ddemo\\x21name.scope");
d71922
-    TEST_SCOPE(".demo", "machine-lxc\\x2d\\x2edemo.scope");
d71922
-    TEST_SCOPE("bull💩", "machine-lxc\\x2dbull\\xf0\\x9f\\x92\\xa9.scope");
d71922
+# define TEST_SCOPE_OLD(name, unitname)         \
d71922
+    TEST_SCOPE(name, unitname, true)
d71922
+# define TEST_SCOPE_NEW(name, unitname)         \
d71922
+    TEST_SCOPE(name, unitname, false)
d71922
 
d71922
-# define TEST_MACHINE(name, machinename)                                \
d71922
+    TEST_SCOPE_OLD("demo", "machine-lxc\\x2ddemo.scope");
d71922
+    TEST_SCOPE_OLD("demo-name", "machine-lxc\\x2ddemo\\x2dname.scope");
d71922
+    TEST_SCOPE_OLD("demo!name", "machine-lxc\\x2ddemo\\x21name.scope");
d71922
+    TEST_SCOPE_OLD(".demo", "machine-lxc\\x2d\\x2edemo.scope");
d71922
+    TEST_SCOPE_OLD("bull💩", "machine-lxc\\x2dbull\\xf0\\x9f\\x92\\xa9.scope");
d71922
+
d71922
+    TEST_SCOPE_NEW("qemu-3-demo", "machine-qemu\\x2d3\\x2ddemo.scope");
d71922
+
d71922
+# define TEST_MACHINE(_name, _id, machinename)                           \
d71922
     do {                                                                \
d71922
         struct testNameData data = {                                    \
d71922
-            name, machinename                                           \
d71922
+            .name = _name, .expected = machinename, .id = _id,         \
d71922
         };                                                              \
d71922
         if (virtTestRun("Test scopename", testMachineName, &data) < 0)  \
d71922
             ret = -1;                                                   \
d71922
     } while (0)
d71922
 
d71922
-    TEST_MACHINE("demo", "qemu-demo");
d71922
-    TEST_MACHINE("demo-name", "qemu-demo\\x2dname");
d71922
-    TEST_MACHINE("demo!name", "qemu-demo\\x21name");
d71922
-    TEST_MACHINE(".demo", "qemu-\\x2edemo");
d71922
-    TEST_MACHINE("bull\U0001f4a9", "qemu-bull\\xf0\\x9f\\x92\\xa9");
d71922
+    TEST_MACHINE("demo", 1, "qemu-1-demo");
d71922
+    TEST_MACHINE("demo-name", 2, "qemu-2-demo-name");
d71922
+    TEST_MACHINE("demo!name", 3, "qemu-3-demoname");
d71922
+    TEST_MACHINE(".demo", 4, "qemu-4-.demo");
d71922
+    TEST_MACHINE("bull\U0001f4a9", 5, "qemu-5-bull");
d71922
+    TEST_MACHINE("demo..name", 6, "qemu-6-demo.name");
d71922
+    TEST_MACHINE("12345678901234567890123456789012345678901234567890123456789", 7,
d71922
+                 "qemu-7-123456789012345678901234567890123456789012345678901234567");
d71922
+    TEST_MACHINE("123456789012345678901234567890123456789012345678901234567890", 8,
d71922
+                 "qemu-8-123456789012345678901234567890123456789012345678901234567");
d71922
 
d71922
 # define TESTS_PM_SUPPORT_HELPER(name, function)                        \
d71922
     do {                                                                \
d71922
-- 
d71922
2.7.2
d71922