11:39 a.m.
Most of the time qemumonitorjson test isn't really testing much. Add support
for actually testing chardev hotplug and fix it for the UDP case.
Peter Krempa (6):
conf: Sanitize formatting of UDP chardev source
tests: qemu: Add support for testing aguments on monitor verbatim
tests: qemumonitorjson: Don't do multiple tests in one virTestRun
tests: qemumonitorjsontest: Do some actual testing in
qemuMonitorJSONTestAttachChardev
qemu: monitor: Simplify construction of chardev backends
qemu: monitor: Properly configure backend for UDP chardevs
src/conf/domain_conf.c | 42 ++++----
src/qemu/qemu_monitor_json.c | 51 +++++-----
tests/qemumonitorjsontest.c | 222 ++++++++++++++++++++++++++++++++-----------
tests/qemumonitortestutils.c | 112 ++++++++++++++++++++++
tests/qemumonitortestutils.h | 6 ++
5 files changed, 326 insertions(+), 107 deletions(-)
--
2.10.0
11:39 a.m.
New subject: [libvirt] [PATCH 1/6] conf: Sanitize formatting of UDP chardev source
Use much simpler logic to determine parts of the code to print.
---
src/conf/domain_conf.c | 42 ++++++++++++++++--------------------------
1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a06da46..3f34540 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21431,32 +21431,22 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
break;
case VIR_DOMAIN_CHR_TYPE_UDP:
- if (def->data.udp.bindService &&
- def->data.udp.bindHost) {
- virBufferEscapeString(buf, "<source mode='bind'
host='%s' ",
- def->data.udp.bindHost);
- virBufferEscapeString(buf, "service='%s'/>\n",
- def->data.udp.bindService);
- } else if (def->data.udp.bindHost) {
- virBufferEscapeString(buf, "<source mode='bind'
host='%s'/>\n",
- def->data.udp.bindHost);
- } else if (def->data.udp.bindService) {
- virBufferEscapeString(buf, "<source mode='bind'
service='%s'/>\n",
- def->data.udp.bindService);
- }
-
- if (def->data.udp.connectService &&
- def->data.udp.connectHost) {
- virBufferEscapeString(buf, "<source mode='connect'
host='%s' ",
- def->data.udp.connectHost);
- virBufferEscapeString(buf, "service='%s'/>\n",
- def->data.udp.connectService);
- } else if (def->data.udp.connectHost) {
- virBufferEscapeString(buf, "<source mode='connect'
host='%s'/>\n",
- def->data.udp.connectHost);
- } else if (def->data.udp.connectService) {
- virBufferEscapeString(buf, "<source mode='connect'
service='%s'/>\n",
- def->data.udp.connectService);
+ if (def->data.udp.bindService || def->data.udp.bindHost) {
+ virBufferAddLit(buf, "<source mode='bind'");
+ if (def->data.udp.bindService)
+ virBufferEscapeString(buf, " host='%s'",
def->data.udp.bindHost);
+ if (def->data.udp.bindService)
+ virBufferEscapeString(buf, " service='%s'",
def->data.udp.bindService);
+ virBufferAddLit(buf, "/>\n");
+ }
+
+ if (def->data.udp.connectService || def->data.udp.connectHost) {
+ virBufferAddLit(buf, "<source mode='connect'");
+ if (def->data.udp.connectService)
+ virBufferEscapeString(buf, " host='%s'",
def->data.udp.connectHost);
+ if (def->data.udp.connectService)
+ virBufferEscapeString(buf, " service='%s'",
def->data.udp.connectService);
+ virBufferAddLit(buf, "/>\n");
}
break;
--
2.10.0
8:37 a.m.
New subject: [libvirt] [PATCH 1/6] conf: Sanitize formatting of UDP chardev source
On 09/27/2016 12:39 PM, Peter Krempa wrote:
Use much simpler logic to determine parts of the code to print.
---
src/conf/domain_conf.c | 42 ++++++++++++++++--------------------------
1 file changed, 16 insertions(+), 26 deletions(-)
ACK
John
11:39 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemu: Add support for testing aguments on monitor verbatim
Add code that takes a string and matches it against the data passed as
arguments from qemu. This is a simpler version of
qemuMonitorTestAddItemParams.
---
tests/qemumonitortestutils.c | 112 +++++++++++++++++++++++++++++++++++++++++++
tests/qemumonitortestutils.h | 6 +++
2 files changed, 118 insertions(+)
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index c86a27a..50bf174 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -439,6 +439,7 @@ struct qemuMonitorTestHandlerData {
char *response;
size_t nargs;
qemuMonitorTestCommandArgsPtr args;
+ char *expectArgs;
};
static void
@@ -458,6 +459,7 @@ qemuMonitorTestHandlerDataFree(void *opaque)
VIR_FREE(data->command_name);
VIR_FREE(data->response);
VIR_FREE(data->args);
+ VIR_FREE(data->expectArgs);
VIR_FREE(data);
}
@@ -668,6 +670,7 @@ qemuMonitorTestProcessCommandWithArgs(qemuMonitorTestPtr test,
}
+
/* this allows to add a responder that is able to check
* a (shallow) structure of arguments for a command */
int
@@ -721,6 +724,115 @@ qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
}
+static int
+qemuMonitorTestProcessCommandWithArgStr(qemuMonitorTestPtr test,
+ qemuMonitorTestItemPtr item,
+ const char *cmdstr)
+{
+ struct qemuMonitorTestHandlerData *data = item->opaque;
+ virJSONValuePtr val = NULL;
+ virJSONValuePtr args;
+ char *argstr = NULL;
+ const char *cmdname;
+ int ret = -1;
+
+ if (!(val = virJSONValueFromString(cmdstr)))
+ return -1;
+
+ if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) {
+ ret = qemuMonitorReportError(test, "Missing command name in %s",
cmdstr);
+ goto cleanup;
+ }
+
+ if (STRNEQ(data->command_name, cmdname)) {
+ ret = qemuMonitorTestAddUnexpectedErrorResponse(test);
+ goto cleanup;
+ }
+
+ if (!(args = virJSONValueObjectGet(val, "arguments"))) {
+ ret = qemuMonitorReportError(test,
+ "Missing arguments section for command
'%s'",
+ data->command_name);
+ goto cleanup;
+ }
+
+ /* convert the arguments to string */
+ if (!(argstr = virJSONValueToString(args, false)))
+ goto cleanup;
+
+ /* verify that the argument value is expected */
+ if (STRNEQ(argstr, data->expectArgs)) {
+ ret = qemuMonitorReportError(test,
+ "%s: expected arguments: '%s', got:
'%s'",
+ data->command_name,
+ data->expectArgs, argstr);
+ goto cleanup;
+ }
+
+
+ VIR_FREE(argstr);
+
+ /* arguments checked out, return the response */
+ ret = qemuMonitorTestAddResponse(test, data->response);
+
+ cleanup:
+ VIR_FREE(argstr);
+ virJSONValueFree(val);
+ return ret;
+}
+
+
+/**
+ * qemuMonitorTestAddItemExpect:
+ *
+ * @test: test monitor object
+ * @cmdname: command name
+ * @cmdargs: expected arguments of the command
+ * @apostrophe: convert apostrophes (') in @cmdargs to quotes (")
+ * @response: simulated response of the command
+ *
+ * Simulates a qemu monitor command. Checks that the 'arguments' of the qmp
+ * command are expected. If @apostrophe is true apostrophes are converted to
+ * quotes for simplification of writing the strings into code.
+ */
+int
+qemuMonitorTestAddItemExpect(qemuMonitorTestPtr test,
+ const char *cmdname,
+ const char *cmdargs,
+ bool apostrophe,
+ const char *response)
+{
+ struct qemuMonitorTestHandlerData *data;
+
+ if (VIR_ALLOC(data) < 0)
+ goto error;
+
+ if (VIR_STRDUP(data->command_name, cmdname) < 0 ||
+ VIR_STRDUP(data->response, response) < 0 ||
+ VIR_STRDUP(data->expectArgs, cmdargs) < 0)
+ goto error;
+
+ if (apostrophe) {
+ char *tmp = data->expectArgs;
+
+ while (*tmp != '\0') {
+ if (*tmp == '\'')
+ *tmp = '"';
+
+ tmp++;
+ }
+ }
+
+ return qemuMonitorTestAddHandler(test,
+ qemuMonitorTestProcessCommandWithArgStr,
+ data, qemuMonitorTestHandlerDataFree);
+
+ error:
+ qemuMonitorTestHandlerDataFree(data);
+ return -1;
+}
+
+
static void
qemuMonitorTestEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virDomainObjPtr vm ATTRIBUTE_UNUSED,
diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h
index 8e2f371..3890cd4 100644
--- a/tests/qemumonitortestutils.h
+++ b/tests/qemumonitortestutils.h
@@ -60,6 +60,12 @@ int qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
...)
ATTRIBUTE_SENTINEL;
+int qemuMonitorTestAddItemExpect(qemuMonitorTestPtr test,
+ const char *cmdname,
+ const char *cmdargs,
+ bool apostrophe,
+ const char *response);
+
# define qemuMonitorTestNewSimple(json, xmlopt) \
qemuMonitorTestNew(json, xmlopt, NULL, NULL, NULL)
--
2.10.0
8:37 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemu: Add support for testing aguments on monitor verbatim
On 09/27/2016 12:39 PM, Peter Krempa wrote:
Add code that takes a string and matches it against the data passed
as
arguments from qemu. This is a simpler version of
qemuMonitorTestAddItemParams.
---
tests/qemumonitortestutils.c | 112 +++++++++++++++++++++++++++++++++++++++++++
tests/qemumonitortestutils.h | 6 +++
2 files changed, 118 insertions(+)
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index c86a27a..50bf174 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -439,6 +439,7 @@ struct qemuMonitorTestHandlerData {
char *response;
size_t nargs;
qemuMonitorTestCommandArgsPtr args;
+ char *expectArgs;
};
static void
@@ -458,6 +459,7 @@ qemuMonitorTestHandlerDataFree(void *opaque)
VIR_FREE(data->command_name);
VIR_FREE(data->response);
VIR_FREE(data->args);
+ VIR_FREE(data->expectArgs);
VIR_FREE(data);
}
@@ -668,6 +670,7 @@ qemuMonitorTestProcessCommandWithArgs(qemuMonitorTestPtr test,
}
+
Technically extra line, but I'll assume it's making possible for the 2
lines between functions...
/* this allows to add a responder that is able to check
* a (shallow) structure of arguments for a command */
int
@@ -721,6 +724,115 @@ qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
}
+static int
+qemuMonitorTestProcessCommandWithArgStr(qemuMonitorTestPtr test,
+ qemuMonitorTestItemPtr item,
+ const char *cmdstr)
+{
+ struct qemuMonitorTestHandlerData *data = item->opaque;
+ virJSONValuePtr val = NULL;
+ virJSONValuePtr args;
+ char *argstr = NULL;
+ const char *cmdname;
+ int ret = -1;
+
+ if (!(val = virJSONValueFromString(cmdstr)))
+ return -1;
+
+ if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) {
+ ret = qemuMonitorReportError(test, "Missing command name in %s",
cmdstr);
+ goto cleanup;
+ }
+
+ if (STRNEQ(data->command_name, cmdname)) {
+ ret = qemuMonitorTestAddUnexpectedErrorResponse(test);
+ goto cleanup;
+ }
+
+ if (!(args = virJSONValueObjectGet(val, "arguments"))) {
+ ret = qemuMonitorReportError(test,
+ "Missing arguments section for command
'%s'",
+ data->command_name);
+ goto cleanup;
+ }
+
+ /* convert the arguments to string */
+ if (!(argstr = virJSONValueToString(args, false)))
+ goto cleanup;
+
+ /* verify that the argument value is expected */
+ if (STRNEQ(argstr, data->expectArgs)) {
+ ret = qemuMonitorReportError(test,
+ "%s: expected arguments: '%s', got:
'%s'",
+ data->command_name,
+ data->expectArgs, argstr);
+ goto cleanup;
+ }
+
+
+ VIR_FREE(argstr);
This one's duplicitous since we fall through to cleanup
+
+ /* arguments checked out, return the response */
+ ret = qemuMonitorTestAddResponse(test, data->response);
+
+ cleanup:
+ VIR_FREE(argstr);
+ virJSONValueFree(val);
+ return ret;
+}
+
+
+/**
+ * qemuMonitorTestAddItemExpect:
+ *
+ * @test: test monitor object
+ * @cmdname: command name
+ * @cmdargs: expected arguments of the command
+ * @apostrophe: convert apostrophes (') in @cmdargs to quotes (")
+ * @response: simulated response of the command
+ *
+ * Simulates a qemu monitor command. Checks that the 'arguments' of the qmp
+ * command are expected. If @apostrophe is true apostrophes are converted to
+ * quotes for simplification of writing the strings into code.
+ */
+int
+qemuMonitorTestAddItemExpect(qemuMonitorTestPtr test,
+ const char *cmdname,
+ const char *cmdargs,
+ bool apostrophe,
+ const char *response)
+{
+ struct qemuMonitorTestHandlerData *data;
+
+ if (VIR_ALLOC(data) < 0)
+ goto error;
+
+ if (VIR_STRDUP(data->command_name, cmdname) < 0 ||
+ VIR_STRDUP(data->response, response) < 0 ||
+ VIR_STRDUP(data->expectArgs, cmdargs) < 0)
+ goto error;
+
+ if (apostrophe) {
+ char *tmp = data->expectArgs;
+
+ while (*tmp != '\0') {
+ if (*tmp == '\'')
+ *tmp = '"';
+
+ tmp++;
+ }
Would there ever be a time when apostrophe would be false?
ACK with at least the duplicitous free resolved. Your call on the above
loop.
John
+ }
+
+ return qemuMonitorTestAddHandler(test,
+ qemuMonitorTestProcessCommandWithArgStr,
+ data, qemuMonitorTestHandlerDataFree);
+
+ error:
+ qemuMonitorTestHandlerDataFree(data);
+ return -1;
+}
+
+
static void
qemuMonitorTestEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virDomainObjPtr vm ATTRIBUTE_UNUSED,
diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h
index 8e2f371..3890cd4 100644
--- a/tests/qemumonitortestutils.h
+++ b/tests/qemumonitortestutils.h
@@ -60,6 +60,12 @@ int qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
...)
ATTRIBUTE_SENTINEL;
+int qemuMonitorTestAddItemExpect(qemuMonitorTestPtr test,
+ const char *cmdname,
+ const char *cmdargs,
+ bool apostrophe,
+ const char *response);
+
# define qemuMonitorTestNewSimple(json, xmlopt) \
qemuMonitorTestNew(json, xmlopt, NULL, NULL, NULL)
10:17 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemu: Add support for testing aguments on monitor verbatim
On Wed, Oct 05, 2016 at 09:37:50 -0400, John Ferlan wrote:
On 09/27/2016 12:39 PM, Peter Krempa wrote:
> Add code that takes a string and matches it against the data passed as
> arguments from qemu. This is a simpler version of
> qemuMonitorTestAddItemParams.
> ---
> tests/qemumonitortestutils.c | 112 +++++++++++++++++++++++++++++++++++++++++++
> tests/qemumonitortestutils.h | 6 +++
> 2 files changed, 118 insertions(+)
[...]
> +/**
> + * qemuMonitorTestAddItemExpect:
> + *
> + * @test: test monitor object
> + * @cmdname: command name
> + * @cmdargs: expected arguments of the command
> + * @apostrophe: convert apostrophes (') in @cmdargs to quotes (")
> + * @response: simulated response of the command
> + *
> + * Simulates a qemu monitor command. Checks that the 'arguments' of the
qmp
> + * command are expected. If @apostrophe is true apostrophes are converted to
> + * quotes for simplification of writing the strings into code.
> + */
> +int
> +qemuMonitorTestAddItemExpect(qemuMonitorTestPtr test,
> + const char *cmdname,
> + const char *cmdargs,
> + bool apostrophe,
> + const char *response)
> +{
> + struct qemuMonitorTestHandlerData *data;
> +
> + if (VIR_ALLOC(data) < 0)
> + goto error;
> +
> + if (VIR_STRDUP(data->command_name, cmdname) < 0 ||
> + VIR_STRDUP(data->response, response) < 0 ||
> + VIR_STRDUP(data->expectArgs, cmdargs) < 0)
> + goto error;
> +
> + if (apostrophe) {
> + char *tmp = data->expectArgs;
> +
> + while (*tmp != '\0') {
> + if (*tmp == '\'')
> + *tmp = '"';
> +
> + tmp++;
> + }
Would there ever be a time when apostrophe would be false?
It certainly can be. If you construct a proper JSON string, then
basically everything is enclosed in quotes. But those are very hard to
do in C code, thus you can cut the corner by using apostrophes. This
won't work though if you e.g. want to send a string containing an
apostrophe, thus you will need to do it properly and set @apostrophe to
false.
11:39 a.m.
New subject: [libvirt] [PATCH 3/6] tests: qemumonitorjson: Don't do multiple tests in one virTestRun
The chardev attach test would do all the tests in one virTestRun
instance. If one sub-test failed then the test would report failure
improperly and the error would be hard to debug since the error pointer
was overwritten.
---
tests/qemumonitorjsontest.c | 147 ++++++++++++++++++++++++++++++--------------
1 file changed, 102 insertions(+), 45 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 4e7bb71..23877f8 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -705,87 +705,143 @@ testQemuMonitorJSONGetCommandLineOptionParameters(const void
*data)
return ret;
}
+
+struct qemuMonitorJSONTestAttachChardevData {
+ qemuMonitorTestPtr test;
+ virDomainChrSourceDefPtr chr;
+ const char *expectPty;
+ bool fail;
+};
+
static int
-testQemuMonitorJSONAttachChardev(const void *data)
+testQemuMonitorJSONAttachChardev(const void *opaque)
{
- virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
- qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
- virDomainChrSourceDef chr;
- int ret = 0;
+ const struct qemuMonitorJSONTestAttachChardevData *data = opaque;
+ int rc;
- if (!test)
+ if ((rc = qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(data->test),
+ "alias", data->chr)) < 0)
+ goto cleanup;
+
+ if (data->chr->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (STRNEQ_NULLABLE(data->expectPty, data->chr->data.file.path)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "expected PTY path: %s got: %s",
+ NULLSTR(data->expectPty),
+ NULLSTR(data->chr->data.file.path));
+ rc = -1;
+ }
+
+ VIR_FREE(data->chr->data.file.path);
+ }
+
+ cleanup:
+ if ((rc != 0) != data->fail)
return -1;
+ else
+ return 0;
+}
-#define DO_CHECK(chrID, reply, fail) \
- if (qemuMonitorTestAddItem(test, "chardev-add", reply) < 0) \
- goto cleanup; \
- if (qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(test), \
- chrID, &chr) < 0) \
- ret = fail ? ret : -1; \
- else \
- ret = fail ? -1 : ret; \
-#define CHECK(chrID, reply) \
- DO_CHECK(chrID, reply, false)
+static int
+qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
+ const char *label,
+ virDomainChrSourceDefPtr chr,
+ const char *reply,
+ const char *expectPty,
+ bool fail)
-#define CHECK_FAIL(chrID, reply) \
- DO_CHECK(chrID, reply, true)
+{
+ struct qemuMonitorJSONTestAttachChardevData data;
+ char *jsonreply = NULL;
+ char *fulllabel = NULL;
+ int ret = -1;
+
+ if (!reply)
+ reply = "";
+
+ if (virAsprintf(&jsonreply, "{\"return\": {%s}}", reply) <
0)
+ goto cleanup;
+
+ if (virAsprintf(&fulllabel, "qemuMonitorJSONTestAttachChardev(%s)",
label) < 0)
+ goto cleanup;
+
+ data.chr = chr;
+ data.fail = fail;
+ data.expectPty = expectPty;
+ if (!(data.test = qemuMonitorTestNewSimple(true, xmlopt)))
+ goto cleanup;
+
+ if (qemuMonitorTestAddItem(data.test, "chardev-add", jsonreply) < 0)
+ goto cleanup;
+
+ if (virTestRun(fulllabel, &testQemuMonitorJSONAttachChardev, &data) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ qemuMonitorTestFree(data.test);
+ VIR_FREE(jsonreply);
+ VIR_FREE(fulllabel);
+ return ret;
+}
+
+static int
+qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt)
+{
+ virDomainChrSourceDef chr;
+ int ret = 0;
+
+#define CHECK(label, fail) \
+ if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, &chr, NULL, NULL, \
+ fail) < 0) \
+ ret = -1
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_NULL };
- CHECK("chr_null", "{\"return\": {}}");
+ CHECK("null", false);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC };
- CHECK("chr_vc", "{\"return\": {}}");
+ CHECK("vc", false);
-#define PTY_PATH "/dev/ttyS0"
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
- CHECK("chr_pty", "{\"return\": {\"pty\" :
\"" PTY_PATH "\"}}");
- if (STRNEQ_NULLABLE(PTY_PATH, chr.data.file.path)) {
- VIR_FREE(chr.data.file.path);
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "expected PTY path: %s got: %s",
- PTY_PATH, NULLSTR(chr.data.file.path));
+ if (qemuMonitorJSONTestAttachOneChardev(xmlopt, "pty", &chr,
+ "\"pty\" :
\"/dev/pts/0\"",
+ "/dev/pts/0", false) < 0)
ret = -1;
- }
- VIR_FREE(chr.data.file.path);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
- CHECK_FAIL("chr_pty_fail", "{\"return\": {}}");
-#undef PTY_PATH
+ CHECK("pty missing path", true);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_FILE };
- CHECK("chr_file", "{\"return\": {}}");
+ CHECK("file", false);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_DEV };
- CHECK("chr_dev", "{\"return\": {}}");
+ CHECK("device", false);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_TCP };
- CHECK("chr_tcp", "{\"return\": {}}");
+ CHECK("tcp", false);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UDP };
- CHECK("chr_udp", "{\"return\": {}}");
+ CHECK("udp", false);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UNIX };
- CHECK("chr_unix", "{\"return\": {}}");
+ CHECK("unix", false);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_SPICEVMC };
- CHECK("chr_spicevmc", "{\"return\": {}}");
+ CHECK("spicevmc", false);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PIPE };
- CHECK_FAIL("chr_pipe", "{\"return\": {}}");
+ CHECK("pipe", true);
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_STDIO };
- CHECK_FAIL("chr_stdio", "{\"return\": {}}");
-
+ CHECK("stdio", true);
#undef CHECK
-#undef CHECK_FAIL
-#undef DO_CHECK
- cleanup:
- qemuMonitorTestFree(test);
return ret;
}
+
static int
testQemuMonitorJSONDetachChardev(const void *data)
{
@@ -2520,7 +2576,8 @@ mymain(void)
DO_TEST(GetCommands);
DO_TEST(GetTPMModels);
DO_TEST(GetCommandLineOptionParameters);
- DO_TEST(AttachChardev);
+ if (qemuMonitorJSONTestAttachChardev(driver.xmlopt) < 0)
+ ret = -1;
DO_TEST(DetachChardev);
DO_TEST(GetListPaths);
DO_TEST(GetObjectProperty);
--
2.10.0
8:38 a.m.
New subject: [libvirt] [PATCH 3/6] tests: qemumonitorjson: Don't do multiple tests in one virTestRun
On 09/27/2016 12:39 PM, Peter Krempa wrote:
The chardev attach test would do all the tests in one virTestRun
instance. If one sub-test failed then the test would report failure
improperly and the error would be hard to debug since the error pointer
was overwritten.
---
tests/qemumonitorjsontest.c | 147 ++++++++++++++++++++++++++++++--------------
1 file changed, 102 insertions(+), 45 deletions(-)
ACK - seems reasonable (not a pleasant diff to read)
John
11:39 a.m.
New subject: [libvirt] [PATCH 4/6] tests: qemumonitorjsontest: Do some actual testing in qemuMonitorJSONTestAttachChardev
Until now the test was rather useless since it didn't check the
arguments formatted and didn't use properly configured chardev objects.
Add the expected arguments and instrument the test to validate them.
Modify some test cases to actually add valid data.
Note that the UDP test data is currently wrong due to a bug.
---
tests/qemumonitorjsontest.c | 93 +++++++++++++++++++++++++++++++++------------
1 file changed, 68 insertions(+), 25 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 23877f8..61344b7 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -747,6 +747,7 @@ static int
qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
const char *label,
virDomainChrSourceDefPtr chr,
+ const char *expectargs,
const char *reply,
const char *expectPty,
bool fail)
@@ -772,7 +773,8 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
if (!(data.test = qemuMonitorTestNewSimple(true, xmlopt)))
goto cleanup;
- if (qemuMonitorTestAddItem(data.test, "chardev-add", jsonreply) < 0)
+ if (qemuMonitorTestAddItemExpect(data.test, "chardev-add",
+ expectargs, true, jsonreply) < 0)
goto cleanup;
if (virTestRun(fulllabel, &testQemuMonitorJSONAttachChardev, &data) < 0)
@@ -793,49 +795,90 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt)
virDomainChrSourceDef chr;
int ret = 0;
-#define CHECK(label, fail) \
- if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, &chr, NULL, NULL, \
- fail) < 0) \
+#define CHECK(label, fail, expectargs) \
+ if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, &chr, expectargs, \
+ NULL, NULL, fail) < 0) \
ret = -1
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_NULL };
- CHECK("null", false);
+ CHECK("null", false,
+
"{'id':'alias','backend':{'type':'null','data':{}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC };
- CHECK("vc", false);
+ CHECK("vc", false,
+
"{'id':'alias','backend':{'type':'null','data':{}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
if (qemuMonitorJSONTestAttachOneChardev(xmlopt, "pty", &chr,
+ "{'id':'alias',"
+
"'backend':{'type':'pty',"
+ "'data':{}}}",
"\"pty\" :
\"/dev/pts/0\"",
"/dev/pts/0", false) < 0)
ret = -1;
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
- CHECK("pty missing path", true);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_FILE };
- CHECK("file", false);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_DEV };
- CHECK("device", false);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_TCP };
- CHECK("tcp", false);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UDP };
- CHECK("udp", false);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UNIX };
- CHECK("unix", false);
+ CHECK("pty missing path", true,
+
"{'id':'alias','backend':{'type':'pty','data':{}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_FILE;
+ chr.data.file.path = (char *) "/test/path";
+ CHECK("file", false,
+
"{'id':'alias','backend':{'type':'file','data':{'out':'/test/path'}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_DEV;
+ chr.data.file.path = (char *) "/test/path";
+ CHECK("device", false,
+
"{'id':'alias','backend':{'type':'serial','data':{'device':'/test/path'}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_TCP;
+ chr.data.tcp.host = (char *) "example.com";
+ chr.data.tcp.service = (char *) "1234";
+ CHECK("tcp", false,
+ "{'id':'alias',"
+ "'backend':{'type':'socket',"
+
"'data':{'addr':{'type':'inet',"
+
"'data':{'host':'example.com',"
+
"'port':'1234'}},"
+ "'wait':false,"
+ "'telnet':false,"
+ "'server':false}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_UDP;
+ chr.data.udp.connectHost = (char *) "example.com";
+ chr.data.udp.connectService = (char *) "1234";
+ CHECK("udp", false,
+ "{'id':'alias',"
+ "'backend':{'type':'socket',"
+
"'data':{'addr':{'type':'inet',"
+
"'data':{'host':'example.com',"
+
"'port':'1234'}}}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+ chr.data.nix.path = (char *) "/path/to/socket";
+ CHECK("unix", false,
+ "{'id':'alias',"
+ "'backend':{'type':'socket',"
+
"'data':{'addr':{'type':'unix',"
+
"'data':{'path':'/path/to/socket'}},"
+ "'wait':false,"
+ "'server':false}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_SPICEVMC };
- CHECK("spicevmc", false);
+ CHECK("spicevmc", false,
+
"{'id':'alias','backend':{'type':'spicevmc','"
+
"data':{'type':'vdagent'}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PIPE };
- CHECK("pipe", true);
+ CHECK("pipe", true, "");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_STDIO };
- CHECK("stdio", true);
+ CHECK("stdio", true, "");
#undef CHECK
return ret;
--
2.10.0
8:38 a.m.
New subject: [libvirt] [PATCH 4/6] tests: qemumonitorjsontest: Do some actual testing in qemuMonitorJSONTestAttachChardev
On 09/27/2016 12:39 PM, Peter Krempa wrote:
Until now the test was rather useless since it didn't check the
arguments formatted and didn't use properly configured chardev objects.
Add the expected arguments and instrument the test to validate them.
Modify some test cases to actually add valid data.
Note that the UDP test data is currently wrong due to a bug.
---
tests/qemumonitorjsontest.c | 93 +++++++++++++++++++++++++++++++++------------
1 file changed, 68 insertions(+), 25 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 23877f8..61344b7 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -747,6 +747,7 @@ static int
qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
const char *label,
virDomainChrSourceDefPtr chr,
+ const char *expectargs,
const char *reply,
const char *expectPty,
bool fail)
@@ -772,7 +773,8 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
if (!(data.test = qemuMonitorTestNewSimple(true, xmlopt)))
goto cleanup;
- if (qemuMonitorTestAddItem(data.test, "chardev-add", jsonreply) < 0)
+ if (qemuMonitorTestAddItemExpect(data.test, "chardev-add",
+ expectargs, true, jsonreply) < 0)
FWIW: The "knowledge" of whether the expectargs has an apostrophe would
seem to lie in the caller. If in fact you still keep the apostrophe
check in patch 2, then I would think the caller would be able to supply
true/false, but that's a minor thing.
Also, something that just occurred to me... 'expectargs' had better not
be NULL; otherwise, patch 2 will core at the while (*tmp != '\0')
ACK in principle - your call on the apostrophe thing - although perhaps
a check in patch 2 should be added for (apostrophe && data->expectArgs)
to protect from future mishaps...
John
goto cleanup;
if (virTestRun(fulllabel, &testQemuMonitorJSONAttachChardev, &data) < 0)
@@ -793,49 +795,90 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt)
virDomainChrSourceDef chr;
int ret = 0;
-#define CHECK(label, fail) \
- if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, &chr, NULL, NULL, \
- fail) < 0) \
+#define CHECK(label, fail, expectargs) \
+ if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, &chr, expectargs, \
+ NULL, NULL, fail) < 0) \
ret = -1
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_NULL };
- CHECK("null", false);
+ CHECK("null", false,
+
"{'id':'alias','backend':{'type':'null','data':{}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC };
- CHECK("vc", false);
+ CHECK("vc", false,
+
"{'id':'alias','backend':{'type':'null','data':{}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
if (qemuMonitorJSONTestAttachOneChardev(xmlopt, "pty", &chr,
+ "{'id':'alias',"
+
"'backend':{'type':'pty',"
+
"'data':{}}}",
"\"pty\" :
\"/dev/pts/0\"",
"/dev/pts/0", false) < 0)
ret = -1;
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
- CHECK("pty missing path", true);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_FILE };
- CHECK("file", false);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_DEV };
- CHECK("device", false);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_TCP };
- CHECK("tcp", false);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UDP };
- CHECK("udp", false);
-
- chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UNIX };
- CHECK("unix", false);
+ CHECK("pty missing path", true,
+
"{'id':'alias','backend':{'type':'pty','data':{}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_FILE;
+ chr.data.file.path = (char *) "/test/path";
+ CHECK("file", false,
+
"{'id':'alias','backend':{'type':'file','data':{'out':'/test/path'}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_DEV;
+ chr.data.file.path = (char *) "/test/path";
+ CHECK("device", false,
+
"{'id':'alias','backend':{'type':'serial','data':{'device':'/test/path'}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_TCP;
+ chr.data.tcp.host = (char *) "example.com";
+ chr.data.tcp.service = (char *) "1234";
+ CHECK("tcp", false,
+ "{'id':'alias',"
+ "'backend':{'type':'socket',"
+
"'data':{'addr':{'type':'inet',"
+
"'data':{'host':'example.com',"
+
"'port':'1234'}},"
+ "'wait':false,"
+ "'telnet':false,"
+ "'server':false}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_UDP;
+ chr.data.udp.connectHost = (char *) "example.com";
+ chr.data.udp.connectService = (char *) "1234";
+ CHECK("udp", false,
+ "{'id':'alias',"
+ "'backend':{'type':'socket',"
+
"'data':{'addr':{'type':'inet',"
+
"'data':{'host':'example.com',"
+
"'port':'1234'}}}}}");
+
+ memset(&chr, 0, sizeof(chr));
+ chr.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+ chr.data.nix.path = (char *) "/path/to/socket";
+ CHECK("unix", false,
+ "{'id':'alias',"
+ "'backend':{'type':'socket',"
+
"'data':{'addr':{'type':'unix',"
+
"'data':{'path':'/path/to/socket'}},"
+ "'wait':false,"
+ "'server':false}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_SPICEVMC };
- CHECK("spicevmc", false);
+ CHECK("spicevmc", false,
+
"{'id':'alias','backend':{'type':'spicevmc','"
+
"data':{'type':'vdagent'}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PIPE };
- CHECK("pipe", true);
+ CHECK("pipe", true, "");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_STDIO };
- CHECK("stdio", true);
+ CHECK("stdio", true, "");
#undef CHECK
return ret;
2:11 a.m.
New subject: [libvirt] [PATCH 4/6] tests: qemumonitorjsontest: Do some actual testing in qemuMonitorJSONTestAttachChardev
On Wed, Oct 05, 2016 at 09:38:16 -0400, John Ferlan wrote:
On 09/27/2016 12:39 PM, Peter Krempa wrote:
> Until now the test was rather useless since it didn't check the
> arguments formatted and didn't use properly configured chardev objects.
>
> Add the expected arguments and instrument the test to validate them.
> Modify some test cases to actually add valid data.
>
> Note that the UDP test data is currently wrong due to a bug.
> ---
> tests/qemumonitorjsontest.c | 93 +++++++++++++++++++++++++++++++++------------
> 1 file changed, 68 insertions(+), 25 deletions(-)
>
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index 23877f8..61344b7 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -747,6 +747,7 @@ static int
> qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
> const char *label,
> virDomainChrSourceDefPtr chr,
> + const char *expectargs,
> const char *reply,
> const char *expectPty,
> bool fail)
> @@ -772,7 +773,8 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr
xmlopt,
> if (!(data.test = qemuMonitorTestNewSimple(true, xmlopt)))
> goto cleanup;
>
> - if (qemuMonitorTestAddItem(data.test, "chardev-add", jsonreply) <
0)
> + if (qemuMonitorTestAddItemExpect(data.test, "chardev-add",
> + expectargs, true, jsonreply) < 0)
FWIW: The "knowledge" of whether the expectargs has an apostrophe would
seem to lie in the caller. If in fact you still keep the apostrophe
That acutally means that the string uses apostrophes instead of quotes
for easier embedding into C.
In this exact case, all the strings tested use apostrophes instead of
quotes so this will always be true. I don't see a benefit of adding it
as an argument for every single test case in this test.
check in patch 2, then I would think the caller would be able to
supply
true/false, but that's a minor thing.
This code is always using the shortcut, since I'm lazy to escape every
single quote.
Also, something that just occurred to me... 'expectargs' had
better not
be NULL; otherwise, patch 2 will core at the while (*tmp != '\0')
There are other monitor testing APIs that you can use if you don't care
about the arguments.
Additionally the worker function that is verifying the data is not able
to expect a NULL string as these APIs are made to test the actual
arguments. For commands which don't have arguments you'll get an earlier
error. The expected string should thus never be NULL, since it would be
a programmer mistake.
And if it will be by accident, the testsuite will crash which is correct
in this regard IMO.
ACK in principle - your call on the apostrophe thing - although
perhaps
a check in patch 2 should be added for (apostrophe && data->expectArgs)
to protect from future mishaps...
John
11:39 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: monitor: Simplify construction of chardev backends
---
src/qemu/qemu_monitor_json.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e1494df..126927e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5937,22 +5937,17 @@ qemuMonitorJSONBuildInetSocketAddress(const char *host,
virJSONValuePtr addr = NULL;
virJSONValuePtr data = NULL;
- if (!(data = virJSONValueNewObject()) ||
- !(addr = virJSONValueNewObject()))
- goto error;
+ if (virJSONValueObjectCreate(&data, "s:host", host,
+ "s:port", port, NULL) < 0)
+ return NULL;
- /* port is really expected as a string here by qemu */
- if (virJSONValueObjectAppendString(data, "host", host) < 0 ||
- virJSONValueObjectAppendString(data, "port", port) < 0 ||
- virJSONValueObjectAppendString(addr, "type", "inet") < 0
||
- virJSONValueObjectAppend(addr, "data", data) < 0)
- goto error;
+ if (virJSONValueObjectCreate(&addr, "s:type", "inet",
+ "a:data", data, NULL) < 0) {
+ virJSONValueFree(data);
+ return NULL;
+ }
return addr;
- error:
- virJSONValueFree(data);
- virJSONValueFree(addr);
- return NULL;
}
static virJSONValuePtr
@@ -5961,20 +5956,16 @@ qemuMonitorJSONBuildUnixSocketAddress(const char *path)
virJSONValuePtr addr = NULL;
virJSONValuePtr data = NULL;
- if (!(data = virJSONValueNewObject()) ||
- !(addr = virJSONValueNewObject()))
- goto error;
+ if (virJSONValueObjectCreate(&data, "s:path", path, NULL) < 0)
+ return NULL;
- if (virJSONValueObjectAppendString(data, "path", path) < 0 ||
- virJSONValueObjectAppendString(addr, "type", "unix") < 0
||
- virJSONValueObjectAppend(addr, "data", data) < 0)
- goto error;
+ if (virJSONValueObjectCreate(&addr, "s:type", "unix",
+ "a:data", data, NULL) < 0) {
+ virJSONValueFree(data);
+ return NULL;
+ }
return addr;
- error:
- virJSONValueFree(data);
- virJSONValueFree(addr);
- return NULL;
}
int
--
2.10.0
11:39 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: monitor: Properly configure backend for UDP chardevs
Since introduction of chardev hotplug the code was wrong for the UDP
case and basically created a TCP socket instead. Use proper objects and
type for UDP.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1377602
---
src/qemu/qemu_monitor_json.c | 12 ++++++++++--
tests/qemumonitorjsontest.c | 20 ++++++++++++++++----
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 126927e..c720376 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6212,12 +6212,20 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
break;
case VIR_DOMAIN_CHR_TYPE_UDP:
- backend_type = "socket";
+ backend_type = "udp";
addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.udp.connectHost,
chr->data.udp.connectService);
if (!addr ||
- virJSONValueObjectAppend(data, "addr", addr) < 0)
+ virJSONValueObjectAppend(data, "remote", addr) < 0)
goto error;
+
+ if (chr->data.udp.bindHost) {
+ addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.udp.bindHost,
+ chr->data.udp.bindService);
+ if (!addr ||
+ virJSONValueObjectAppend(data, "local", addr) < 0)
+ goto error;
+ }
addr = NULL;
break;
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 61344b7..0574f8c 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -853,10 +853,22 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt)
chr.data.udp.connectService = (char *) "1234";
CHECK("udp", false,
"{'id':'alias',"
- "'backend':{'type':'socket',"
-
"'data':{'addr':{'type':'inet',"
-
"'data':{'host':'example.com',"
-
"'port':'1234'}}}}}");
+ "'backend':{'type':'udp',"
+
"'data':{'remote':{'type':'inet',"
+
"'data':{'host':'example.com',"
+
"'port':'1234'}}}}}");
+
+ chr.data.udp.bindHost = (char *) "localhost";
+ chr.data.udp.bindService = (char *) "4321";
+ CHECK("udp", false,
+ "{'id':'alias',"
+ "'backend':{'type':'udp',"
+
"'data':{'remote':{'type':'inet',"
+
"'data':{'host':'example.com',"
+
"'port':'1234'}},"
+
"'local':{'type':'inet',"
+
"'data':{'host':'localhost',"
+
"'port':'4321'}}}}}");
memset(&chr, 0, sizeof(chr));
chr.type = VIR_DOMAIN_CHR_TYPE_UNIX;
--
2.10.0
8:39 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: monitor: Properly configure backend for UDP chardevs
On 09/27/2016 12:39 PM, Peter Krempa wrote:
Since introduction of chardev hotplug the code was wrong for the UDP
case and basically created a TCP socket instead. Use proper objects and
type for UDP.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1377602
---
src/qemu/qemu_monitor_json.c | 12 ++++++++++--
tests/qemumonitorjsontest.c | 20 ++++++++++++++++----
2 files changed, 26 insertions(+), 6 deletions(-)
As I read the bug, the code below, and the qemu_command code - it seems
the bug describes issues with both qemu_command and the hotplug code.
Although for sure the hotplug code is wrong - there's no adjustment in
the qemu_command code, so it doesn't seem this patch addresses all the
concerns in the bz.
ACK to the change, but I'm not sure you can claim victory over the bz if
the qemu_command logic is still flawed...
John
9:59 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: monitor: Properly configure backend for UDP chardevs
On Wed, Oct 05, 2016 at 09:39:06 -0400, John Ferlan wrote:
On 09/27/2016 12:39 PM, Peter Krempa wrote:
> Since introduction of chardev hotplug the code was wrong for the UDP
> case and basically created a TCP socket instead. Use proper objects and
> type for UDP.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1377602
> ---
> src/qemu/qemu_monitor_json.c | 12 ++++++++++--
> tests/qemumonitorjsontest.c | 20 ++++++++++++++++----
> 2 files changed, 26 insertions(+), 6 deletions(-)
>
As I read the bug, the code below, and the qemu_command code - it seems
the bug describes issues with both qemu_command and the hotplug code.
The command line portion actually is not wrong. It generates the
address pair with type UDP as it should. The reporter of that bugzilla
has just improper expectations of UDP socket capabilities.
Also it's very unlikely that somebody is actually using the qemu UDP
backend. It was broken for rather long time up until last year.
Peter
2969
days inactive
2978
days old
15 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa