From c9bd08ee352373ed121c10f0adf11a4a0a29c857 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 11 Mar 2020 17:56:12 +0100 Subject: [PATCH] conf: Don't generate clashing machine names for embed driver So far, when using the qemu:///embed driver, management applications can't chose whether they want to register their domains in machined or not. While having that option is certainly desired, it will require more work. What we can do meanwhile is to generate names that include part of hash of the root directory. This is to ensure that if two applications using different roots but the same domain name (and ID) start the domain no clashing name for machined is generated. Signed-off-by: Michal Privoznik Reviewed-by: Andrea Bolognani --- src/conf/domain_conf.c | 21 +++++++++++++++++---- src/conf/domain_conf.h | 1 + src/lxc/lxc_domain.c | 2 +- src/qemu/qemu_domain.c | 4 +++- tests/virsystemdtest.c | 35 ++++++++++++++++++++--------------- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9c56cdbebd..e0432fc47d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -62,6 +62,7 @@ #include "virdomainsnapshotobjlist.h" #include "virdomaincheckpointobjlist.h" #include "virutil.h" +#include "vircrypto.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -31076,21 +31077,33 @@ virDomainMachineNameAppendValid(virBufferPtr buf, char * virDomainGenerateMachineName(const char *drivername, + const char *root, int id, const char *name, bool privileged) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (privileged) { - virBufferAsprintf(&buf, "%s-", drivername); - } else { + virBufferAsprintf(&buf, "%s-", drivername); + + if (root) { + g_autofree char *hash = NULL; + + /* When two embed drivers start two domains with the same @name and @id + * we would generate a non-unique name. Include parts of hashed @root + * which guarantees uniqueness. The first 8 characters of SHA256 ought + * to be enough for anybody. */ + if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, root, &hash) < 0) + return NULL; + + virBufferAsprintf(&buf, "embed-%.8s-", hash); + } else if (!privileged) { g_autofree char *username = NULL; if (!(username = virGetUserName(geteuid()))) { virBufferFreeAndReset(&buf); return NULL; } - virBufferAsprintf(&buf, "%s-%s-", username, drivername); + virBufferAsprintf(&buf, "%s-", username); } virBufferAsprintf(&buf, "%d-", id); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 91b776c28a..73bd097cf8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3649,6 +3649,7 @@ int virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk, char * virDomainGenerateMachineName(const char *drivername, + const char *root, int id, const char *name, bool privileged); diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 03d0f46b24..ebd2c2b56e 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -406,7 +406,7 @@ virLXCDomainGetMachineName(virDomainDefPtr def, pid_t pid) } if (!ret) - ret = virDomainGenerateMachineName("lxc", def->id, def->name, true); + ret = virDomainGenerateMachineName("lxc", NULL, def->id, def->name, true); return ret; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index edc8ba2ddb..0e2252f6cf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -16424,6 +16424,7 @@ qemuDomainGetMachineName(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverPtr driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); char *ret = NULL; if (vm->pid > 0) { @@ -16433,7 +16434,8 @@ qemuDomainGetMachineName(virDomainObjPtr vm) } if (!ret) - ret = virDomainGenerateMachineName("qemu", vm->def->id, vm->def->name, + ret = virDomainGenerateMachineName("qemu", cfg->root, + vm->def->id, vm->def->name, virQEMUDriverIsPrivileged(driver)); return ret; diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index fa0980c845..050941dce8 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -379,6 +379,7 @@ testGetMachineName(const void *opaque G_GNUC_UNUSED) struct testNameData { const char *name; const char *expected; + const char *root; int id; bool legacy; }; @@ -413,8 +414,8 @@ testMachineName(const void *opaque) int ret = -1; char *actual = NULL; - if (!(actual = virDomainGenerateMachineName("qemu", data->id, - data->name, true))) + if (!(actual = virDomainGenerateMachineName("qemu", data->root, + data->id, data->name, true))) goto cleanup; if (STRNEQ(actual, data->expected)) { @@ -724,30 +725,34 @@ mymain(void) TEST_SCOPE_NEW("qemu-3-demo", "machine-qemu\\x2d3\\x2ddemo.scope"); -# define TEST_MACHINE(_name, _id, machinename) \ +# define TEST_MACHINE(_name, _root, _id, machinename) \ do { \ struct testNameData data = { \ - .name = _name, .expected = machinename, .id = _id, \ + .name = _name, .expected = machinename, .root = _root, .id = _id, \ }; \ if (virTestRun("Test scopename", testMachineName, &data) < 0) \ ret = -1; \ } while (0) - TEST_MACHINE("demo", 1, "qemu-1-demo"); - TEST_MACHINE("demo-name", 2, "qemu-2-demo-name"); - TEST_MACHINE("demo!name", 3, "qemu-3-demoname"); - TEST_MACHINE(".demo", 4, "qemu-4-demo"); - TEST_MACHINE("bull\U0001f4a9", 5, "qemu-5-bull"); - TEST_MACHINE("demo..name", 6, "qemu-6-demo.name"); - TEST_MACHINE("12345678901234567890123456789012345678901234567890123456789", 7, + TEST_MACHINE("demo", NULL, 1, "qemu-1-demo"); + TEST_MACHINE("demo-name", NULL, 2, "qemu-2-demo-name"); + TEST_MACHINE("demo!name", NULL, 3, "qemu-3-demoname"); + TEST_MACHINE(".demo", NULL, 4, "qemu-4-demo"); + TEST_MACHINE("bull\U0001f4a9", NULL, 5, "qemu-5-bull"); + TEST_MACHINE("demo..name", NULL, 6, "qemu-6-demo.name"); + TEST_MACHINE("12345678901234567890123456789012345678901234567890123456789", NULL, 7, "qemu-7-123456789012345678901234567890123456789012345678901234567"); - TEST_MACHINE("123456789012345678901234567890123456789012345678901234567890", 8, + TEST_MACHINE("123456789012345678901234567890123456789012345678901234567890", NULL, 8, "qemu-8-123456789012345678901234567890123456789012345678901234567"); - TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)", 100, + TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)", + NULL, 100, "qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec"); - TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)", 10, + TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)", + NULL, 10, "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec-c"); - TEST_MACHINE("demo.-.test.", 11, "qemu-11-demo.test"); + TEST_MACHINE("demo.-.test.", NULL, 11, "qemu-11-demo.test"); + TEST_MACHINE("demo", "/tmp/root1", 1, "qemu-embed-0991f456-1-demo"); + TEST_MACHINE("demo", "/tmp/root2", 1, "qemu-embed-95d47ff5-1-demo"); # define TESTS_PM_SUPPORT_HELPER(name, function) \ do { \