[libvirt] [PATCH 0/4] support for QEMU vhost-user
This series adds support for the QEMU vhost-user feature to libvirt. vhost-user enables the communication between a QEMU virtual machine and other userspace process using the Virtio transport protocol. It uses a char dev (e.g. Unix socket) for the control plane, while the data plane based on shared memory. The XML looks like: <interface type='vhostuser'> <source type='unix' path='/tmp/vhost.sock' mode='server'/> <mac address='52:54:00:3b:83:1a'/> <model type='virtio'/> </interface> Our use case is the deployment of Snabbswitch in an OpenStack/NFV environment. Snabbswitch uses a Unix socket to implement the vhost-user control plane, thus we focused on the support for the type=unix attribute of the <source> element. To test it with Snabbswitch, it is necessary to apply the following patches (respectively from Chen Fan and Michele Paolino): http://www.redhat.com/archives/libvir-list/2014-June/msg01195.html http://www.redhat.com/archives/libvir-list/2014-June/msg01418.html It is also possible to directly checkout the Virtual Open Systems' libvirt repository(branch "vhost-user_support) at the address: https://github.com/virtualopensystems/libvirt. This patch is based on the previous work from Luke Gorrie: http://www.redhat.com/archives/libvir-list/2014-May/msg00934.html Michele Paolino (4): vhost-user support: domain configuration vhost-user support: qemu command-line vhost-user support: tests and docs vhost-user support: lxc,xenxs,uml docs/formatdomain.html.in | 34 +++++++++ docs/schemas/domaincommon.rng | 39 +++++++++++ src/conf/domain_conf.c | 81 ++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++- src/lxc/lxc_process.c | 1 + src/qemu/qemu_command.c | 58 ++++++++++++++++ src/uml/uml_conf.c | 5 ++ src/xenxs/xen_sxpr.c | 1 + .../qemuxml2argv-net-vhostuser.args | 7 ++ .../qemuxml2argv-net-vhostuser.xml | 33 +++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 12 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml -- 1.9.3
vhost-user is added as a virDomainChrSourceDefPtr variable in the virtual network interface configuration structure. This patch adds support to parse vhost-user element. The XML file looks like: <interface type='vhostuser'> <source type='unix' path='/tmp/vhost.sock' mode='server'/> <mac address='52:54:00:3b:83:1a'/> <model type='virtio'/> </interface> Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- src/conf/domain_conf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++-- 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ea09bdc..de52ca4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "user", "ethernet", + "vhostuser", "server", "client", "mcast", @@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def->data.ethernet.ipaddr); break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + virDomainChrSourceDefFree(def->data.vhostuser); + break; + case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -6665,6 +6670,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *mode = NULL; char *linkstate = NULL; char *addrtype = NULL; + char *vhostuser_mode = NULL; + char *vhostuser_path = NULL; + char *vhostuser_type = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; @@ -6710,6 +6718,21 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + } else if (!vhostuser_path && !vhostuser_mode && !vhostuser_type + && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + xmlStrEqual(cur->name, BAD_CAST "source")) { + vhostuser_type = virXMLPropString(cur, "type"); + if (!vhostuser_type || STREQ(vhostuser_type, "unix")) { + vhostuser_path = virXMLPropString(cur, "path"); + vhostuser_mode = virXMLPropString(cur, "mode"); + } + else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("type='%s' unsupported for" + " <interface type='vhostuser'>"), + vhostuser_type); + goto error; + } } else if (!def->virtPortProfile && xmlStrEqual(cur->name, BAD_CAST "virtualport")) { if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -6867,6 +6890,50 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, actual = NULL; break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if (!model || STRNEQ(model, "virtio")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Wrong or no <model> 'type' attribute " + "specified with <interface type='vhostuser'/>. " + "vhostuser requires the virtio-net* frontend")); + goto error; + } + + if (VIR_ALLOC(def->data.vhostuser) < 0) + goto error; + + if (STREQ(vhostuser_type, "unix")) { + + (def->data.vhostuser)->type = VIR_DOMAIN_CHR_TYPE_UNIX; + + if (vhostuser_path == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'path' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + (def->data.vhostuser)->data.nix.path = vhostuser_path; + + if (vhostuser_mode == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'mode' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + if (STREQ(vhostuser_mode, "server")) + (def->data.vhostuser)->data.nix.listen = true; + + } + + vhostuser_type = NULL; + vhostuser_path = NULL; + vhostuser_mode = NULL; + break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: if (dev != NULL) { def->data.ethernet.dev = dev; @@ -7112,6 +7179,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(portgroup); VIR_FREE(address); VIR_FREE(port); + VIR_FREE(vhostuser_type); + VIR_FREE(vhostuser_path); + VIR_FREE(vhostuser_mode); VIR_FREE(ifname); VIR_FREE(dev); virDomainActualNetDefFree(actual); @@ -15989,6 +16059,17 @@ virDomainNetDefFormat(virBufferPtr buf, def->data.ethernet.ipaddr); break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if ((def->data.vhostuser)->type == VIR_DOMAIN_CHR_TYPE_UNIX) { + virBufferAddLit(buf, "<source type='unix'"); + virBufferEscapeString(buf, " path='%s'", + (def->data.vhostuser)->data.nix.path); + if ((def->data.vhostuser)->data.nix.listen) + virBufferAddLit(buf, " mode='server'"); + virBufferAddLit(buf, "/>\n"); + } + break; + case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferEscapeString(buf, "<source bridge='%s'/>\n", def->data.bridge.brname); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eb5bad7..91f7524 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -133,6 +133,12 @@ typedef virDomainIdMapDef *virDomainIdMapDefPtr; typedef struct _virDomainPanicDef virDomainPanicDef; typedef virDomainPanicDef *virDomainPanicDefPtr; +/* forward declarations virDomainChrSourceDef, required by + * virDomainNetDef + */ +typedef struct _virDomainChrSourceDef virDomainChrSourceDef; +typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -795,6 +801,7 @@ struct _virDomainFSDef { typedef enum { VIR_DOMAIN_NET_TYPE_USER, VIR_DOMAIN_NET_TYPE_ETHERNET, + VIR_DOMAIN_NET_TYPE_VHOSTUSER, VIR_DOMAIN_NET_TYPE_SERVER, VIR_DOMAIN_NET_TYPE_CLIENT, VIR_DOMAIN_NET_TYPE_MCAST, @@ -880,6 +887,7 @@ struct _virDomainNetDef { char *dev; char *ipaddr; } ethernet; + virDomainChrSourceDefPtr vhostuser; struct { char *address; int port; @@ -1006,8 +1014,6 @@ typedef enum { } virDomainChrSpicevmcName; /* The host side information for a character device. */ -typedef struct _virDomainChrSourceDef virDomainChrSourceDef; -typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; struct _virDomainChrSourceDef { int type; /* virDomainChrType */ union { -- 1.9.3
On 02.07.2014 15:20, Michele Paolino wrote:
vhost-user is added as a virDomainChrSourceDefPtr variable in the virtual network interface configuration structure. This patch adds support to parse vhost-user element. The XML file looks like:
<interface type='vhostuser'> <source type='unix' path='/tmp/vhost.sock' mode='server'/> <mac address='52:54:00:3b:83:1a'/> <model type='virtio'/> </interface>
Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- src/conf/domain_conf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++-- 2 files changed, 89 insertions(+), 2 deletions(-)
Introducing a new device (subtype of a device) must always go hand in hand with documentation and XML schema adjustment. Moreover, it would be nice to test the new XML (e.g. domainschematest or qemuxml2argv stub (a .xml file under tests/qemuxml2argvdata without .args counterpart which will be introduced once qemu implementation is done)).
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ea09bdc..de52ca4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "user", "ethernet", + "vhostuser", "server", "client", "mcast", @@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def->data.ethernet.ipaddr); break;
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + virDomainChrSourceDefFree(def->data.vhostuser); + break; + case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -6665,6 +6670,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *mode = NULL; char *linkstate = NULL; char *addrtype = NULL; + char *vhostuser_mode = NULL; + char *vhostuser_path = NULL; + char *vhostuser_type = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; @@ -6710,6 +6718,21 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + } else if (!vhostuser_path && !vhostuser_mode && !vhostuser_type + && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + xmlStrEqual(cur->name, BAD_CAST "source")) { + vhostuser_type = virXMLPropString(cur, "type"); + if (!vhostuser_type || STREQ(vhostuser_type, "unix")) { + vhostuser_path = virXMLPropString(cur, "path"); + vhostuser_mode = virXMLPropString(cur, "mode"); + } + else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("type='%s' unsupported for" + " <interface type='vhostuser'>"), + vhostuser_type); + goto error;
I'd move this check a few lines below to the other checks.
+ } } else if (!def->virtPortProfile && xmlStrEqual(cur->name, BAD_CAST "virtualport")) { if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -6867,6 +6890,50 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, actual = NULL; break;
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if (!model || STRNEQ(model, "virtio")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Wrong or no <model> 'type' attribute " + "specified with <interface type='vhostuser'/>. " + "vhostuser requires the virtio-net* frontend")); + goto error; + } + + if (VIR_ALLOC(def->data.vhostuser) < 0) + goto error; + + if (STREQ(vhostuser_type, "unix")) {
Ouch, in the code I've commented above, you allow vhostuser_type to be a NULL, so this needs to be STREQ_NULLABLE().
+ + (def->data.vhostuser)->type = VIR_DOMAIN_CHR_TYPE_UNIX;
I haven't seen such braces usage since DV stopped writing patches :)
+ + if (vhostuser_path == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'path' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + (def->data.vhostuser)->data.nix.path = vhostuser_path; + + if (vhostuser_mode == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'mode' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + if (STREQ(vhostuser_mode, "server")) + (def->data.vhostuser)->data.nix.listen = true;
Should we disallow other modes? What if user passes: <interface type='vhostuser'> <source type='unix' path='/some/dummy/path' mode='blah'/> </interface>
+ + } + + vhostuser_type = NULL; + vhostuser_path = NULL; + vhostuser_mode = NULL;
I don't think you want to set vhostuser_type or vhostuser_mode to NULL here. In case of _path it's okay as you are stealing the pointer into the CharDev struct, but the _type and _mode are not stolen anywhere so you're leaking them effectively.
+ break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: if (dev != NULL) { def->data.ethernet.dev = dev; @@ -7112,6 +7179,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(portgroup); VIR_FREE(address); VIR_FREE(port); + VIR_FREE(vhostuser_type); + VIR_FREE(vhostuser_path); + VIR_FREE(vhostuser_mode); VIR_FREE(ifname); VIR_FREE(dev); virDomainActualNetDefFree(actual); @@ -15989,6 +16059,17 @@ virDomainNetDefFormat(virBufferPtr buf, def->data.ethernet.ipaddr); break;
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if ((def->data.vhostuser)->type == VIR_DOMAIN_CHR_TYPE_UNIX) { + virBufferAddLit(buf, "<source type='unix'"); + virBufferEscapeString(buf, " path='%s'", + (def->data.vhostuser)->data.nix.path); + if ((def->data.vhostuser)->data.nix.listen) + virBufferAddLit(buf, " mode='server'"); + virBufferAddLit(buf, "/>\n"); + } + break; + case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferEscapeString(buf, "<source bridge='%s'/>\n", def->data.bridge.brname); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eb5bad7..91f7524 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -133,6 +133,12 @@ typedef virDomainIdMapDef *virDomainIdMapDefPtr; typedef struct _virDomainPanicDef virDomainPanicDef; typedef virDomainPanicDef *virDomainPanicDefPtr;
+/* forward declarations virDomainChrSourceDef, required by + * virDomainNetDef + */ +typedef struct _virDomainChrSourceDef virDomainChrSourceDef; +typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -795,6 +801,7 @@ struct _virDomainFSDef { typedef enum { VIR_DOMAIN_NET_TYPE_USER, VIR_DOMAIN_NET_TYPE_ETHERNET, + VIR_DOMAIN_NET_TYPE_VHOSTUSER, VIR_DOMAIN_NET_TYPE_SERVER, VIR_DOMAIN_NET_TYPE_CLIENT, VIR_DOMAIN_NET_TYPE_MCAST,
This change requires other modifications too. For instance I'm seeing this: CC xenxs/libvirt_xenxs_la-xen_sxpr.lo xenxs/xen_sxpr.c: In function 'xenFormatSxprNet': xenxs/xen_sxpr.c:1889:5: error: enumeration value 'VIR_DOMAIN_NET_TYPE_VHOSTUSER' not handled in switch [-Werror=switch] switch (def->type) { ^ cc1: all warnings being treated as errors and its repetitions in other drivers.
@@ -880,6 +887,7 @@ struct _virDomainNetDef { char *dev; char *ipaddr; } ethernet; + virDomainChrSourceDefPtr vhostuser; struct { char *address; int port; @@ -1006,8 +1014,6 @@ typedef enum { } virDomainChrSpicevmcName;
/* The host side information for a character device. */ -typedef struct _virDomainChrSourceDef virDomainChrSourceDef; -typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; struct _virDomainChrSourceDef { int type; /* virDomainChrType */ union {
I think this are the bits you need to squash in (don't forget the documentation on RNG schema): diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4920a4e..1d64f68 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6650,17 +6650,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && xmlStrEqual(cur->name, BAD_CAST "source")) { vhostuser_type = virXMLPropString(cur, "type"); - if (!vhostuser_type || STREQ(vhostuser_type, "unix")) { - vhostuser_path = virXMLPropString(cur, "path"); - vhostuser_mode = virXMLPropString(cur, "mode"); - } - else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("type='%s' unsupported for" - " <interface type='vhostuser'>"), - vhostuser_type); - goto error; - } + vhostuser_path = virXMLPropString(cur, "path"); + vhostuser_mode = virXMLPropString(cur, "mode"); } else if (!def->virtPortProfile && xmlStrEqual(cur->name, BAD_CAST "virtualport")) { if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -6819,43 +6810,51 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - if (!model || STRNEQ(model, "virtio")) { + if (STRNEQ_NULLABLE(model, "virtio")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Wrong or no <model> 'type' attribute " - "specified with <interface type='vhostuser'/>. " - "vhostuser requires the virtio-net* frontend")); + "specified with <interface type='vhostuser'/>. " + "vhostuser requires the virtio-net* frontend")); + goto error; + } + + if (STRNEQ_NULLABLE(vhostuser_type, "unix")) { + if (vhostuser_type) + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("type='%s' unsupported for" + " <interface type='vhostuser'>"), + vhostuser_type); + else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("no type specified for " + "<interface type='vhostuser'>")); + goto error; + } + + if (vhostuser_path == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'path' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + if (vhostuser_mode == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'mode' attribute " + "specified with <interface " + "type='vhostuser'/>")); goto error; } if (VIR_ALLOC(def->data.vhostuser) < 0) goto error; - if (STREQ(vhostuser_type, "unix")) { + def->data.vhostuser->type = VIR_DOMAIN_CHR_TYPE_UNIX; + def->data.vhostuser->data.nix.path = vhostuser_path; - (def->data.vhostuser)->type = VIR_DOMAIN_CHR_TYPE_UNIX; - - if (vhostuser_path == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'path' attribute " - "specified with <interface " - "type='vhostuser'/>")); - goto error; - } - - (def->data.vhostuser)->data.nix.path = vhostuser_path; - - if (vhostuser_mode == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'mode' attribute " - "specified with <interface " - "type='vhostuser'/>")); - goto error; - } - - if (STREQ(vhostuser_mode, "server")) - (def->data.vhostuser)->data.nix.listen = true; - - } + if (STREQ(vhostuser_mode, "server")) + (def->data.vhostuser)->data.nix.listen = true; vhostuser_type = NULL; vhostuser_path = NULL; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0b4a0b5..a193a93 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -987,6 +987,7 @@ libxlMakeNic(virDomainDefPtr def, break; } case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 7a206d2..1d5d689 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -248,6 +248,11 @@ umlBuildCommandLineNet(virConnectPtr conn, _("hostdev networking type not supported")); goto error; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vhostuser networking type not supported")); + goto error; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 9e59804..3d46773 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1932,6 +1932,7 @@ xenFormatSxprNet(virConnectPtr conn, break; case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: Michal
On 10/07/2014 18:01, Michal Privoznik wrote:
On 02.07.2014 15:20, Michele Paolino wrote:
vhost-user is added as a virDomainChrSourceDefPtr variable in the virtual network interface configuration structure. This patch adds support to parse vhost-user element. The XML file looks like:
<interface type='vhostuser'> <source type='unix' path='/tmp/vhost.sock' mode='server'/> <mac address='52:54:00:3b:83:1a'/> <model type='virtio'/> </interface>
Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- src/conf/domain_conf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++-- 2 files changed, 89 insertions(+), 2 deletions(-) Introducing a new device (subtype of a device) must always go hand in hand with documentation and XML schema adjustment. Moreover, it would be nice to test the new XML (e.g. domainschematest or qemuxml2argv stub (a .xml file under tests/qemuxml2argvdata without .args counterpart which will be introduced once qemu implementation is done)).
Yes, these files are present in 3/4. I will post the v2 of this series in a single patch.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ea09bdc..de52ca4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "user", "ethernet", + "vhostuser", "server", "client", "mcast", @@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def->data.ethernet.ipaddr); break;
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + virDomainChrSourceDefFree(def->data.vhostuser); + break; + case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -6665,6 +6670,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *mode = NULL; char *linkstate = NULL; char *addrtype = NULL; + char *vhostuser_mode = NULL; + char *vhostuser_path = NULL; + char *vhostuser_type = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; @@ -6710,6 +6718,21 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + } else if (!vhostuser_path && !vhostuser_mode && !vhostuser_type + && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + xmlStrEqual(cur->name, BAD_CAST "source")) { + vhostuser_type = virXMLPropString(cur, "type"); + if (!vhostuser_type || STREQ(vhostuser_type, "unix")) { + vhostuser_path = virXMLPropString(cur, "path"); + vhostuser_mode = virXMLPropString(cur, "mode"); + } + else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("type='%s' unsupported for" + " <interface type='vhostuser'>"), + vhostuser_type); + goto error; I'd move this check a few lines below to the other checks.
The check has been done here because if in future we will decide to support other chardevs in addition to the unix socket, the XML file will be different. For example, if we want to add the support for a TCP socket, the path attribute needs to be replaced with host, service and protocol. After a quick look at the _virDomainChrSourceDef structure, the XML description in this case should look like: <source type='tcp' host='host' service='serv' mode='mode' protocol='prot'/> Do we want to add these additional checks only when the support for other chardevs will be added, or is there an alternative solution?
+ } } else if (!def->virtPortProfile && xmlStrEqual(cur->name, BAD_CAST "virtualport")) { if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -6867,6 +6890,50 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, actual = NULL; break;
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if (!model || STRNEQ(model, "virtio")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Wrong or no <model> 'type' attribute " + "specified with <interface type='vhostuser'/>. " + "vhostuser requires the virtio-net* frontend")); + goto error; + } + + if (VIR_ALLOC(def->data.vhostuser) < 0) + goto error; + + if (STREQ(vhostuser_type, "unix")) { Ouch, in the code I've commented above, you allow vhostuser_type to be a NULL, so this needs to be STREQ_NULLABLE().
+ + (def->data.vhostuser)->type = VIR_DOMAIN_CHR_TYPE_UNIX; I haven't seen such braces usage since DV stopped writing patches :)
+ + if (vhostuser_path == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'path' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + (def->data.vhostuser)->data.nix.path = vhostuser_path; + + if (vhostuser_mode == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'mode' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + if (STREQ(vhostuser_mode, "server")) + (def->data.vhostuser)->data.nix.listen = true; Should we disallow other modes? What if user passes:
<interface type='vhostuser'> <source type='unix' path='/some/dummy/path' mode='blah'/> </interface>
The nix structure in the _virDomainChrSourceDef uses a boolean to define the socket mode. My proposal is to set listen=true only if mode='server' and have it equal to false (client) otherwise.
+ + } + + vhostuser_type = NULL; + vhostuser_path = NULL; + vhostuser_mode = NULL; I don't think you want to set vhostuser_type or vhostuser_mode to NULL here. In case of _path it's okay as you are stealing the pointer into the CharDev struct, but the _type and _mode are not stolen anywhere so you're leaking them effectively.
The (converted) content of vhostuser_type and vhostuser_mode can be retrieved in "(def->data.vhostuser)->data.nix.listen" and "(def->data.vhostuser)->type". Do we want their content available anyway?
+ break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: if (dev != NULL) { def->data.ethernet.dev = dev; @@ -7112,6 +7179,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(portgroup); VIR_FREE(address); VIR_FREE(port); + VIR_FREE(vhostuser_type); + VIR_FREE(vhostuser_path); + VIR_FREE(vhostuser_mode); VIR_FREE(ifname); VIR_FREE(dev); virDomainActualNetDefFree(actual); @@ -15989,6 +16059,17 @@ virDomainNetDefFormat(virBufferPtr buf, def->data.ethernet.ipaddr); break;
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if ((def->data.vhostuser)->type == VIR_DOMAIN_CHR_TYPE_UNIX) { + virBufferAddLit(buf, "<source type='unix'"); + virBufferEscapeString(buf, " path='%s'", + (def->data.vhostuser)->data.nix.path); + if ((def->data.vhostuser)->data.nix.listen) + virBufferAddLit(buf, " mode='server'"); + virBufferAddLit(buf, "/>\n"); + } + break; + case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferEscapeString(buf, "<source bridge='%s'/>\n", def->data.bridge.brname); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eb5bad7..91f7524 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -133,6 +133,12 @@ typedef virDomainIdMapDef *virDomainIdMapDefPtr; typedef struct _virDomainPanicDef virDomainPanicDef; typedef virDomainPanicDef *virDomainPanicDefPtr;
+/* forward declarations virDomainChrSourceDef, required by + * virDomainNetDef + */ +typedef struct _virDomainChrSourceDef virDomainChrSourceDef; +typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -795,6 +801,7 @@ struct _virDomainFSDef { typedef enum { VIR_DOMAIN_NET_TYPE_USER, VIR_DOMAIN_NET_TYPE_ETHERNET, + VIR_DOMAIN_NET_TYPE_VHOSTUSER, VIR_DOMAIN_NET_TYPE_SERVER, VIR_DOMAIN_NET_TYPE_CLIENT, VIR_DOMAIN_NET_TYPE_MCAST, This change requires other modifications too. For instance I'm seeing this:
CC xenxs/libvirt_xenxs_la-xen_sxpr.lo xenxs/xen_sxpr.c: In function 'xenFormatSxprNet': xenxs/xen_sxpr.c:1889:5: error: enumeration value 'VIR_DOMAIN_NET_TYPE_VHOSTUSER' not handled in switch [-Werror=switch] switch (def->type) { ^ cc1: all warnings being treated as errors
and its repetitions in other drivers.
@@ -880,6 +887,7 @@ struct _virDomainNetDef { char *dev; char *ipaddr; } ethernet; + virDomainChrSourceDefPtr vhostuser; struct { char *address; int port; @@ -1006,8 +1014,6 @@ typedef enum { } virDomainChrSpicevmcName;
/* The host side information for a character device. */ -typedef struct _virDomainChrSourceDef virDomainChrSourceDef; -typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; struct _virDomainChrSourceDef { int type; /* virDomainChrType */ union {
I think this are the bits you need to squash in (don't forget the documentation on RNG schema):
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4920a4e..1d64f68 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6650,17 +6650,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && xmlStrEqual(cur->name, BAD_CAST "source")) { vhostuser_type = virXMLPropString(cur, "type"); - if (!vhostuser_type || STREQ(vhostuser_type, "unix")) { - vhostuser_path = virXMLPropString(cur, "path"); - vhostuser_mode = virXMLPropString(cur, "mode"); - } - else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("type='%s' unsupported for" - " <interface type='vhostuser'>"), - vhostuser_type); - goto error; - } + vhostuser_path = virXMLPropString(cur, "path"); + vhostuser_mode = virXMLPropString(cur, "mode"); } else if (!def->virtPortProfile && xmlStrEqual(cur->name, BAD_CAST "virtualport")) { if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -6819,43 +6810,51 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, break;
case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - if (!model || STRNEQ(model, "virtio")) { + if (STRNEQ_NULLABLE(model, "virtio")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Wrong or no <model> 'type' attribute " - "specified with <interface type='vhostuser'/>. " - "vhostuser requires the virtio-net* frontend")); + "specified with <interface type='vhostuser'/>. " + "vhostuser requires the virtio-net* frontend")); + goto error; + } + + if (STRNEQ_NULLABLE(vhostuser_type, "unix")) { + if (vhostuser_type) + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("type='%s' unsupported for" + " <interface type='vhostuser'>"), + vhostuser_type); + else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("no type specified for " + "<interface type='vhostuser'>")); + goto error; + } + + if (vhostuser_path == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'path' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + if (vhostuser_mode == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'mode' attribute " + "specified with <interface " + "type='vhostuser'/>")); goto error; }
if (VIR_ALLOC(def->data.vhostuser) < 0) goto error;
- if (STREQ(vhostuser_type, "unix")) { + def->data.vhostuser->type = VIR_DOMAIN_CHR_TYPE_UNIX; + def->data.vhostuser->data.nix.path = vhostuser_path;
- (def->data.vhostuser)->type = VIR_DOMAIN_CHR_TYPE_UNIX; - - if (vhostuser_path == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'path' attribute " - "specified with <interface " - "type='vhostuser'/>")); - goto error; - } - - (def->data.vhostuser)->data.nix.path = vhostuser_path; - - if (vhostuser_mode == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'mode' attribute " - "specified with <interface " - "type='vhostuser'/>")); - goto error; - } - - if (STREQ(vhostuser_mode, "server")) - (def->data.vhostuser)->data.nix.listen = true; - - } + if (STREQ(vhostuser_mode, "server")) + (def->data.vhostuser)->data.nix.listen = true;
vhostuser_type = NULL; vhostuser_path = NULL; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0b4a0b5..a193a93 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -987,6 +987,7 @@ libxlMakeNic(virDomainDefPtr def, break; } case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 7a206d2..1d5d689 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -248,6 +248,11 @@ umlBuildCommandLineNet(virConnectPtr conn, _("hostdev networking type not supported")); goto error;
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vhostuser networking type not supported")); + goto error; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 9e59804..3d46773 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1932,6 +1932,7 @@ xenFormatSxprNet(virConnectPtr conn, break;
case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST:
Michal
-- Michele Paolino
On 11.07.2014 13:06, Michele Paolino wrote:
On 10/07/2014 18:01, Michal Privoznik wrote:
On 02.07.2014 15:20, Michele Paolino wrote:
vhost-user is added as a virDomainChrSourceDefPtr variable in the virtual network interface configuration structure. This patch adds support to parse vhost-user element. The XML file looks like:
<interface type='vhostuser'> <source type='unix' path='/tmp/vhost.sock' mode='server'/> <mac address='52:54:00:3b:83:1a'/> <model type='virtio'/> </interface>
Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- src/conf/domain_conf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++-- 2 files changed, 89 insertions(+), 2 deletions(-) Introducing a new device (subtype of a device) must always go hand in hand with documentation and XML schema adjustment. Moreover, it would be nice to test the new XML (e.g. domainschematest or qemuxml2argv stub (a .xml file under tests/qemuxml2argvdata without .args counterpart which will be introduced once qemu implementation is done)).
Yes, these files are present in 3/4. I will post the v2 of this series in a single patch.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ea09bdc..de52ca4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "user", "ethernet", + "vhostuser", "server", "client", "mcast", @@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def->data.ethernet.ipaddr); break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + virDomainChrSourceDefFree(def->data.vhostuser); + break; + case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -6665,6 +6670,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *mode = NULL; char *linkstate = NULL; char *addrtype = NULL; + char *vhostuser_mode = NULL; + char *vhostuser_path = NULL; + char *vhostuser_type = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; @@ -6710,6 +6718,21 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + } else if (!vhostuser_path && !vhostuser_mode && !vhostuser_type + && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + xmlStrEqual(cur->name, BAD_CAST "source")) { + vhostuser_type = virXMLPropString(cur, "type"); + if (!vhostuser_type || STREQ(vhostuser_type, "unix")) { + vhostuser_path = virXMLPropString(cur, "path"); + vhostuser_mode = virXMLPropString(cur, "mode"); + } + else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("type='%s' unsupported for" + " <interface type='vhostuser'>"), + vhostuser_type); + goto error; I'd move this check a few lines below to the other checks.
The check has been done here because if in future we will decide to support other chardevs in addition to the unix socket, the XML file will be different.
Until then I find the code easier to read with checks grouped.
For example, if we want to add the support for a TCP socket, the path attribute needs to be replaced with host, service and protocol. After a quick look at the _virDomainChrSourceDef structure, the XML description in this case should look like:
<source type='tcp' host='host' service='serv' mode='mode' protocol='prot'/>
Do we want to add these additional checks only when the support for other chardevs will be added, or is there an alternative solution?
Yes. There's no need to introduce the checks for unsupported combinations now.
+ } } else if (!def->virtPortProfile && xmlStrEqual(cur->name, BAD_CAST "virtualport")) { if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -6867,6 +6890,50 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, actual = NULL; break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if (!model || STRNEQ(model, "virtio")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Wrong or no <model> 'type' attribute " + "specified with <interface type='vhostuser'/>. " + "vhostuser requires the virtio-net* frontend")); + goto error; + } + + if (VIR_ALLOC(def->data.vhostuser) < 0) + goto error; + + if (STREQ(vhostuser_type, "unix")) { Ouch, in the code I've commented above, you allow vhostuser_type to be a NULL, so this needs to be STREQ_NULLABLE().
+ + (def->data.vhostuser)->type = VIR_DOMAIN_CHR_TYPE_UNIX; I haven't seen such braces usage since DV stopped writing patches :)
+ + if (vhostuser_path == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'path' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + (def->data.vhostuser)->data.nix.path = vhostuser_path; + + if (vhostuser_mode == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'mode' attribute " + "specified with <interface " + "type='vhostuser'/>")); + goto error; + } + + if (STREQ(vhostuser_mode, "server")) + (def->data.vhostuser)->data.nix.listen = true; Should we disallow other modes? What if user passes:
<interface type='vhostuser'> <source type='unix' path='/some/dummy/path' mode='blah'/> </interface>
The nix structure in the _virDomainChrSourceDef uses a boolean to define the socket mode. My proposal is to set listen=true only if mode='server' and have it equal to false (client) otherwise.
Correct and I understood that. But the problem is mode='blah' has the same effect as mode='client' or any other value but 'server'. I think we should reject other strings than 'server' or 'client'. Or even reject 'client' too (and assume client mode whenever vhostuser_mode == NULL). I'll leave that up to you.
+ + } + + vhostuser_type = NULL; + vhostuser_path = NULL; + vhostuser_mode = NULL; I don't think you want to set vhostuser_type or vhostuser_mode to NULL here. In case of _path it's okay as you are stealing the pointer into the CharDev struct, but the _type and _mode are not stolen anywhere so you're leaking them effectively.
The (converted) content of vhostuser_type and vhostuser_mode can be retrieved in "(def->data.vhostuser)->data.nix.listen" and "(def->data.vhostuser)->type".
Correct, but you still need to free() the vhostuser_type and vhostuser_mode after the conversion. For instance, consider following analogical code: { char *c = strdup("123"); int i; virStrToLong_ui(c, NULL, 10, &i); return; } At the point of 'return' @i contains converted value, but the memory allocated for @c is leaked. Same goes for virXMLPropString() instead of strdup().
Do we want their content available anyway?
Yes, but only for the time of parsing. After that you can free them. Michal
Build the qemu-command line for vhost-user. Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- src/qemu/qemu_command.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ab8cec5..c1cf001 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6930,6 +6930,61 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, } static int +qemuBuildVhostuserCommandLine(virCommandPtr cmd, + virDomainDefPtr def, + virDomainNetDefPtr net, + virQEMUCapsPtr qemuCaps) +{ + virBuffer chardev_buf = VIR_BUFFER_INITIALIZER; + virBuffer netdev_buf = VIR_BUFFER_INITIALIZER; + + char* nic = NULL; + + if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Netdev support unavailable")); + goto error; + } + + if ((net->data.vhostuser)->type == VIR_DOMAIN_CHR_TYPE_UNIX) { + virBufferAsprintf(&chardev_buf, "socket,id=char%s,path=%s%s", + net->info.alias, (net->data.vhostuser)->data.nix.path, + (net->data.vhostuser)->data.nix.listen ? ",server" : ""); + } + + virBufferAsprintf(&netdev_buf, "type=vhost-user,id=host%s,chardev=char%s", + net->info.alias, net->info.alias); + + if (virBufferError(&chardev_buf) || virBufferError(&netdev_buf)) { + virReportOOMError(); + goto error; + } + + virCommandAddArgList(cmd, "-chardev", virBufferContentAndReset(&chardev_buf), + NULL); + virCommandAddArgList(cmd, "-netdev", virBufferContentAndReset(&netdev_buf), + NULL); + + if (!(nic = qemuBuildNicDevStr(def, net, -1, 0, 0, qemuCaps))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Error generating NIC -device string")); + goto error; + } + + virCommandAddArgList(cmd, "-device", nic, NULL); + VIR_FREE(nic); + + return 0; + + error: + virBufferFreeAndReset(&chardev_buf); + virBufferFreeAndReset(&netdev_buf); + VIR_FREE(nic); + + return -1; +} + +static int qemuBuildInterfaceCommandLine(virCommandPtr cmd, virQEMUDriverPtr driver, virConnectPtr conn, @@ -6952,6 +7007,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, int actualType = virDomainNetGetActualType(net); size_t i; + if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) + return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps); + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { /* NET_TYPE_HOSTDEV devices are really hostdev devices, so * their commandlines are constructed with other hostdevs. -- 1.9.3
On 02.07.2014 15:20, Michele Paolino wrote:
Build the qemu-command line for vhost-user.
Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- src/qemu/qemu_command.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
This is the patch where .args and qemuxml2argvtest.c modifications would have been done if you go with my suggestion in 1/4.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ab8cec5..c1cf001 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6930,6 +6930,61 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, }
static int +qemuBuildVhostuserCommandLine(virCommandPtr cmd, + virDomainDefPtr def, + virDomainNetDefPtr net, + virQEMUCapsPtr qemuCaps) +{ + virBuffer chardev_buf = VIR_BUFFER_INITIALIZER; + virBuffer netdev_buf = VIR_BUFFER_INITIALIZER; + + char* nic = NULL; + + if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Netdev support unavailable")); + goto error; + } + + if ((net->data.vhostuser)->type == VIR_DOMAIN_CHR_TYPE_UNIX) { + virBufferAsprintf(&chardev_buf, "socket,id=char%s,path=%s%s", + net->info.alias, (net->data.vhostuser)->data.nix.path, + (net->data.vhostuser)->data.nix.listen ? ",server" : ""); + } + + virBufferAsprintf(&netdev_buf, "type=vhost-user,id=host%s,chardev=char%s", + net->info.alias, net->info.alias); + + if (virBufferError(&chardev_buf) || virBufferError(&netdev_buf)) { + virReportOOMError(); + goto error; + } + + virCommandAddArgList(cmd, "-chardev", virBufferContentAndReset(&chardev_buf), + NULL); + virCommandAddArgList(cmd, "-netdev", virBufferContentAndReset(&netdev_buf), + NULL);
For some reason we tend to store the return value of virBufferContentAndReset() into a local variable which is then passed to virCommandAddArgList(). Maybe for better debugging options.
+ + if (!(nic = qemuBuildNicDevStr(def, net, -1, 0, 0, qemuCaps))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Error generating NIC -device string")); + goto error; + } + + virCommandAddArgList(cmd, "-device", nic, NULL); + VIR_FREE(nic); + + return 0; + + error: + virBufferFreeAndReset(&chardev_buf); + virBufferFreeAndReset(&netdev_buf); + VIR_FREE(nic); + + return -1; +} + +static int qemuBuildInterfaceCommandLine(virCommandPtr cmd, virQEMUDriverPtr driver, virConnectPtr conn, @@ -6952,6 +7007,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, int actualType = virDomainNetGetActualType(net); size_t i;
+ if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) + return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps); + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { /* NET_TYPE_HOSTDEV devices are really hostdev devices, so * their commandlines are constructed with other hostdevs.
Michal
This patch adds the test files and the documentation for vhost-user. Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- docs/formatdomain.html.in | 34 +++++++++++++++++++ docs/schemas/domaincommon.rng | 39 ++++++++++++++++++++++ .../qemuxml2argv-net-vhostuser.args | 7 ++++ .../qemuxml2argv-net-vhostuser.xml | 33 ++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 115 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3f8bbee..606b7d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.5</span> </p> + <h5><a name="elementVhostuser">vhost-user interface</a></h5> + + <p> + vhost-user enables the communication between a QEMU virtual machine + and other userspace process using the Virtio transport protocol. + A char dev (e.g. Unix socket) is used for the control plane, while + the data plane is based on shared memory. + </p> + +<pre> + ... + <devices> + <interface type='vhostuser'> + <source type='unix' path='/tmp/vhost.sock' mode='server'> + </source> + <mac address='52:54:00:3b:83:1a'> + </mac> + <model type='virtio'> + </model> + </interface> + </devices> + ...</pre> + + <p> + The <code><source></code> element has to be specified + along with the type of char device. + Currently, only type='unix' is supported, where the path (the + directory path of the socket) and mode attributes are required. + Both <code>mode='server'</code> and <code>mode='client'</code> + are supported. + vhost-user requires the virtio model type, thus the + <code><model></code> element is mandatory. + </p> + <h4><a name="elementsInput">Input devices</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..f85dd61 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,45 @@ </group> <group> <attribute name="type"> + <value>vhostuser</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="type"> + <choice> + <value>null</value> + <value>vc</value> + <value>pty</value> + <value>dev</value> + <value>file</value> + <value>pipe</value> + <value>stdio</value> + <value>udp</value> + <value>tcp</value> + <value>unix</value> + <value>spicevmc</value> + <value>spiceport</value> + <value>nmdm</value> + </choice> + </attribute> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute> + <attribute name="mode"> + <choice> + <value>server</value> + <value>client</value> + </choice> + </attribute> + <empty/> + </element> + </optional> + <ref name="interface-options"/> + </interleave> + </group> + <group> + <attribute name="type"> <value>network</value> </attribute> <interleave> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args new file mode 100644 index 0000000..cc66ec3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \ +-chardev socket,id=charnet0,path=/tmp/vhost.sock,server \ +-netdev type=vhost-user,id=hostnet0,chardev=charnet0 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ee:96:6b,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml new file mode 100644 index 0000000..b49d48e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <interface type='vhostuser'> + <mac address='52:54:00:ee:96:6b'/> + <source type='unix' path='/tmp/vhost.sock' mode='server'/> + <model type='virtio'/> + </interface> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b4414d6..d4b082a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -927,6 +927,7 @@ mymain(void) DO_TEST_FAILURE("misc-enable-s4", NONE); DO_TEST("misc-no-reboot", NONE); DO_TEST("misc-uuid", QEMU_CAPS_NAME, QEMU_CAPS_UUID); + DO_TEST("net-vhostuser", QEMU_CAPS_DEVICE, QEMU_CAPS_NETDEV); DO_TEST("net-user", NONE); DO_TEST("net-virtio", NONE); DO_TEST("net-virtio-device", diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 43cd022..1fd0ad1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -245,6 +245,7 @@ mymain(void) DO_TEST("misc-disable-suspends"); DO_TEST("misc-enable-s4"); DO_TEST("misc-no-reboot"); + DO_TEST("net-vhostuser"); DO_TEST("net-user"); DO_TEST("net-virtio"); DO_TEST("net-virtio-device"); -- 1.9.3
On 02.07.2014 15:20, Michele Paolino wrote:
This patch adds the test files and the documentation for vhost-user.
Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- docs/formatdomain.html.in | 34 +++++++++++++++++++ docs/schemas/domaincommon.rng | 39 ++++++++++++++++++++++ .../qemuxml2argv-net-vhostuser.args | 7 ++++ .../qemuxml2argv-net-vhostuser.xml | 33 ++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 115 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml
Aha! You are doing here what I've just requested in 1/4. Still I rather see it in a single patch (even though it will be bigger) because it make maintainers life easier. You know, backporting a single patch versus digging through 'git log' to search for this commit ...
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3f8bbee..606b7d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.5</span> </p>
+ <h5><a name="elementVhostuser">vhost-user interface</a></h5> + + <p> + vhost-user enables the communication between a QEMU virtual machine + and other userspace process using the Virtio transport protocol. + A char dev (e.g. Unix socket) is used for the control plane, while + the data plane is based on shared memory. + </p> + +<pre> + ... + <devices> + <interface type='vhostuser'> + <source type='unix' path='/tmp/vhost.sock' mode='server'> + </source> + <mac address='52:54:00:3b:83:1a'> + </mac> + <model type='virtio'> + </model> + </interface> + </devices> + ...</pre> + + <p> + The <code><source></code> element has to be specified + along with the type of char device. + Currently, only type='unix' is supported, where the path (the + directory path of the socket) and mode attributes are required. + Both <code>mode='server'</code> and <code>mode='client'</code> + are supported. + vhost-user requires the virtio model type, thus the + <code><model></code> element is mandatory. + </p> + <h4><a name="elementsInput">Input devices</a></h4>
<p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..f85dd61 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,45 @@ </group> <group> <attribute name="type"> + <value>vhostuser</value> + </attribute> + <interleave> + <optional> + <element name="source">
I wouldn't say <source/> is optional in case of <interface type='vhostnet'/>. It contains crucial information that helps us construct the correct qemu command line which would not be possible otherwise.
+ <attribute name="type"> + <choice> + <value>null</value> + <value>vc</value> + <value>pty</value> + <value>dev</value> + <value>file</value> + <value>pipe</value> + <value>stdio</value> + <value>udp</value> + <value>tcp</value> + <value>unix</value> + <value>spicevmc</value> + <value>spiceport</value> + <value>nmdm</value>
Honestly, I'm inclined to not enumerate all of these here now. I mean, only 'unix' is supported now. On the other hand, it's acceptable to have a RNG schema that allows something more than our XML parser.
+ </choice> + </attribute> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute> + <attribute name="mode"> + <choice> + <value>server</value> + <value>client</value> + </choice> + </attribute> + <empty/> + </element> + </optional> + <ref name="interface-options"/> + </interleave> + </group> + <group> + <attribute name="type"> <value>network</value> </attribute> <interleave> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args new file mode 100644 index 0000000..cc66ec3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \ +-chardev socket,id=charnet0,path=/tmp/vhost.sock,server \ +-netdev type=vhost-user,id=hostnet0,chardev=charnet0 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ee:96:6b,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml new file mode 100644 index 0000000..b49d48e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <interface type='vhostuser'> + <mac address='52:54:00:ee:96:6b'/> + <source type='unix' path='/tmp/vhost.sock' mode='server'/> + <model type='virtio'/> + </interface> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b4414d6..d4b082a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -927,6 +927,7 @@ mymain(void) DO_TEST_FAILURE("misc-enable-s4", NONE); DO_TEST("misc-no-reboot", NONE); DO_TEST("misc-uuid", QEMU_CAPS_NAME, QEMU_CAPS_UUID); + DO_TEST("net-vhostuser", QEMU_CAPS_DEVICE, QEMU_CAPS_NETDEV); DO_TEST("net-user", NONE); DO_TEST("net-virtio", NONE); DO_TEST("net-virtio-device", diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 43cd022..1fd0ad1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -245,6 +245,7 @@ mymain(void) DO_TEST("misc-disable-suspends"); DO_TEST("misc-enable-s4"); DO_TEST("misc-no-reboot"); + DO_TEST("net-vhostuser"); DO_TEST("net-user"); DO_TEST("net-virtio"); DO_TEST("net-virtio-device");
Nice, I've forgotten about qemuxml2xmltest completely. Michal
On 10/07/2014 18:01, Michal Privoznik wrote:
On 02.07.2014 15:20, Michele Paolino wrote:
This patch adds the test files and the documentation for vhost-user.
Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- docs/formatdomain.html.in | 34 +++++++++++++++++++ docs/schemas/domaincommon.rng | 39 ++++++++++++++++++++++ .../qemuxml2argv-net-vhostuser.args | 7 ++++ .../qemuxml2argv-net-vhostuser.xml | 33 ++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 115 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml
Aha! You are doing here what I've just requested in 1/4. Still I rather see it in a single patch (even though it will be bigger) because it make maintainers life easier. You know, backporting a single patch versus digging through 'git log' to search for this commit ...
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3f8bbee..606b7d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.5</span> </p>
+ <h5><a name="elementVhostuser">vhost-user interface</a></h5> + + <p> + vhost-user enables the communication between a QEMU virtual machine + and other userspace process using the Virtio transport protocol. + A char dev (e.g. Unix socket) is used for the control plane, while + the data plane is based on shared memory. + </p> + +<pre> + ... + <devices> + <interface type='vhostuser'> + <source type='unix' path='/tmp/vhost.sock' mode='server'> + </source> + <mac address='52:54:00:3b:83:1a'> + </mac> + <model type='virtio'> + </model> + </interface> + </devices> + ...</pre> + + <p> + The <code><source></code> element has to be specified + along with the type of char device. + Currently, only type='unix' is supported, where the path (the + directory path of the socket) and mode attributes are required. + Both <code>mode='server'</code> and <code>mode='client'</code> + are supported. + vhost-user requires the virtio model type, thus the + <code><model></code> element is mandatory. + </p> + <h4><a name="elementsInput">Input devices</a></h4>
<p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..f85dd61 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,45 @@ </group> <group> <attribute name="type"> + <value>vhostuser</value> + </attribute> + <interleave> + <optional> + <element name="source">
I wouldn't say <source/> is optional in case of <interface type='vhostnet'/>. It contains crucial information that helps us construct the correct qemu command line which would not be possible otherwise.
I agree, but the schemas of the other interfaces it's the same. Please see the Laine's comment at http://www.redhat.com/archives/libvir-list/2014-June/msg00279.html
+ <attribute name="type"> + <choice> + <value>null</value> + <value>vc</value> + <value>pty</value> + <value>dev</value> + <value>file</value> + <value>pipe</value> + <value>stdio</value> + <value>udp</value> + <value>tcp</value> + <value>unix</value> + <value>spicevmc</value> + <value>spiceport</value> + <value>nmdm</value>
Honestly, I'm inclined to not enumerate all of these here now. I mean, only 'unix' is supported now. On the other hand, it's acceptable to have a RNG schema that allows something more than our XML parser.
+ </choice> + </attribute> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute> + <attribute name="mode"> + <choice> + <value>server</value> + <value>client</value> + </choice> + </attribute> + <empty/> + </element> + </optional> + <ref name="interface-options"/> + </interleave> + </group> + <group> + <attribute name="type"> <value>network</value> </attribute> <interleave> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args new file mode 100644 index 0000000..cc66ec3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \ +-chardev socket,id=charnet0,path=/tmp/vhost.sock,server \ +-netdev type=vhost-user,id=hostnet0,chardev=charnet0 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ee:96:6b,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml new file mode 100644 index 0000000..b49d48e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <interface type='vhostuser'> + <mac address='52:54:00:ee:96:6b'/> + <source type='unix' path='/tmp/vhost.sock' mode='server'/> + <model type='virtio'/> + </interface> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b4414d6..d4b082a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -927,6 +927,7 @@ mymain(void) DO_TEST_FAILURE("misc-enable-s4", NONE); DO_TEST("misc-no-reboot", NONE); DO_TEST("misc-uuid", QEMU_CAPS_NAME, QEMU_CAPS_UUID); + DO_TEST("net-vhostuser", QEMU_CAPS_DEVICE, QEMU_CAPS_NETDEV); DO_TEST("net-user", NONE); DO_TEST("net-virtio", NONE); DO_TEST("net-virtio-device", diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 43cd022..1fd0ad1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -245,6 +245,7 @@ mymain(void) DO_TEST("misc-disable-suspends"); DO_TEST("misc-enable-s4"); DO_TEST("misc-no-reboot"); + DO_TEST("net-vhostuser"); DO_TEST("net-user"); DO_TEST("net-virtio"); DO_TEST("net-virtio-device");
Nice, I've forgotten about qemuxml2xmltest completely.
Michal
-- Michele Paolino
On 11.07.2014 13:07, Michele Paolino wrote:
On 10/07/2014 18:01, Michal Privoznik wrote:
On 02.07.2014 15:20, Michele Paolino wrote:
This patch adds the test files and the documentation for vhost-user.
Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- docs/formatdomain.html.in | 34 +++++++++++++++++++ docs/schemas/domaincommon.rng | 39 ++++++++++++++++++++++ .../qemuxml2argv-net-vhostuser.args | 7 ++++ .../qemuxml2argv-net-vhostuser.xml | 33 ++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 115 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..f85dd61 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,45 @@ </group> <group> <attribute name="type"> + <value>vhostuser</value> + </attribute> + <interleave> + <optional> + <element name="source">
I wouldn't say <source/> is optional in case of <interface type='vhostnet'/>. It contains crucial information that helps us construct the correct qemu command line which would not be possible otherwise.
I agree, but the schemas of the other interfaces it's the same. Please see the Laine's comment at http://www.redhat.com/archives/libvir-list/2014-June/msg00279.html
Well, not for all other interface types. Moreover, currently the your patches require <source/>. But I can live with schema wider than our parser. Michal
vhost-user is a qemu feature. Initial support for lxc,uml and xenxs added. Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- src/lxc/lxc_process.c | 1 + src/uml/uml_conf.c | 5 +++++ src/xenxs/xen_sxpr.c | 1 + 3 files changed, 7 insertions(+) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 0aef13a..854f65d 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -437,6 +437,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_ETHERNET: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 464d56d..d33d9b4 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -182,6 +182,11 @@ umlBuildCommandLineNet(virConnectPtr conn, } break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vhostuser networking type not supported")); + goto error; + case VIR_DOMAIN_NET_TYPE_SERVER: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("TCP server networking type not supported")); diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index aacf74c..fe49c42 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1933,6 +1933,7 @@ xenFormatSxprNet(virConnectPtr conn, virBufferEscapeSexpr(buf, "(ip '%s')", def->data.ethernet.ipaddr); break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: -- 1.9.3
On 02.07.2014 15:20, Michele Paolino wrote:
vhost-user is a qemu feature. Initial support for lxc,uml and xenxs added.
Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com> --- src/lxc/lxc_process.c | 1 + src/uml/uml_conf.c | 5 +++++ src/xenxs/xen_sxpr.c | 1 + 3 files changed, 7 insertions(+)
Right. Well, here at libvirt we require the sources to be able to compile after each commit. So this needs to go into 1/4. Michal
ping :) On Wed, Jul 2, 2014 at 3:20 PM, Michele Paolino < m.paolino@virtualopensystems.com> wrote:
This series adds support for the QEMU vhost-user feature to libvirt. vhost-user enables the communication between a QEMU virtual machine and other userspace process using the Virtio transport protocol. It uses a char dev (e.g. Unix socket) for the control plane, while the data plane based on shared memory.
The XML looks like:
<interface type='vhostuser'> <source type='unix' path='/tmp/vhost.sock' mode='server'/> <mac address='52:54:00:3b:83:1a'/> <model type='virtio'/> </interface>
Our use case is the deployment of Snabbswitch in an OpenStack/NFV environment. Snabbswitch uses a Unix socket to implement the vhost-user control plane, thus we focused on the support for the type=unix attribute of the <source> element. To test it with Snabbswitch, it is necessary to apply the following patches (respectively from Chen Fan and Michele Paolino): http://www.redhat.com/archives/libvir-list/2014-June/msg01195.html http://www.redhat.com/archives/libvir-list/2014-June/msg01418.html
It is also possible to directly checkout the Virtual Open Systems' libvirt repository(branch "vhost-user_support) at the address: https://github.com/virtualopensystems/libvirt.
This patch is based on the previous work from Luke Gorrie: http://www.redhat.com/archives/libvir-list/2014-May/msg00934.html
Michele Paolino (4): vhost-user support: domain configuration vhost-user support: qemu command-line vhost-user support: tests and docs vhost-user support: lxc,xenxs,uml
docs/formatdomain.html.in | 34 +++++++++ docs/schemas/domaincommon.rng | 39 +++++++++++ src/conf/domain_conf.c | 81 ++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++- src/lxc/lxc_process.c | 1 + src/qemu/qemu_command.c | 58 ++++++++++++++++ src/uml/uml_conf.c | 5 ++ src/xenxs/xen_sxpr.c | 1 + .../qemuxml2argv-net-vhostuser.args | 7 ++ .../qemuxml2argv-net-vhostuser.xml | 33 +++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 12 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml
-- 1.9.3
participants (2)
-
Michal Privoznik -
Michele Paolino