Re: [libvirt] [PATCH 1/3] Revert "systemd: Escape only needed characters for machined"

[Martin Kletzander]

On Wed, Feb 03, 2016 at 09:31:50PM +0100, Andrea Bolognani wrote:

On Wed, 2016-02-03 at 14:07 +0100, Pavel Hrdina wrote:

On Tue, Feb 02, 2016 at 09:22:47PM +0100, Martin Kletzander wrote:
> This reverts commit 0e0149ce91d84f40b98acf4c4bb0da6e29b9c15c.
> 
> That commit was added to comply with systemd rules that were changed in
> the meantime, so this patch is pointless.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/util/virsystemd.c  | 23 +++
>  tests/virsystemdtest.c |  4 ++--
>  2 files changed, 9 insertions(+), 18 deletions(-)
 
ACK


I have a guest named debian-q35, and after this commit I can no
longer start it:

  error: Failed to start domain debian-q35
  error: Invalid machine name

Looks like the name gets escaped as

 qemu-debian\x2dq35

and systemd doesn't like it. After reverting your revert I can
start the guest again. I'm running

  systemd-222-13.fc23.x86_64

which I assume is pretty recent.



Sorry for that, I completely forgot that in my latest series the first
two patches are dependent on the last one.  I'm trying never to do that,
but this time I wanted to split it so that it's nice to review, polus
the revert was there...

Anyway, really sorry for that, I'm just now testing the (hopefully) last
version of the third patch, will post it really soon.


Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] Revert "systemd: Escape only needed characters for machined"

[Pavel Hrdina]

On Tue, Feb 02, 2016 at 09:22:47PM +0100, Martin Kletzander wrote:
> This reverts commit 0e0149ce91d84f40b98acf4c4bb0da6e29b9c15c.
> 
> That commit was added to comply with systemd rules that were changed in
> the meantime, so this patch is pointless.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/util/virsystemd.c  | 23 +++
>  tests/virsystemdtest.c |  4 ++--
>  2 files changed, 9 insertions(+), 18 deletions(-)

ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] Revert "systemd: Escape only needed characters for machined"

[Andrea Bolognani]

On Wed, 2016-02-03 at 14:07 +0100, Pavel Hrdina wrote:
> On Tue, Feb 02, 2016 at 09:22:47PM +0100, Martin Kletzander wrote:
> > This reverts commit 0e0149ce91d84f40b98acf4c4bb0da6e29b9c15c.
> > 
> > That commit was added to comply with systemd rules that were changed in
> > the meantime, so this patch is pointless.
> > 
> > Signed-off-by: Martin Kletzander 
> > ---
> >  src/util/virsystemd.c  | 23 +++
> >  tests/virsystemdtest.c |  4 ++--
> >  2 files changed, 9 insertions(+), 18 deletions(-)
> 
> ACK

I have a guest named debian-q35, and after this commit I can no
longer start it:

  error: Failed to start domain debian-q35
  error: Invalid machine name

Looks like the name gets escaped as

 qemu-debian\x2dq35

and systemd doesn't like it. After reverting your revert I can
start the guest again. I'm running

  systemd-222-13.fc23.x86_64

which I assume is pretty recent.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/3] Revert "systemd: Escape only needed characters for machined"

[Martin Kletzander]

This reverts commit 0e0149ce91d84f40b98acf4c4bb0da6e29b9c15c.

That commit was added to comply with systemd rules that were changed in
the meantime, so this patch is pointless.

Signed-off-by: Martin Kletzander 
---
 src/util/virsystemd.c  | 23 +++
 tests/virsystemdtest.c |  4 ++--
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 337d6a208ab6..abd883c73844 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -38,17 +38,8 @@

 VIR_LOG_INIT("util.systemd");

-/**
- * virSystemdEscapeName:
- *
- * This function escapes various characters in @name and appends that
- * escaped string to @buf, in order to comply with the requirements
- * from systemd/machined.  Parameter @full_escape decides whether to
- * also escape dot as a first character and '-'.
- */
 static void virSystemdEscapeName(virBufferPtr buf,
- const char *name,
- bool full_escape)
+ const char *name)
 {
 static const char hextable[16] = "0123456789abcdef";

@@ -66,7 +57,7 @@ static void virSystemdEscapeName(virBufferPtr buf,
 "ABCDEFGHIJKLMNOPQRSTUVWXYZ"\
 ":-_.\\"

-if (full_escape && *name == '.') {
+if (*name == '.') {
 ESCAPE(*name);
 name++;
 }
@@ -74,7 +65,7 @@ static void virSystemdEscapeName(virBufferPtr buf,
 while (*name) {
 if (*name == '/')
 virBufferAddChar(buf, '-');
-else if ((full_escape && *name == '-') ||
+else if (*name == '-' ||
  *name == '\\' ||
  !strchr(VALID_CHARS, *name))
 ESCAPE(*name);
@@ -94,9 +85,9 @@ char *virSystemdMakeScopeName(const char *name,
 virBuffer buf = VIR_BUFFER_INITIALIZER;

 virBufferAddLit(, "machine-");
-virSystemdEscapeName(, drivername, true);
+virSystemdEscapeName(, drivername);
 virBufferAddLit(, "\\x2d");
-virSystemdEscapeName(, name, true);
+virSystemdEscapeName(, name);
 virBufferAddLit(, ".scope");

 if (virBufferCheckError() < 0)
@@ -113,7 +104,7 @@ char *virSystemdMakeSliceName(const char *partition)
 if (*partition == '/')
 partition++;

-virSystemdEscapeName(, partition, true);
+virSystemdEscapeName(, partition);
 virBufferAddLit(, ".slice");

 if (virBufferCheckError() < 0)
@@ -139,7 +130,7 @@ char *virSystemdMakeMachineName(const char *name,
 virBufferAsprintf(, "%s-%s-", username, drivername);
 }

-virSystemdEscapeName(, name, false);
+virSystemdEscapeName(, name);

 machinename = virBufferContentAndReset();
  cleanup:
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 49d37c2032ec..06fec5495bc2 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -517,9 +517,9 @@ mymain(void)
 } while (0)

 TEST_MACHINE("demo", "qemu-demo");
-TEST_MACHINE("demo-name", "qemu-demo-name");
+TEST_MACHINE("demo-name", "qemu-demo\\x2dname");
 TEST_MACHINE("demo!name", "qemu-demo\\x21name");
-TEST_MACHINE(".demo", "qemu-.demo");
+TEST_MACHINE(".demo", "qemu-\\x2edemo");
 TEST_MACHINE("bull\U0001f4a9", "qemu-bull\\xf0\\x9f\\x92\\xa9");

 # define TESTS_PM_SUPPORT_HELPER(name, function)\
-- 
2.7.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list