7:57 a.m.
Pavel Hrdina (3):
util: introduce virStringMatch
util: introduce virBufferEscapeRegex
qemu: improve detection of UNIX path generated by libvirt
src/libvirt_private.syms | 2 +
src/qemu/qemu_domain.c | 51 ++++++++++++++++++----
src/util/virbuffer.c | 19 ++++++++
src/util/virbuffer.h | 3 ++
src/util/virstring.c | 33 ++++++++++++++
src/util/virstring.h | 3 ++
.../qemuxml2argv-channel-unix-gen-path1.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-gen-path2.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-gen-path3.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-user-path.xml | 17 ++++++++
.../qemuxml2xmlout-channel-unix-gen-path1.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-gen-path2.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-gen-path3.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-user-path.xml | 33 ++++++++++++++
tests/qemuxml2xmltest.c | 5 +++
tests/virbuftest.c | 40 +++++++++++++++++
tests/virstringtest.c | 47 ++++++++++++++++++++
17 files changed, 391 insertions(+), 9 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
--
2.13.0
7:57 a.m.
New subject: [libvirt] [PATCH v2 1/3] util: introduce virStringMatch
Simply tries to match the provided regex on a string and returns
the result. Useful if caller don't care about the matched substring
and want to just test if some pattern patches a string.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
changes in v2:
- virStringMatch returns bool instead of int
src/libvirt_private.syms | 1 +
src/util/virstring.c | 33 +++++++++++++++++++++++++++++++++
src/util/virstring.h | 3 +++
tests/virstringtest.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 84 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index afb9100c50..d32c6e7549 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2621,6 +2621,7 @@ virStringListHasString;
virStringListJoin;
virStringListLength;
virStringListRemove;
+virStringMatch;
virStringReplace;
virStringSearch;
virStringSortCompare;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 335e773d78..c8d6b0ca02 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -979,6 +979,39 @@ virStringSearch(const char *str,
}
/**
+ * virStringMatch:
+ * @str: string to match
+ * @regexp: POSIX Extended regular expression pattern used for matching
+ *
+ * Performs a POSIX extended regex match against a string.
+ * Returns true on match, false on error or no match.
+ */
+bool
+virStringMatch(const char *str,
+ const char *regexp)
+{
+ regex_t re;
+ int rv;
+
+ VIR_DEBUG("match '%s' for '%s'", str, regexp);
+
+ if ((rv = regcomp(&re, regexp, REG_EXTENDED | REG_NOSUB)) != 0) {
+ char error[100];
+ regerror(rv, &re, error, sizeof(error));
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("error while compiling regular expression '%s':
%s"),
+ regexp, error);
+ return false;
+ }
+
+ rv = regexec(&re, str, 0, NULL, 0);
+
+ regfree(&re);
+
+ return rv == 0;
+}
+
+/**
* virStringReplace:
* @haystack: the source string to process
* @oldneedle: the substring to locate
diff --git a/src/util/virstring.h b/src/util/virstring.h
index c545ca3f0d..0038a405b1 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -274,6 +274,9 @@ ssize_t virStringSearch(const char *str,
char ***matches)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+bool virStringMatch(const char *str,
+ const char *regexp);
+
char *virStringReplace(const char *haystack,
const char *oldneedle,
const char *newneedle)
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index 96bc79f832..97c6e76dcb 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -480,6 +480,38 @@ testStringSearch(const void *opaque)
}
+struct stringMatchData {
+ const char *str;
+ const char *regexp;
+ bool expectMatch;
+};
+
+static int
+testStringMatch(const void *opaque)
+{
+ const struct stringMatchData *data = opaque;
+ bool match;
+
+ match = virStringMatch(data->str, data->regexp);
+
+ if (data->expectMatch) {
+ if (!match) {
+ fprintf(stderr, "expected match for '%s' on '%s' but got
no match\n",
+ data->regexp, data->str);
+ return -1;
+ }
+ } else {
+ if (match) {
+ fprintf(stderr, "expected no match for '%s' on '%s' but
got match\n",
+ data->regexp, data->str);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+
struct stringReplaceData {
const char *haystack;
const char *oldneedle;
@@ -803,6 +835,21 @@ mymain(void)
const char *matches3[] = { "foo", "bar" };
TEST_SEARCH("1foo2bar3eek", "([a-z]+)", 2, 2, matches3, false);
+#define TEST_MATCH(s, r, m) \
+ do { \
+ struct stringMatchData data = { \
+ .str = s, \
+ .regexp = r, \
+ .expectMatch = m, \
+ }; \
+ if (virTestRun("virStringMatch " s, testStringMatch, &data) < 0)
\
+ ret = -1; \
+ } while (0)
+
+ TEST_MATCH("foo", "foo", true);
+ TEST_MATCH("foobar", "f[o]+", true);
+ TEST_MATCH("foobar", "^f[o]+$", false);
+
#define TEST_REPLACE(h, o, n, r) \
do { \
struct stringReplaceData data = { \
--
2.13.0
9:26 a.m.
New subject: [libvirt] [PATCH v2 1/3] util: introduce virStringMatch
On Fri, May 12, 2017 at 02:57:54PM +0200, Pavel Hrdina wrote:
Simply tries to match the provided regex on a string and returns
the result. Useful if caller don't care about the matched substring
and want to just test if some pattern patches a string.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
changes in v2:
- virStringMatch returns bool instead of int
src/libvirt_private.syms | 1 +
src/util/virstring.c | 33 +++++++++++++++++++++++++++++++++
src/util/virstring.h | 3 +++
tests/virstringtest.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 84 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index afb9100c50..d32c6e7549 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2621,6 +2621,7 @@ virStringListHasString;
virStringListJoin;
virStringListLength;
virStringListRemove;
+virStringMatch;
virStringReplace;
virStringSearch;
virStringSortCompare;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 335e773d78..c8d6b0ca02 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -979,6 +979,39 @@ virStringSearch(const char *str,
}
/**
+ * virStringMatch:
+ * @str: string to match
+ * @regexp: POSIX Extended regular expression pattern used for matching
+ *
+ * Performs a POSIX extended regex match against a string.
+ * Returns true on match, false on error or no match.
+ */
+bool
+virStringMatch(const char *str,
+ const char *regexp)
+{
+ regex_t re;
+ int rv;
+
+ VIR_DEBUG("match '%s' for '%s'", str, regexp);
+
+ if ((rv = regcomp(&re, regexp, REG_EXTENDED | REG_NOSUB)) != 0) {
+ char error[100];
+ regerror(rv, &re, error, sizeof(error));
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("error while compiling regular expression '%s':
%s"),
+ regexp, error);
+ return false;
Don't report the error, just return false. Or do VIR_WARN() if you
want. The conversion to bool was meant together with the fact that
lastError should not be set.
ACK with that.
9:33 a.m.
New subject: [libvirt] [PATCH v2 1/3] util: introduce virStringMatch
On Fri, May 12, 2017 at 04:26:43PM +0200, Martin Kletzander wrote:
On Fri, May 12, 2017 at 02:57:54PM +0200, Pavel Hrdina wrote:
>Simply tries to match the provided regex on a string and returns
>the result. Useful if caller don't care about the matched substring
>and want to just test if some pattern patches a string.
>
>Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>---
>
>changes in v2:
> - virStringMatch returns bool instead of int
>
> src/libvirt_private.syms | 1 +
> src/util/virstring.c | 33 +++++++++++++++++++++++++++++++++
> src/util/virstring.h | 3 +++
> tests/virstringtest.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 84 insertions(+)
>
>diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>index afb9100c50..d32c6e7549 100644
>--- a/src/libvirt_private.syms
>+++ b/src/libvirt_private.syms
>@@ -2621,6 +2621,7 @@ virStringListHasString;
> virStringListJoin;
> virStringListLength;
> virStringListRemove;
>+virStringMatch;
> virStringReplace;
> virStringSearch;
> virStringSortCompare;
>diff --git a/src/util/virstring.c b/src/util/virstring.c
>index 335e773d78..c8d6b0ca02 100644
>--- a/src/util/virstring.c
>+++ b/src/util/virstring.c
>@@ -979,6 +979,39 @@ virStringSearch(const char *str,
> }
>
> /**
>+ * virStringMatch:
>+ * @str: string to match
>+ * @regexp: POSIX Extended regular expression pattern used for matching
>+ *
>+ * Performs a POSIX extended regex match against a string.
>+ * Returns true on match, false on error or no match.
>+ */
>+bool
>+virStringMatch(const char *str,
>+ const char *regexp)
>+{
>+ regex_t re;
>+ int rv;
>+
>+ VIR_DEBUG("match '%s' for '%s'", str, regexp);
>+
>+ if ((rv = regcomp(&re, regexp, REG_EXTENDED | REG_NOSUB)) != 0) {
>+ char error[100];
>+ regerror(rv, &re, error, sizeof(error));
>+ virReportError(VIR_ERR_INTERNAL_ERROR,
>+ _("error while compiling regular expression
'%s': %s"),
>+ regexp, error);
>+ return false;
Don't report the error, just return false. Or do VIR_WARN() if you
want. The conversion to bool was meant together with the fact that
lastError should not be set.
ACK with that.
I'll go with VIR_WARN(), thanks.
Pavel
7:57 a.m.
New subject: [libvirt] [PATCH v2 2/3] util: introduce virBufferEscapeRegex
Add a helper to escape all possible meta-characters used for
POSIX extended regular expressions.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
new in v2
src/libvirt_private.syms | 1 +
src/util/virbuffer.c | 19 +++++++++++++++++++
src/util/virbuffer.h | 3 +++
tests/virbuftest.c | 40 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 63 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d32c6e7549..bbe283529b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1312,6 +1312,7 @@ virBufferCurrentContent;
virBufferError;
virBufferEscape;
virBufferEscapeN;
+virBufferEscapeRegex;
virBufferEscapeSexpr;
virBufferEscapeShell;
virBufferEscapeString;
diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 80c8e289d4..f07b119c0f 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -556,6 +556,25 @@ virBufferEscapeSexpr(virBufferPtr buf,
}
/**
+ * virBufferEscapeRegex:
+ * @buf: the buffer to append to
+ * @format: a printf like format string but with only one %s parameter
+ * @str: the string argument which needs to be escaped
+ *
+ * Do a formatted print with a single string to a buffer. The @str is
+ * escaped to avoid using POSIX extended regular expression meta-characters.
+ * Escaping is not applied to characters specified in @format. Auto
+ * indentation may be applied.
+ */
+void
+virBufferEscapeRegex(virBufferPtr buf,
+ const char *format,
+ const char *str)
+{
+ virBufferEscape(buf, '\\', "^$.|?*+()[]{}\\", format, str);
+}
+
+/**
* virBufferEscape:
* @buf: the buffer to append to
* @escape: the escape character to inject
diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h
index d1b64ca3a3..7a7014aa70 100644
--- a/src/util/virbuffer.h
+++ b/src/util/virbuffer.h
@@ -88,6 +88,9 @@ void virBufferEscapeString(virBufferPtr buf, const char *format,
const char *str);
void virBufferEscapeSexpr(virBufferPtr buf, const char *format,
const char *str);
+void virBufferEscapeRegex(virBufferPtr buf,
+ const char *format,
+ const char *str);
void virBufferEscapeShell(virBufferPtr buf, const char *str);
void virBufferURIEncodeString(virBufferPtr buf, const char *str);
diff --git a/tests/virbuftest.c b/tests/virbuftest.c
index 5905fee7d4..940e4c1c61 100644
--- a/tests/virbuftest.c
+++ b/tests/virbuftest.c
@@ -405,6 +405,35 @@ testBufEscapeN(const void *opaque)
static int
+testBufEscapeRegex(const void *opaque)
+{
+ const struct testBufAddStrData *data = opaque;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *actual;
+ int ret = -1;
+
+ virBufferEscapeRegex(&buf, "%s", data->data);
+
+ if (!(actual = virBufferContentAndReset(&buf))) {
+ VIR_TEST_DEBUG("testBufEscapeN: buf is empty");
+ goto cleanup;
+ }
+
+ if (STRNEQ_NULLABLE(actual, data->expect)) {
+ VIR_TEST_DEBUG("testBufEscapeN: Strings don't match:\n");
+ virTestDifference(stderr, data->expect, actual);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(actual);
+ return ret;
+}
+
+
+static int
testBufSetIndent(const void *opaque ATTRIBUTE_UNUSED)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -492,6 +521,17 @@ mymain(void)
DO_TEST_ESCAPEN("equal=escape", "equal\\=escape");
DO_TEST_ESCAPEN("comma,equal=escape", "comma,,equal\\=escape");
+#define DO_TEST_ESCAPE_REGEX(data, expect) \
+ do { \
+ struct testBufAddStrData info = { data, expect }; \
+ if (virTestRun("Buf: EscapeRegex", testBufEscapeRegex, &info) <
0) \
+ ret = -1; \
+ } while (0)
+
+ DO_TEST_ESCAPE_REGEX("noescape", "noescape");
+ DO_TEST_ESCAPE_REGEX("^$.|?*+()[]{}\\",
+ "\\^\\$\\.\\|\\?\\*\\+\\(\\)\\[\\]\\{\\}\\\\");
+
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.13.0
9:26 a.m.
New subject: [libvirt] [PATCH v2 2/3] util: introduce virBufferEscapeRegex
On Fri, May 12, 2017 at 02:57:55PM +0200, Pavel Hrdina wrote:
Add a helper to escape all possible meta-characters used for
POSIX extended regular expressions.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
new in v2
src/libvirt_private.syms | 1 +
src/util/virbuffer.c | 19 +++++++++++++++++++
src/util/virbuffer.h | 3 +++
tests/virbuftest.c | 40 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 63 insertions(+)
Looks good, ACK.
7:57 a.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: improve detection of UNIX path generated by libvirt
Currently we consider all UNIX paths with specific prefix as generated
by libvirt, but that's a wrong assumption. Let's make the detection
better by actually checking whether the whole path matches one of the
paths that we generate or generated in the past.
The UNIX path isn't stored in config XML since libvirt-1.3.0.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1446980
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
changes in v2:
- dropped the magic to split the path into 3 parts and use only one
regexp to match the path
src/qemu/qemu_domain.c | 51 ++++++++++++++++++----
.../qemuxml2argv-channel-unix-gen-path1.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-gen-path2.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-gen-path3.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-user-path.xml | 17 ++++++++
.../qemuxml2xmlout-channel-unix-gen-path1.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-gen-path2.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-gen-path3.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-user-path.xml | 33 ++++++++++++++
tests/qemuxml2xmltest.c | 5 +++
10 files changed, 244 insertions(+), 9 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cc02c801e1..00e37d3428 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3154,24 +3154,57 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
/*
- * Clear auto generated unix socket path, i.e., the one which starts with our
- * channel directory.
+ * Clear auto generated unix socket paths:
+ *
+ * libvirt 1.2.18 and older:
+ * {cfg->channelTargetDir}/{dom-name}.{target-name}
+ *
+ * libvirt 1.2.19 - 1.3.2:
+ * {cfg->channelTargetDir}/domain-{dom-name}/{target-name}
+ *
+ * libvirt 1.3.3 and newer:
+ * {cfg->channelTargetDir}/domain-{dom-id}-{short-dom-name}/{target-name}
+ *
+ * The unix socket path was stored in config XML until libvirt 1.3.0.
+ * If someone specifies the same path as we generate, they shouldn't do it.
+ *
+ * This function clears the path for migration as well, so we need to clear
+ * the path event if we are not storing it in the XML.
*/
-static void
+static int
qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
virQEMUDriverPtr driver)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *regexp = NULL;
+ int ret = -1;
- if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
- chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
- chr->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
- chr->source->data.nix.path &&
- STRPREFIX(chr->source->data.nix.path, cfg->channelTargetDir)) {
+ if (chr->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL ||
+ chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
+ chr->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
+ !chr->source->data.nix.path) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ virBufferEscapeRegex(&buf, "^%s", cfg->channelTargetDir);
+ virBufferAddLit(&buf, "/([^/]+\\.)|(domain-[^/]+/)");
+ virBufferEscapeRegex(&buf, "%s$", chr->target.name);
+
+ if (virBufferCheckError(&buf) < 0)
+ goto cleanup;
+
+ regexp = virBufferContentAndReset(&buf);
+
+ if (virStringMatch(chr->source->data.nix.path, regexp))
VIR_FREE(chr->source->data.nix.path);
- }
+ ret = 0;
+ cleanup:
+ VIR_FREE(regexp);
virObjectUnref(cfg);
+ return ret;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
b/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
new file mode 100644
index 0000000000..25c84e922b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
@@ -0,0 +1,17 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <channel type='unix'>
+ <source mode='bind'
path='/tmp/channel/QEMUGuest1.org.qemu.guest_agent.0'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ </channel>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
b/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
new file mode 100644
index 0000000000..2d7ca0ae77
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
@@ -0,0 +1,17 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <channel type='unix'>
+ <source mode='bind'
path='/tmp/channel/domain-QEMUGuest1/org.qemu.guest_agent.0'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ </channel>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
b/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
new file mode 100644
index 0000000000..20477016c9
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
@@ -0,0 +1,17 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <channel type='unix'>
+ <source mode='bind'
path='/tmp/channel/domain-1-QEMUGuest1/org.qemu.guest_agent.0'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ </channel>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
b/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
new file mode 100644
index 0000000000..45fdf08a66
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
@@ -0,0 +1,17 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <channel type='unix'>
+ <source mode='bind'
path='/tmp/channel/QEMUGuest1/org.qemu.guest_agent.0'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ </channel>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
new file mode 100644
index 0000000000..aa2a3099d7
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</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='x86_64' 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-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='virtio-serial' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <channel type='unix'>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
new file mode 100644
index 0000000000..aa2a3099d7
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</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='x86_64' 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-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='virtio-serial' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <channel type='unix'>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
new file mode 100644
index 0000000000..aa2a3099d7
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</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='x86_64' 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-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='virtio-serial' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <channel type='unix'>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
new file mode 100644
index 0000000000..488212d761
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
@@ -0,0 +1,33 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</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='x86_64' 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-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='virtio-serial' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <channel type='unix'>
+ <source mode='bind'
path='/tmp/channel/QEMUGuest1/org.qemu.guest_agent.0'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2dccde746e..044faf2a5d 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -551,6 +551,11 @@ mymain(void)
DO_TEST("channel-virtio", NONE);
DO_TEST("channel-virtio-state", NONE);
+ DO_TEST_FULL("channel-unix-gen-path1", WHEN_INACTIVE, GIC_NONE, NONE);
+ DO_TEST_FULL("channel-unix-gen-path2", WHEN_INACTIVE, GIC_NONE, NONE);
+ DO_TEST_FULL("channel-unix-gen-path3", WHEN_INACTIVE, GIC_NONE, NONE);
+ DO_TEST_FULL("channel-unix-user-path", WHEN_INACTIVE, GIC_NONE, NONE);
+
DO_TEST("hostdev-usb-address", NONE);
DO_TEST("hostdev-pci-address", NONE);
DO_TEST("hostdev-vfio", NONE);
--
2.13.0
9:26 a.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: improve detection of UNIX path generated by libvirt
On Fri, May 12, 2017 at 02:57:56PM +0200, Pavel Hrdina wrote:
Currently we consider all UNIX paths with specific prefix as
generated
by libvirt, but that's a wrong assumption. Let's make the detection
better by actually checking whether the whole path matches one of the
paths that we generate or generated in the past.
The UNIX path isn't stored in config XML since libvirt-1.3.0.
1.3.1, I believe.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1446980
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
changes in v2:
- dropped the magic to split the path into 3 parts and use only one
regexp to match the path
src/qemu/qemu_domain.c | 51 ++++++++++++++++++----
.../qemuxml2argv-channel-unix-gen-path1.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-gen-path2.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-gen-path3.xml | 17 ++++++++
.../qemuxml2argv-channel-unix-user-path.xml | 17 ++++++++
.../qemuxml2xmlout-channel-unix-gen-path1.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-gen-path2.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-gen-path3.xml | 32 ++++++++++++++
.../qemuxml2xmlout-channel-unix-user-path.xml | 33 ++++++++++++++
tests/qemuxml2xmltest.c | 5 +++
10 files changed, 244 insertions(+), 9 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
Just have one file that tests it all.
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cc02c801e1..00e37d3428 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3154,24 +3154,57 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
/*
- * Clear auto generated unix socket path, i.e., the one which starts with our
- * channel directory.
+ * Clear auto generated unix socket paths:
+ *
+ * libvirt 1.2.18 and older:
+ * {cfg->channelTargetDir}/{dom-name}.{target-name}
+ *
+ * libvirt 1.2.19 - 1.3.2:
+ * {cfg->channelTargetDir}/domain-{dom-name}/{target-name}
+ *
+ * libvirt 1.3.3 and newer:
+ * {cfg->channelTargetDir}/domain-{dom-id}-{short-dom-name}/{target-name}
+ *
+ * The unix socket path was stored in config XML until libvirt 1.3.0.
+ * If someone specifies the same path as we generate, they shouldn't do it.
+ *
+ * This function clears the path for migration as well, so we need to clear
+ * the path event if we are not storing it in the XML.
*/
-static void
+static int
This ^^ is not reflected anywhere. It's a pity that such function (that
just conditionally frees something) can fail.
qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
virQEMUDriverPtr driver)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *regexp = NULL;
+ int ret = -1;
- if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
- chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
- chr->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
- chr->source->data.nix.path &&
- STRPREFIX(chr->source->data.nix.path, cfg->channelTargetDir)) {
+ if (chr->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL ||
+ chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
+ chr->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
+ !chr->source->data.nix.path) {
This would be more readable if you postponed the initialization of @cfg
and just return 0 from this. Optionally break this into multiple
conditions.
+ ret = 0;
+ goto cleanup;
+ }
+
+ virBufferEscapeRegex(&buf, "^%s", cfg->channelTargetDir);
+ virBufferAddLit(&buf, "/([^/]+\\.)|(domain-[^/]+/)");
+ virBufferEscapeRegex(&buf, "%s$", chr->target.name);
+
+ if (virBufferCheckError(&buf) < 0)
+ goto cleanup;
+
No need to do this ^^, [1]
+ regexp = virBufferContentAndReset(&buf);
+
[1] Just do this:
if ((regexp = virBufferContentAndReset(&buf)) < 0)
goto cleanup;
or similar.
9:45 a.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: improve detection of UNIX path generated by libvirt
On Fri, May 12, 2017 at 04:26:35PM +0200, Martin Kletzander wrote:
On Fri, May 12, 2017 at 02:57:56PM +0200, Pavel Hrdina wrote:
>Currently we consider all UNIX paths with specific prefix as generated
>by libvirt, but that's a wrong assumption. Let's make the detection
>better by actually checking whether the whole path matches one of the
>paths that we generate or generated in the past.
>
>The UNIX path isn't stored in config XML since libvirt-1.3.0.
>
1.3.1, I believe.
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1446980
>
>Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>---
>
>changes in v2:
> - dropped the magic to split the path into 3 parts and use only one
> regexp to match the path
>
> src/qemu/qemu_domain.c | 51 ++++++++++++++++++----
> .../qemuxml2argv-channel-unix-gen-path1.xml | 17 ++++++++
> .../qemuxml2argv-channel-unix-gen-path2.xml | 17 ++++++++
> .../qemuxml2argv-channel-unix-gen-path3.xml | 17 ++++++++
> .../qemuxml2argv-channel-unix-user-path.xml | 17 ++++++++
> .../qemuxml2xmlout-channel-unix-gen-path1.xml | 32 ++++++++++++++
> .../qemuxml2xmlout-channel-unix-gen-path2.xml | 32 ++++++++++++++
> .../qemuxml2xmlout-channel-unix-gen-path3.xml | 32 ++++++++++++++
> .../qemuxml2xmlout-channel-unix-user-path.xml | 33 ++++++++++++++
> tests/qemuxml2xmltest.c | 5 +++
> 10 files changed, 244 insertions(+), 9 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
> create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
> create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
> create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
> create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
>
Just have one file that tests it all.
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index cc02c801e1..00e37d3428 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -3154,24 +3154,57 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
>
>
> /*
>- * Clear auto generated unix socket path, i.e., the one which starts with our
>- * channel directory.
>+ * Clear auto generated unix socket paths:
>+ *
>+ * libvirt 1.2.18 and older:
>+ * {cfg->channelTargetDir}/{dom-name}.{target-name}
>+ *
>+ * libvirt 1.2.19 - 1.3.2:
>+ * {cfg->channelTargetDir}/domain-{dom-name}/{target-name}
>+ *
>+ * libvirt 1.3.3 and newer:
>+ * {cfg->channelTargetDir}/domain-{dom-id}-{short-dom-name}/{target-name}
>+ *
>+ * The unix socket path was stored in config XML until libvirt 1.3.0.
>+ * If someone specifies the same path as we generate, they shouldn't do it.
>+ *
>+ * This function clears the path for migration as well, so we need to clear
>+ * the path event if we are not storing it in the XML.
> */
>-static void
>+static int
This ^^ is not reflected anywhere. It's a pity that such function (that
just conditionally frees something) can fail.
I've somehow lost the change to the callers to handle the failure, sigh.
> qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
> virQEMUDriverPtr driver)
> {
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>+ virBuffer buf = VIR_BUFFER_INITIALIZER;
>+ char *regexp = NULL;
>+ int ret = -1;
>
>- if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
>- chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>- chr->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>- chr->source->data.nix.path &&
>- STRPREFIX(chr->source->data.nix.path, cfg->channelTargetDir)) {
>+ if (chr->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL ||
>+ chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
>+ chr->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
>+ !chr->source->data.nix.path) {
This would be more readable if you postponed the initialization of @cfg
and just return 0 from this. Optionally break this into multiple
conditions.
>+ ret = 0;
>+ goto cleanup;
>+ }
>+
>+ virBufferEscapeRegex(&buf, "^%s", cfg->channelTargetDir);
>+ virBufferAddLit(&buf, "/([^/]+\\.)|(domain-[^/]+/)");
>+ virBufferEscapeRegex(&buf, "%s$", chr->target.name);
>+
>+ if (virBufferCheckError(&buf) < 0)
>+ goto cleanup;
>+
No need to do this ^^, [1]
>+ regexp = virBufferContentAndReset(&buf);
>+
[1] Just do this:
if ((regexp = virBufferContentAndReset(&buf)) < 0)
goto cleanup;
or similar.
It's not equivalent, the virBufferCheckError() also reports an error
which I want to do. I'll fix the callers to check the return value
of qemuDomainChrDefDropDefaultPath().
Pavel
4:26 a.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: improve detection of UNIX path generated by libvirt
On Fri, May 12, 2017 at 04:45:11PM +0200, Pavel Hrdina wrote:
On Fri, May 12, 2017 at 04:26:35PM +0200, Martin Kletzander wrote:
> On Fri, May 12, 2017 at 02:57:56PM +0200, Pavel Hrdina wrote:
> >Currently we consider all UNIX paths with specific prefix as generated
> >by libvirt, but that's a wrong assumption. Let's make the detection
> >better by actually checking whether the whole path matches one of the
> >paths that we generate or generated in the past.
> >
> >The UNIX path isn't stored in config XML since libvirt-1.3.0.
> >
>
> 1.3.1, I believe.
>
> >Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1446980
> >
> >Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> >---
> >
> >changes in v2:
> > - dropped the magic to split the path into 3 parts and use only one
> > regexp to match the path
> >
> > src/qemu/qemu_domain.c | 51 ++++++++++++++++++----
> > .../qemuxml2argv-channel-unix-gen-path1.xml | 17 ++++++++
> > .../qemuxml2argv-channel-unix-gen-path2.xml | 17 ++++++++
> > .../qemuxml2argv-channel-unix-gen-path3.xml | 17 ++++++++
> > .../qemuxml2argv-channel-unix-user-path.xml | 17 ++++++++
> > .../qemuxml2xmlout-channel-unix-gen-path1.xml | 32 ++++++++++++++
> > .../qemuxml2xmlout-channel-unix-gen-path2.xml | 32 ++++++++++++++
> > .../qemuxml2xmlout-channel-unix-gen-path3.xml | 32 ++++++++++++++
> > .../qemuxml2xmlout-channel-unix-user-path.xml | 33 ++++++++++++++
> > tests/qemuxml2xmltest.c | 5 +++
> > 10 files changed, 244 insertions(+), 9 deletions(-)
> > create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
> > create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
> > create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
> > create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
> > create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
> > create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
> > create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
> > create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
> >
>
> Just have one file that tests it all.
>
> >diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> >index cc02c801e1..00e37d3428 100644
> >--- a/src/qemu/qemu_domain.c
> >+++ b/src/qemu/qemu_domain.c
> >@@ -3154,24 +3154,57 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
> >
> >
> > /*
> >- * Clear auto generated unix socket path, i.e., the one which starts with our
> >- * channel directory.
> >+ * Clear auto generated unix socket paths:
> >+ *
> >+ * libvirt 1.2.18 and older:
> >+ * {cfg->channelTargetDir}/{dom-name}.{target-name}
> >+ *
> >+ * libvirt 1.2.19 - 1.3.2:
> >+ * {cfg->channelTargetDir}/domain-{dom-name}/{target-name}
> >+ *
> >+ * libvirt 1.3.3 and newer:
> >+ *
{cfg->channelTargetDir}/domain-{dom-id}-{short-dom-name}/{target-name}
> >+ *
> >+ * The unix socket path was stored in config XML until libvirt 1.3.0.
> >+ * If someone specifies the same path as we generate, they shouldn't do it.
> >+ *
> >+ * This function clears the path for migration as well, so we need to clear
> >+ * the path event if we are not storing it in the XML.
> > */
> >-static void
> >+static int
>
> This ^^ is not reflected anywhere. It's a pity that such function (that
> just conditionally frees something) can fail.
I've somehow lost the change to the callers to handle the failure, sigh.
>
> > qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
> > virQEMUDriverPtr driver)
> > {
> > virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> >+ virBuffer buf = VIR_BUFFER_INITIALIZER;
> >+ char *regexp = NULL;
> >+ int ret = -1;
> >
> >- if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
> >- chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO
&&
> >- chr->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
> >- chr->source->data.nix.path &&
> >- STRPREFIX(chr->source->data.nix.path, cfg->channelTargetDir))
{
> >+ if (chr->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL ||
> >+ chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
> >+ chr->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
> >+ !chr->source->data.nix.path) {
>
> This would be more readable if you postponed the initialization of @cfg
> and just return 0 from this. Optionally break this into multiple
> conditions.
>
> >+ ret = 0;
> >+ goto cleanup;
> >+ }
> >+
> >+ virBufferEscapeRegex(&buf, "^%s", cfg->channelTargetDir);
> >+ virBufferAddLit(&buf, "/([^/]+\\.)|(domain-[^/]+/)");
> >+ virBufferEscapeRegex(&buf, "%s$", chr->target.name);
> >+
> >+ if (virBufferCheckError(&buf) < 0)
> >+ goto cleanup;
> >+
>
> No need to do this ^^, [1]
>
> >+ regexp = virBufferContentAndReset(&buf);
> >+
>
> [1] Just do this:
>
> if ((regexp = virBufferContentAndReset(&buf)) < 0)
> goto cleanup;
>
> or similar.
It's not equivalent, the virBufferCheckError() also reports an error
which I want to do. I'll fix the callers to check the return value
of qemuDomainChrDefDropDefaultPath().
Yep, my bad, sorry. Thanks.
Pavel
2748
days inactive
2751
days old
9 comments
2 participants
participants (2)
-
Martin Kletzander -
Pavel Hrdina