5:46 p.m.
This series adds qemu cli comma escaping to several places that
are dependent on the VM name, to enable names with embedded commas.
Patch 4 makes use of qemu -name guest=X value to allow names with
'=' in them.
There's likely other places that are VM name dependent that need
escaping too, but this hits the mandatory ones. I'm going to through
the remaining list on the BiteSizedTasks page
Cole Robinson (4):
qemu: command: escape commas in VM name
qemu: command: escape commas in secret master path
qemu: command: escape commas in chardev socket path
qemu: command: Use -name guest= if available
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 22 ++++++++++++--------
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 1 +
.../qemuxml2argvdata/qemuxml2argv-name-escape.args | 24 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-name-escape.xml | 18 ++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
10 files changed, 65 insertions(+), 8 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
--
2.7.3
5:46 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: command: escape commas in VM name
This isn't sufficient on its own, since the VM name is used for things
like monitor paths, which we don't escape yet
---
src/qemu/qemu_command.c | 2 +-
.../qemuxml2argvdata/qemuxml2argv-name-escape.args | 24 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-name-escape.xml | 18 ++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
4 files changed, 45 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ef4a62d..8324639 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6729,7 +6729,7 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
virCommandAddArg(cmd, "-name");
- virBufferAsprintf(&buf, "%s", def->name);
+ virBufferEscape(&buf, ',', ",", "%s", def->name);
if (cfg->setProcessName &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_NAME_PROCESS))
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
new file mode 100644
index 0000000..94a3133
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
@@ -0,0 +1,24 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name foo,,bar,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo,\
+bar/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo,bar/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
new file mode 100644
index 0000000..3a8c3cd
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
@@ -0,0 +1,18 @@
+<domain type='qemu'>
+ <name>foo,bar</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8842b2f..13b07c7 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1875,6 +1875,8 @@ mymain(void)
VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS,
NONE);
+ DO_TEST("name-escape", QEMU_CAPS_NAME_DEBUG_THREADS,
+ QEMU_CAPS_OBJECT_SECRET, QEMU_CAPS_CHARDEV);
DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
--
2.7.3
5:46 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: command: escape commas in secret master path
Need to convert the local function to virBuffer usage, so we
can use virBufferEscape
---
src/qemu/qemu_command.c | 7 +++++--
tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8324639..6defa56 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -181,6 +181,7 @@ qemuBuildMasterKeyCommandLine(virCommandPtr cmd,
int ret = -1;
char *alias = NULL;
char *path = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
/* If the -object secret does not exist, then just return. This just
* means the domain won't be able to use a secret master key and is
@@ -203,12 +204,14 @@ qemuBuildMasterKeyCommandLine(virCommandPtr cmd,
goto cleanup;
virCommandAddArg(cmd, "-object");
- virCommandAddArgFormat(cmd, "secret,id=%s,format=raw,file=%s",
- alias, path);
+ virBufferAsprintf(&buf, "secret,id=%s,format=raw,file=", alias);
+ virBufferEscape(&buf, ',', ",", "%s", path);
+ virCommandAddArgBuffer(cmd, &buf);
ret = 0;
cleanup:
+ virBufferFreeAndReset(&buf);
VIR_FREE(alias);
VIR_FREE(path);
return ret;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
index 94a3133..a5e35b8 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
@@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
/usr/bin/qemu \
-name foo,,bar,debug-threads=on \
-S \
--object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo,\
+-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo,,\
bar/master-key.aes \
-M pc \
-m 214 \
--
2.7.3
5:46 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: command: escape commas in chardev socket path
---
src/qemu/qemu_command.c | 9 ++++-----
tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +-
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6defa56..262e8d9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4772,11 +4772,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
- virBufferAsprintf(&buf,
- "socket,id=char%s,path=%s%s",
- alias,
- dev->data.nix.path,
- dev->data.nix.listen ? ",server,nowait" :
"");
+ virBufferAsprintf(&buf, "socket,id=char%s,path=", alias);
+ virBufferEscape(&buf, ',', ",", "%s",
dev->data.nix.path);
+ if (dev->data.nix.listen)
+ virBufferAddLit(&buf, ",server,nowait");
break;
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
index a5e35b8..772d94f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
@@ -15,7 +15,7 @@ bar/master-key.aes \
-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
-nographic \
-nodefaults \
--chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo,bar/monitor.sock,\
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo,,bar/monitor.sock,\
server,nowait \
-mon chardev=charmonitor,id=monitor,mode=readline \
-no-acpi \
--
2.7.3
5:46 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: command: Use -name guest= if available
-name guest= is the explicit parameter for passing a VM name. Using
it is required to allow a VM with an '=' in the name
https://bugzilla.redhat.com/show_bug.cgi?id=1276485
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 4 ++++
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 1 +
tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 8 ++++----
tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml | 2 +-
tests/qemuxml2argvtest.c | 2 +-
10 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index de54788..57c972d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -325,6 +325,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"pxb",
"pxb-pcie", /* 220 */
+ "name-guest",
);
@@ -2649,6 +2650,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] =
{
{ "spice", "gl", QEMU_CAPS_SPICE_GL },
{ "chardev", "logfile", QEMU_CAPS_CHARDEV_LOGFILE },
{ "name", "debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS },
+ { "name", "guest", QEMU_CAPS_NAME_GUEST },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 83b8be3..309017f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -356,6 +356,7 @@ typedef enum {
/* 220 */
QEMU_CAPS_DEVICE_PXB_PCIE, /* -device pxb-pcie */
+ QEMU_CAPS_NAME_GUEST, /* Is -name guest= available */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 262e8d9..8f64ef2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6731,6 +6731,10 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
virCommandAddArg(cmd, "-name");
+ /* The 'guest' option let's us handle a name with '=' embedded in
it */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NAME_GUEST))
+ virBufferAddLit(&buf, "guest=");
+
virBufferEscape(&buf, ',', ",", "%s", def->name);
if (cfg->setProcessName &&
diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
index 8175743..1cb4116 100644
--- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
@@ -162,4 +162,5 @@
<flag name='qxl.vram64_size_mb'/>
<flag name='qxl-vga.vram64_size_mb'/>
<flag name='debug-threads'/>
+ <flag name='name-guest'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
index 2cdf961..f2b8c62 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
@@ -174,4 +174,5 @@
<flag name='qxl-vga.vram64_size_mb'/>
<flag name='debug-threads'/>
<flag name='pxb'/>
+ <flag name='name-guest'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
index ecaa165..046a25c 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
@@ -175,4 +175,5 @@
<flag name='qxl-vga.vram64_size_mb'/>
<flag name='debug-threads'/>
<flag name='pxb'/>
+ <flag name='name-guest'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps
b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps
index e243f8a..24c92c9 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps
@@ -180,4 +180,5 @@
<flag name='secret'/>
<flag name='pxb'/>
<flag name='pxb-pcie'/>
+ <flag name='name-guest'/>
</qemuCaps>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
index 772d94f..7ecdca5 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
@@ -5,17 +5,17 @@ USER=test \
LOGNAME=test \
QEMU_AUDIO_DRV=none \
/usr/bin/qemu \
--name foo,,bar,debug-threads=on \
+-name guest=foo=1,,bar=2,debug-threads=on \
-S \
--object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo,,\
-bar/master-key.aes \
+-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo=1,,\
+bar=2/master-key.aes \
-M pc \
-m 214 \
-smp 1 \
-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
-nographic \
-nodefaults \
--chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo,,bar/monitor.sock,\
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo=1,,bar=2/monitor.sock,\
server,nowait \
-mon chardev=charmonitor,id=monitor,mode=readline \
-no-acpi \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
index 3a8c3cd..0057062 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
@@ -1,5 +1,5 @@
<domain type='qemu'>
- <name>foo,bar</name>
+ <name>foo=1,bar=2</name>
<uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
<memory unit='KiB'>219100</memory>
<currentMemory unit='KiB'>219100</currentMemory>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 13b07c7..d32e104 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1876,7 +1876,7 @@ mymain(void)
NONE);
DO_TEST("name-escape", QEMU_CAPS_NAME_DEBUG_THREADS,
- QEMU_CAPS_OBJECT_SECRET, QEMU_CAPS_CHARDEV);
+ QEMU_CAPS_OBJECT_SECRET, QEMU_CAPS_CHARDEV, QEMU_CAPS_NAME_GUEST);
DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
--
2.7.3
7:47 a.m.
On 04/22/2016 06:46 PM, Cole Robinson wrote:
This series adds qemu cli comma escaping to several places that
are dependent on the VM name, to enable names with embedded commas.
Patch 4 makes use of qemu -name guest=X value to allow names with
'=' in them.
There's likely other places that are VM name dependent that need
escaping too, but this hits the mandatory ones. I'm going to through
the remaining list on the BiteSizedTasks page
Cole Robinson (4):
qemu: command: escape commas in VM name
qemu: command: escape commas in secret master path
qemu: command: escape commas in chardev socket path
qemu: command: Use -name guest= if available
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 22 ++++++++++++--------
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 1 +
.../qemuxml2argvdata/qemuxml2argv-name-escape.args | 24 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-name-escape.xml | 18 ++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
10 files changed, 65 insertions(+), 8 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
Is "ewww" the proper word here ;-)
What about the vmagent path?
Since the 'path' that the masterKey uses is essentially the priv->libDir
path, would fixing that path to have escaped chars work? (just typing
without researching, thinking)... Similarly channelTargetDir. IOW:
qemuDomainSetPrivatePaths
What about places in qemu_process which use the [obj->]def->name to
build Path's (stateDir).
Does this work with domain name rename? migrations? If the target host
doesn't support "guest=".
I guess I'm also somewhat surprised that there'd be no issues w/ cgroups
and systemd interactions.
Just some quick Saturday morning thoughts with only 1 cup of coffee...
John
8:06 a.m.
On 04/23/2016 08:47 AM, John Ferlan wrote:
On 04/22/2016 06:46 PM, Cole Robinson wrote:
> This series adds qemu cli comma escaping to several places that
> are dependent on the VM name, to enable names with embedded commas.
>
> Patch 4 makes use of qemu -name guest=X value to allow names with
> '=' in them.
>
> There's likely other places that are VM name dependent that need
> escaping too, but this hits the mandatory ones. I'm going to through
> the remaining list on the BiteSizedTasks page
>
> Cole Robinson (4):
> qemu: command: escape commas in VM name
> qemu: command: escape commas in secret master path
> qemu: command: escape commas in chardev socket path
> qemu: command: Use -name guest= if available
>
> src/qemu/qemu_capabilities.c | 2 ++
> src/qemu/qemu_capabilities.h | 1 +
> src/qemu/qemu_command.c | 22 ++++++++++++--------
> tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 +
> tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 +
> tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 1 +
> tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 1 +
> .../qemuxml2argvdata/qemuxml2argv-name-escape.args | 24 ++++++++++++++++++++++
> .../qemuxml2argvdata/qemuxml2argv-name-escape.xml | 18 ++++++++++++++++
> tests/qemuxml2argvtest.c | 2 ++
> 10 files changed, 65 insertions(+), 8 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
>
Is "ewww" the proper word here ;-)
Maybe, seems to be popular on the list recently ;)
What about the vmagent path?
? 'git grep -i vmagent' doesn't give me anything. I guess you mean qemu-ga
path... that's handled by the chardev socket= escaping, which is what most
<channel> devices will be using.
Since the 'path' that the masterKey uses is essentially the
priv->libDir
path, would fixing that path to have escaped chars work? (just typing
without researching, thinking)... Similarly channelTargetDir. IOW:
qemuDomainSetPrivatePaths
Escaping these paths is a qemu command line thing only. Editing libDir
directly means that non-cli uses of libDir in the code will be trying to
access the wrong path (one with a literal double comma in it, vs the actual
single comma)
What about places in qemu_process which use the
[obj->]def->name to
build Path's (stateDir).
Those don't matter, they can have embedded comma/equals just fine. Those
characters are only a problem on the qemu command line which uses it as a
delimiter, so it needs special handling. For regular FS paths those characters
are totally legit
Does this work with domain name rename? migrations? If the target
host
doesn't support "guest=".
If the target host doesn't support guest= it may fail, but only if the name
has an embedded equals sign. I think that's fine.
I guess I'm also somewhat surprised that there'd be no issues
w/ cgroups
and systemd interactions.
Just some quick Saturday morning thoughts with only 1 cup of coffee...
See above, I think maybe you are confusing this 'escaping' with general shell
escaping? Or I myself need more coffee, still working on cup #1 :)
- Cole
3171
days inactive
3172
days old
6 comments
2 participants
participants (2)
-
Cole Robinson -
John Ferlan