On Mon, Feb 03, 2014 at 05:41:00PM +0100, Martin Kletzander wrote:
signed-off-by: martin kletzander <mkletzan(a)redhat.com
---
notes:
this applies on top of "qemu: minor cleanups":
https://www.redhat.com/archives/libvir-list/2014-january/msg01584.html
docs/formatdomain.html.in | 22 +++++
docs/schemas/domaincommon.rng | 4 +
src/conf/domain_audit.c | 3 +-
src/conf/domain_conf.c | 40 ++++++++-
src/conf/domain_conf.h | 6 +-
src/qemu/qemu_capabilities.c | 8 ++
src/qemu/qemu_capabilities.h | 3 +-
src/qemu/qemu_command.c | 96 +++++++++++++---------
src/qemu/qemu_monitor_json.c | 3 +-
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
.../qemuxml2argv-serial-spiceport-nospice.args | 6 ++
.../qemuxml2argv-serial-spiceport-nospice.xml | 40 +++++++++
.../qemuxml2argv-serial-spiceport.args | 13 +++
.../qemuxml2argv-serial-spiceport.xml | 43 ++++++++++
tests/qemuxml2argvtest.c | 7 ++
tests/qemuxml2xmltest.c | 2 +
18 files changed, 255 insertions(+), 44 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fa1ecb5..8cdd0e9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -437,7 +437,8 @@ VIR_ENUM_IMPL(virDomainChr, VIR_DOMAIN_CHR_TYPE_LAST,
"udp",
"tcp",
"unix",
- "spicevmc")
+ "spicevmc",
+ "spiceport")
VIR_ENUM_IMPL(virDomainChrTcpProtocol, VIR_DOMAIN_CHR_TCP_PROTOCOL_LAST,
"raw",
@@ -1583,6 +1584,12 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
STREQ_NULLABLE(src->data.nix.path, tgt->data.nix.path);
break;
+ case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+ return STREQ_NULLABLE(src->data.spiceport.channel,
+ tgt->data.spiceport.channel);
+ return true;
this 'return true' is not needed
+ break;
+
case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_STDIO:
@@ -7090,6 +7097,9 @@ error:
return ret;
}
+#define SERIAL_CHANNEL_NAME_CHARS \
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-."
+
/* Parse the source half of the XML definition for a character device,
* where node is the first element of node->children of the parent
* element. def->type must already be valid. Return -1 on failure,
@@ -7110,6 +7120,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
char *path = NULL;
char *mode = NULL;
char *protocol = NULL;
+ char *channel = NULL;
int remaining = 0;
while (cur != NULL) {
@@ -7154,6 +7165,11 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
VIR_FREE(mode);
break;
+ case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+ if (!channel)
+ channel = virXMLPropString(cur, "channel");
+ break;
+
case VIR_DOMAIN_CHR_TYPE_LAST:
case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
@@ -7293,6 +7309,21 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
def->data.nix.path = path;
path = NULL;
break;
+
+ case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+ if (!channel) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Missing source channel attribute for char
device"));
+ goto error;
+ }
Code before that uses VIR_ERR_INTERNAL_ERROR for missing attributes, but
VIR_ERR_XML_ERROR seems indeed better.
+ if (strcspn(channel, SERIAL_CHANNEL_NAME_CHARS)) {
If I understood the man page correctly, strcspn will return the number of
characters at the beginning of channel which are not in
SERIAL_CHANNEL_NAME_CHARS. It seems this won't catch "org.éâò".
The net code does:
if (strspn(channel, SERIAL_CHANNEL_NAME_CHARS) < strlen(channel)) {
which seems ok.
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid character in source channel for char
device"));
The network code uses VIR_ERR_INVALID_ARG here.
+ goto error;
+ }
+ def->data.spiceport.channel = channel;
+ channel = NULL;
+ break;
}
cleanup:
@@ -7303,6 +7334,7 @@ cleanup:
VIR_FREE(connectHost);
VIR_FREE(connectService);
VIR_FREE(path);
+ VIR_FREE(channel);
return remaining;
@@ -15651,6 +15683,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
virBufferEscapeString(buf, " path='%s'",
def->data.nix.path);
virBufferAddLit(buf, "/>\n");
break;
+
+ case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+ virBufferAsprintf(buf, "<source channel='%s'/>\n",
+ def->data.spiceport.channel);
+ break;
+
}
virBufferAdjustIndent(buf, -6);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d8f2e49..b07aa8f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1,7 +1,7 @@
/*
* domain_conf.h: domain XML processing
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -1104,6 +1104,7 @@ enum virDomainChrType {
VIR_DOMAIN_CHR_TYPE_TCP,
VIR_DOMAIN_CHR_TYPE_UNIX,
VIR_DOMAIN_CHR_TYPE_SPICEVMC,
+ VIR_DOMAIN_CHR_TYPE_SPICEPORT,
VIR_DOMAIN_CHR_TYPE_LAST
};
@@ -1152,6 +1153,9 @@ struct _virDomainChrSourceDef {
bool listen;
} nix;
int spicevmc;
+ struct {
+ char *channel;
+ } spiceport;
} data;
};
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8aec293..317b374 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -248,6 +248,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"pvpanic",
"enable-fips",
"spice-file-xfer-disable",
+ "spiceport",
);
struct _virQEMUCaps {
@@ -1012,6 +1013,8 @@ virQEMUCapsComputeCmdFlags(const char *help,
virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV);
if (strstr(help, "-chardev spicevmc"))
virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEVMC);
+ if (strstr(help, "-chardev spiceport"))
+ virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT);
}
if (strstr(help, "-balloon"))
virQEMUCapsSet(qemuCaps, QEMU_CAPS_BALLOON);
@@ -2570,6 +2573,11 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
if (qemuCaps->version >= 1006000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+ /* -chardev spiceport is supported from 1.4.0,
+ * but it's in qapi only since 1.5.0 */
+ if (qemuCaps->version >= 1005000)
+ virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT);
+
I'd tend to put this before
if (qemuCaps->version >= 1006000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
so that the qemu version tests are sorted by version.
if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
goto cleanup;
if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 23dccce..a4eecb6 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -1,7 +1,7 @@
/*
* qemu_capabilities.h: QEMU capabilities generation
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -202,6 +202,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_PANIC = 161, /* -device pvpanic */
QEMU_CAPS_ENABLE_FIPS = 162, /* -enable-fips */
QEMU_CAPS_SPICE_FILE_XFER_DISABLE = 163, /* -spice disable-agent-file-xfer */
+ QEMU_CAPS_CHARDEV_SPICEPORT = 164, /* -chardev spiceport */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7e1cd53..c1635e0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5977,6 +5977,16 @@ qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char
*alias,
virDomainChrSpicevmcTypeToString(dev->data.spicevmc));
break;
+ case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("spiceport not supported in this QEMU binary"));
+ goto error;
+ }
+ virBufferAsprintf(&buf, "spiceport,id=char%s,name=%s", alias,
+ dev->data.spiceport.channel);
+ break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported chardev '%s'"),
@@ -6075,6 +6085,8 @@ qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const char
*prefix)
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
/* spicevmc doesn't have any '-serial' compatible option */
+ case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+ /* spiceport doesn't have any '-serial' compatible option */
As you suggested when I raised this for TYPE_SPICEVMC, you can remove the
confusing comment for TYPE_SPICE_PORT too.
Rest of the patch looks good.
Christophe