[PATCH v2] conf: Don't generate machine names with a dot

Michal Privoznik posted 1 patch 4 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/5348817cbd0e972efd495648f7374d6171212ee4.1584022628.git.mprivozn@redhat.com
src/conf/domain_conf.c | 6 +++---
tests/virsystemdtest.c | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
[PATCH v2] conf: Don't generate machine names with a dot
Posted by Michal Privoznik 4 years ago
According to the linked BZ, machined expects either valid
hostname or valid FQDN (see systemd commit
v239-3092-gd65652f1f2). While in case of multiple dots, a
trailing one doesn't violate FQDN, it does violate the rule in
case of something simple, like "domain.". But it's safe to remove
it in both cases.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1808499
Fixes: 45464db8ba502764cf37ec9335770248bdb3d9a8

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

v2 of:

https://www.redhat.com/archives/libvir-list/2020-February/msg01138.html

diff to v1:
- Adjusted commit message as suggested by Jano
- Fixed ".-" and "-." occurrences too, again suggested by Jano

 src/conf/domain_conf.c | 6 +++---
 tests/virsystemdtest.c | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d2d97daf80..246a78d39b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30933,7 +30933,7 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
         if (strlen(virBufferCurrentContent(buf)) >= 64)
             break;
 
-        if (*name == '.') {
+        if (*name == '.' || *name == '-') {
             if (!skip_dot)
                 virBufferAddChar(buf, *name);
             skip_dot = true;
@@ -30948,8 +30948,8 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
         virBufferAddChar(buf, *name);
     }
 
-    /* trailing dashes are not allowed */
-    virBufferTrimChars(buf, "-");
+    /* trailing dashes or dots are not allowed */
+    virBufferTrimChars(buf, "-.");
 }
 
 #undef HOSTNAME_CHARS
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 1e36298189..f48a714cca 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -746,7 +746,8 @@ mymain(void)
     TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)", 100,
                  "qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec");
     TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)", 10,
-                 "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec");
+                 "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec-c");
+    TEST_MACHINE("demo.-.test.", 11, "qemu-11-demo.test");
 
 # define TESTS_PM_SUPPORT_HELPER(name, function) \
     do { \
-- 
2.24.1

Re: [PATCH v2] conf: Don't generate machine names with a dot
Posted by Ján Tomko 4 years ago
On a Thursday in 2020, Michal Privoznik wrote:
>According to the linked BZ, machined expects either valid
>hostname or valid FQDN (see systemd commit
>v239-3092-gd65652f1f2). While in case of multiple dots, a
>trailing one doesn't violate FQDN, it does violate the rule in
>case of something simple, like "domain.". But it's safe to remove
>it in both cases.
>
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1808499
>Fixes: 45464db8ba502764cf37ec9335770248bdb3d9a8
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
>
>v2 of:
>
>https://www.redhat.com/archives/libvir-list/2020-February/msg01138.html
>
>diff to v1:
>- Adjusted commit message as suggested by Jano
>- Fixed ".-" and "-." occurrences too, again suggested by Jano
>
> src/conf/domain_conf.c | 6 +++---
> tests/virsystemdtest.c | 3 ++-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index d2d97daf80..246a78d39b 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -30933,7 +30933,7 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
>         if (strlen(virBufferCurrentContent(buf)) >= 64)
>             break;
>
>-        if (*name == '.') {
>+        if (*name == '.' || *name == '-') {
>             if (!skip_dot)
>                 virBufferAddChar(buf, *name);
>             skip_dot = true;
>@@ -30948,8 +30948,8 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
>         virBufferAddChar(buf, *name);
>     }
>
>-    /* trailing dashes are not allowed */
>-    virBufferTrimChars(buf, "-");
>+    /* trailing dashes or dots are not allowed */
>+    virBufferTrimChars(buf, "-.");
> }

Since we use '-' as a delimiter, a domain name of .demo could still result in
an invalid name. All that's needed is initializing skip_dot to true.
(Feel free to rename it to 'skip' too, since it also affects dashes now.

We already have a test for that, but it needs
adjustment to test valid results:

diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index f48a714cca..1a041d0212 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -736,7 +736,7 @@ mymain(void)
      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(".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,

With that:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
>
> #undef HOSTNAME_CHARS
>diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
>index 1e36298189..f48a714cca 100644
>--- a/tests/virsystemdtest.c
>+++ b/tests/virsystemdtest.c
>@@ -746,7 +746,8 @@ mymain(void)
>     TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)", 100,
>                  "qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec");
>     TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)", 10,
>-                 "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec");
>+                 "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec-c");
>+    TEST_MACHINE("demo.-.test.", 11, "qemu-11-demo.test");
>
> # define TESTS_PM_SUPPORT_HELPER(name, function) \
>     do { \
>-- 
>2.24.1
>