Re: [libvirt] [PATCH 1/4] vhost-user support: domain configuration
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.paol...@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
Re: [libvirt] [PATCH 3/4] vhost-user support: tests and docs
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.paol...@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=sinceSince 0.9.5/span /p +h5a name=elementVhostuservhost-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 + ... + lt;devicesgt; +lt;interface type='vhostuser'gt; + lt;source type='unix' path='/tmp/vhost.sock' mode='server'gt; + lt;/sourcegt; + lt;mac address='52:54:00:3b:83:1a'gt; + lt;/macgt; + lt;model type='virtio'gt; + lt;/modelgt; +lt;/interfacegt; + lt;/devicesgt; + .../pre + +p + The codelt;sourcegt;/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 codemode='server'/code and codemode='client'/code + are supported. + vhost-user requires the virtio model type, thus the + codelt;modelgt;/code element is mandatory. +/p + h4a name=elementsInputInput 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 +valuevhostuser/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 +valuenull/value +valuevc/value +valuepty/value +valuedev/value +valuefile/value +valuepipe/value +valuestdio/value +valueudp/value +valuetcp/value +valueunix/value +valuespicevmc/value +valuespiceport/value +valuenmdm/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 +valueserver/value +valueclient/value + /choice +/attribute +empty/ + /element +/optional +ref name=interface-options/ + /interleave +/group +group + attribute name=type valuenetwork/value /attribute interleave diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args new file mode 100644 index 000..cc66ec3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
[libvirt] [PATCH v2] support for QEMU vhost-user
This patch 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 changes from v1: * addressed comments * removed unnecessary checks * series merged in a single patch The previous version of this patch can be found at: http://www.redhat.com/archives/libvir-list/2014-July/msg00111.html Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/formatdomain.html.in | 34 + docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 87 ++ src/conf/domain_conf.h | 10 ++- src/libxl/libxl_conf.c | 1 + src/lxc/lxc_process.c | 1 + src/qemu/qemu_command.c| 63 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 + 13 files changed, 267 insertions(+), 2 deletions(-) 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=sinceSince 0.9.5/span /p +h5a name=elementVhostuservhost-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 + ... + lt;devicesgt; +lt;interface type='vhostuser'gt; + lt;source type='unix' path='/tmp/vhost.sock' mode='server'gt; + lt;/sourcegt; + lt;mac address='52:54:00:3b:83:1a'gt; + lt;/macgt; + lt;model type='virtio'gt; + lt;/modelgt; +lt;/interfacegt; + lt;/devicesgt; + .../pre + +p + The codelt;sourcegt;/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 codemode='server'/code and codemode='client'/code + are supported. + vhost-user requires the virtio model type, thus the + codelt;modelgt;/code element is mandatory. +/p + h4a name=elementsInputInput devices/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..c9c02b6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,31 @@ /group group attribute name=type +valuevhostuser/value + /attribute + interleave + element name=source +attribute name=type + choice +valueunix/value + /choice +/attribute +attribute name=path + ref name=absFilePath/ +/attribute +attribute name=mode + choice +valueserver/value +valueclient/value + /choice +/attribute +empty/ + /element +ref name=interface-options/ + /interleave +/group +group + attribute name=type valuenetwork/value /attribute interleave diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8df43b7..fb286c6 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
Re: [libvirt] [PATCH 0/4] support for QEMU vhost-user
ping :) On Wed, Jul 2, 2014 at 3:20 PM, Michele Paolino m.paol...@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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] docs: formatdomain.html fixes
Fixed some XML tags in the formatdomain page. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/formatdomain.html.in | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 901c26a..b69da4c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3856,7 +3856,7 @@ qemu-kvm -net nic,model=? /dev/null lt;outbound average='128' peak='256' burst='256'/gt; lt;/bandwidthgt;/b lt;/interfacegt; - lt;devicesgt; + lt;/devicesgt; .../pre p @@ -3888,7 +3888,7 @@ qemu-kvm -net nic,model=? /dev/null blt;/vlangt;/b ... lt;/interfacegt; - lt;devicesgt; + lt;/devicesgt; .../pre p @@ -3932,7 +3932,7 @@ qemu-kvm -net nic,model=? /dev/null lt;target dev='vnet0'/gt; blt;link state='down'/gt;/b lt;/interfacegt; - lt;devicesgt; + lt;/devicesgt; .../pre p @@ -4905,7 +4905,7 @@ qemu-kvm -net nic,model=? /dev/null lt;devicesgt; lt;sound model='ich6'gt; lt;codec type='micro'/gt; -lt;sound/gt; +lt;/soundgt; lt;/devicesgt; .../pre -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] docs: Fix broken link in the HACKING page
The link to the page how to get your code into an open source project has been fixed. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/hacking.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 9456520..6a92f46 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -260,7 +260,7 @@ p There is more on this subject, including lots of links to background reading on the subject, on - a href=http://et.redhat.com/~rjones/how-to-supply-code-to-open-source-projects/; + a href=http://people.redhat.com/rjones/how-to-supply-code-to-open-source-projects/; Richard Jones' guide to working with open source projects/a. /p -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] docs: hacking and formatdomain fixes
replaced link in haking.html.in and fixed XML tags in formatdomain.html.in Michele Paolino (2): docs: Fix broken link in the HACKING page docs: formatdomain.html fixes docs/formatdomain.html.in | 8 docs/hacking.html.in | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] vhost-user support: lxc,xenxs,uml
vhost-user is a qemu feature. Initial support for lxc,uml and xenxs added. Signed-off-by: Michele Paolino m.paol...@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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] vhost-user support: qemu command-line
Build the qemu-command line for vhost-user. Signed-off-by: Michele Paolino m.paol...@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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] vhost-user support: domain configuration
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.paol...@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
[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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] vhost-user support: tests and docs
This patch adds the test files and the documentation for vhost-user. Signed-off-by: Michele Paolino m.paol...@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=sinceSince 0.9.5/span /p +h5a name=elementVhostuservhost-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 + ... + lt;devicesgt; +lt;interface type='vhostuser'gt; + lt;source type='unix' path='/tmp/vhost.sock' mode='server'gt; + lt;/sourcegt; + lt;mac address='52:54:00:3b:83:1a'gt; + lt;/macgt; + lt;model type='virtio'gt; + lt;/modelgt; +lt;/interfacegt; + lt;/devicesgt; + .../pre + +p + The codelt;sourcegt;/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 codemode='server'/code and codemode='client'/code + are supported. + vhost-user requires the virtio model type, thus the + codelt;modelgt;/code element is mandatory. +/p + h4a name=elementsInputInput 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 +valuevhostuser/value + /attribute + interleave +optional + element name=source +attribute name=type + choice +valuenull/value +valuevc/value +valuepty/value +valuedev/value +valuefile/value +valuepipe/value +valuestdio/value +valueudp/value +valuetcp/value +valueunix/value +valuespicevmc/value +valuespiceport/value +valuenmdm/value + /choice +/attribute +attribute name=path + ref name=absFilePath/ +/attribute +attribute name=mode + choice +valueserver/value +valueclient/value + /choice +/attribute +empty/ + /element +/optional +ref name=interface-options/ + /interleave +/group +group + attribute name=type valuenetwork/value /attribute interleave diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args new file mode 100644 index 000..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 000..b49d48e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml @@ -0,0 +1,33 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-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
[libvirt] [PATCH 0/2] memdev device: add share argument for type=file
This patch enables the possibility to run a qemu virtual machine with the share option for the memory-backend-file. The xml description looks like: memdev type='file' share='on' nameram0/name source mem-path='/hugetlbfs'/ capacity unit='MiB'1024/capacity /memdev This work is based on the previous work of Chen Fan[1]. We are aware of the existing conflict with the previous numa patches[2]. We are sharing this because it is a dependency for some use cases of the qemu vhost-user support(e.g. snabbswitch). [1] http://www.redhat.com/archives/libvir-list/2014-June/msg01195.html [2] http://www.redhat.com/archives/libvir-list/2014-June/msg00201.html Michele Paolino (2): Add share argument to memdev devices(type=file) Documentation and test for the share argument in memdev device docs/formatdomain.html.in | 7 +++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 16 src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c| 3 +++ tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml | 2 +- 7 files changed, 32 insertions(+), 2 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Add share argument to memdev devices(type=file)
Add the share option in the XML parser and in the memdev domain structure. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- src/conf/domain_conf.c | 16 src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 3 +++ 3 files changed, 20 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 465a223..e73b6f5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9250,6 +9250,13 @@ virDomainMemDevDefParseXML(xmlNodePtr node, goto error; } +if ((tmp = virXMLPropString(node, share)) != NULL) { +if (STREQ(tmp, on)) +def-share = true; +VIR_FREE(tmp); +} + if ((tmp = virXMLPropString(node, merge)) != NULL) { if (STREQ(tmp, yes)) def-merge = true; @@ -9297,6 +9304,13 @@ virDomainMemDevDefParseXML(xmlNodePtr node, goto error; } +if (def-type == VIR_DOMAIN_MEMDEV_RAM def-share) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(The share argument should be not specified when + memory device type is 'ram')); +goto error; +} + def-hostnodes = virXPathString(string(./source/@host-nodes), ctxt); policy = virXPathString(string(./source/@policy), ctxt); @@ -16434,6 +16448,8 @@ virDomainMemDevDefFormat(virBufferPtr buf, } virBufferAsprintf(buf, memdev type='%s', type); +if (def-type == VIR_DOMAIN_MEMDEV_FILE def-share) +virBufferAddLit(buf, share='on'); if (def-merge) virBufferAddLit(buf, merge='yes'); if (def-dump) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 06e6781..eb5bad7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1508,6 +1508,7 @@ struct _virDomainMemDevDef { unsigned long long capacity; /* bytes */ +bool share; bool merge; bool dump; bool prealloc; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index db6717f..25e5b3c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4631,6 +4631,9 @@ qemuBuildMemObjectStr(virDomainMemDevDefPtr dev, if (dev-type == VIR_DOMAIN_MEMDEV_FILE dev-mempath) virBufferAsprintf(buf, ,mem-path=%s, dev-mempath); +if (dev-type == VIR_DOMAIN_MEMDEV_FILE dev-share) +virBufferAsprintf(buf, ,share=on); + if (dev-hostnodes) virBufferAsprintf(buf, ,host-nodes=%s, dev-hostnodes); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Documentation and test for the share argument in memdev device
This patch adds documentation and test for the memdev device (type=file) share option. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/formatdomain.html.in | 7 +++ docs/schemas/domaincommon.rng | 3 +++ tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml | 2 +- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0bc0139..3f8bbee 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5307,6 +5307,13 @@ qemu-kvm -net nic,model=? /dev/null The optional prealloc attribute enables memory preallocation. /p /dd +dtcodeshare/code/dt +dd + p +The optional share attribute enables the share option for the backend +memory-backed-file(type = 'file'). + /p +/dd dtcodesource/code/dt ddThe source element describes the device as seen from the host. the optional codemem-path/code element is required when specified type = 'file'. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 709d13e..af51eee 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3157,6 +3157,9 @@ /choice /attribute optional +attribute name='share' + valueon/value +/attribute attribute name='merge' valueyes/value /attribute diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args index 3a1cd94..6c92194 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args @@ -1,7 +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 \ --object memory-backend-file,id=ram0,size=109568K,merge=yes,dump=yes,prealloc=yes,mem-path=/hugepages,\ +-object memory-backend-file,id=ram0,size=109568K,merge=yes,dump=yes,prealloc=yes,mem-path=/hugepages,share=on,\ host-nodes=0,policy=bind \ -object memory-backend-file,id=ram1,size=109568K,mem-path=/hugepages,host-nodes=0,policy=bind \ -smp 16 -numa node,nodeid=0,cpus=0-7,memdev=ram0 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml index 03e3a28..d30ee80 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml @@ -21,7 +21,7 @@ on_crashdestroy/on_crash devices emulator/usr/bin/qemu/emulator -memdev type='file' merge='yes' dump='yes' prealloc='yes' +memdev type='file' share='on' merge='yes' dump='yes' prealloc='yes' nameram0/name capacity unit='KiB'109550/capacity source mem-path='/hugepages' host-nodes='0' policy='bind'/ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add support for error messages greater than 1024 characters
Guys, as for this patch, is it ACKable? are there suggested modifications? Michele On 16/12/2013 16:00, Daniel P. Berrange wrote: On Mon, Dec 16, 2013 at 09:50:30AM -0500, Cole Robinson wrote: On 12/16/2013 04:27 AM, Laine Stump wrote: On 12/14/2013 07:15 PM, Cole Robinson wrote: On 12/11/2013 03:33 PM, Michele Paolino wrote: In libvirt, the default error message length is 1024 bytes. This is not enough for qemu to print long error messages such as the list of supported ARM machine models (more than 1700 chars). This is raised when the machine entry in the XML file is wrong, but there may be now or in future other verbose error messages. The above patch enables libvirt to print error messages 1024 for qemu. Signed-off-by: Michele Paolinom.paol...@virtualopensystems.com --- src/qemu/qemu_process.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bd9546e..c2e2136 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1904,10 +1904,25 @@ cleanup: * a possible read of the fd in the monitor code doesn't influence this * error delivery option */ ignore_value(lseek(logfd, pos, SEEK_SET)); -qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true); -virReportError(VIR_ERR_INTERNAL_ERROR, - _(process exited while connecting to monitor: %s), - buf); +len = qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true); + +/* virReportError error buffer is limited to 1024 byte*/ +if (len 1024){ +virReportError(VIR_ERR_INTERNAL_ERROR, +_(process exited while connecting to monitor: %s), +buf); +} else { + if (STRPREFIX(buf, Supported machines are:)) + virReportError(VIR_ERR_INTERNAL_ERROR, + _(process exited while connecting to monitor: + please check machine model)); + else + virReportError(VIR_ERR_INTERNAL_ERROR, + _(process exited while connecting to monitor)); + + VIR_ERROR(%s, buf); +} + ret = -1; } This kind of error scraping is a slipper slop IMO, and for this particular case I don't think it's even that interesting. I definitely agree with that. Libvirt already parses the machine types for each qemu emulator and reports it in virsh capabilities XML. Seems reasonable that we could validate the XML machine type at define time and catch an invalid machine type error long before we even try and start the VM. Do we really want to refuse to define a particular guest just because the host where we're defining it doesn't currently support the given machine type? That's yet another slippery slope - for example, should we also be validating all the devices in hostdev at definition time? I think the proper time to do that validation is at domain start time when we should have the proper qemu binary (and necessary drivers/hardware) available. My suggestion for define time was because IIRC that's where we do the capabilities arch + os type + domain type validation as well. Doing validation at that level is nice because we don't need to duplicate it in each driver, but it also sucks when qemu is accidentally uninstalled and libvirt is restarted, it now refuses to parse all those pre-existing qemu/kvm configs, and virsh list --all is empty to the great confusion of users. Yeah, that is actually quite a big problem I'd like us to fix one day by moving that validation out of the define stage into the start stage Daniel -- *Michele Paolino*, Virtualization RD Engineer Virtual Open Systems /Open Source KVM Virtualization Developments/ /Multicore Systems Virtualization Porting Services/ Web/:/www.virtualopensystems. http://www.virtualopensystems.com/com http://www.virtualopensystems.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add support for error messages greater than 1024 characters
On 16/12/2013 16:00, Daniel P. Berrange wrote: On Mon, Dec 16, 2013 at 09:50:30AM -0500, Cole Robinson wrote: On 12/16/2013 04:27 AM, Laine Stump wrote: On 12/14/2013 07:15 PM, Cole Robinson wrote: On 12/11/2013 03:33 PM, Michele Paolino wrote: In libvirt, the default error message length is 1024 bytes. This is not enough for qemu to print long error messages such as the list of supported ARM machine models (more than 1700 chars). This is raised when the machine entry in the XML file is wrong, but there may be now or in future other verbose error messages. The above patch enables libvirt to print error messages 1024 for qemu. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- src/qemu/qemu_process.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bd9546e..c2e2136 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1904,10 +1904,25 @@ cleanup: * a possible read of the fd in the monitor code doesn't influence this * error delivery option */ ignore_value(lseek(logfd, pos, SEEK_SET)); -qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true); -virReportError(VIR_ERR_INTERNAL_ERROR, - _(process exited while connecting to monitor: %s), - buf); +len = qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true); + +/* virReportError error buffer is limited to 1024 byte*/ +if (len 1024){ +virReportError(VIR_ERR_INTERNAL_ERROR, +_(process exited while connecting to monitor: %s), +buf); +} else { + if (STRPREFIX(buf, Supported machines are:)) + virReportError(VIR_ERR_INTERNAL_ERROR, + _(process exited while connecting to monitor: + please check machine model)); + else + virReportError(VIR_ERR_INTERNAL_ERROR, + _(process exited while connecting to monitor)); + + VIR_ERROR(%s, buf); +} + ret = -1; } This kind of error scraping is a slipper slop IMO, and for this particular case I don't think it's even that interesting. I definitely agree with that. Libvirt already parses the machine types for each qemu emulator and reports it in virsh capabilities XML. Seems reasonable that we could validate the XML machine type at define time and catch an invalid machine type error long before we even try and start the VM. Do we really want to refuse to define a particular guest just because the host where we're defining it doesn't currently support the given machine type? That's yet another slippery slope - for example, should we also be validating all the devices in hostdev at definition time? I think the proper time to do that validation is at domain start time when we should have the proper qemu binary (and necessary drivers/hardware) available. My suggestion for define time was because IIRC that's where we do the capabilities arch + os type + domain type validation as well. Doing validation at that level is nice because we don't need to duplicate it in each driver, but it also sucks when qemu is accidentally uninstalled and libvirt is restarted, it now refuses to parse all those pre-existing qemu/kvm configs, and virsh list --all is empty to the great confusion of users. Indeed. In my opinion start time is the best choice because it's more flexible and gives to the user the possibility to catch a more precise error message. Yeah, that is actually quite a big problem I'd like us to fix one day by moving that validation out of the define stage into the start stage Daniel -- *Michele Paolino*, Virtualization RD Engineer Virtual Open Systems /Open Source KVM Virtualization Developments/ /Multicore Systems Virtualization Porting Services/ Web/:/www.virtualopensystems. http://www.virtualopensystems.com/com http://www.virtualopensystems.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix libvirt alignment on arm platforms
On 13/12/2013 08:54, Michal Privoznik wrote: On 12.12.2013 20:27, Eric Blake wrote: On 12/12/2013 11:55 AM, Michal Privoznik wrote: int detail; -}; +} ATTRIBUTE_PACKED; (What an ancient e-mail :-) ) 1970 was how many years before libvirt was even started? Indeed, this is a 43 years old thread :-) I've had some problem with the ntpd deamon on the OMAP5 yesterday. For this reason I've resent the patch with the prefix [resend]. I've raised the problem here: https://www.redhat.com/archives/libvir-list/2013-December/msg00635.html And Eric replied suggesting a fix: https://www.redhat.com/archives/libvir-list/2013-December/msg00662.html But I must say I like your approach more. Eric? I still think ATTRIBUTE_PACKED in a parent class is wrong; it forces the children to be packed, and may make it impossible to implement atomic operations on a child member that was supposed to be aligned. I'd much rather fix the alignment in the parent class using portable constructs than a compiler-specific non-alignment directive. Oh right. The locking could be impossible, because if two struct members are packed in a word, e.g.: struct S { bool a; bool b; ... }; then modifying S.a requires whole struct to be fetched from mem, modifying S.a a and storing it back. However, even if our locking code to protect say S.b was written well, S.b can still get changed without lock. As S.a is being changed, another thread changes S.b, but storing S.a means overwriting S.b without lock held. I have not thought of that. So NACK then - we need Eric's approach. I saw the Eric's patch. I will test it today. Michal -- *Michele Paolino*, Virtualization RD Engineer Virtual Open Systems /Open Source KVM Virtualization Developments/ /Multicore Systems Virtualization Porting Services/ Web/:/www.virtualopensystems. http://www.virtualopensystems.com/com http://www.virtualopensystems.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] fix libvirt alignment on arm platforms
With the changes added by the latest commits (e.g. 8a29ffcf9aee45b61a0a12ee45c656cfd52333e8) related to new events feature v2, we are unable to compile libvirt on ARM target (OMAP5). The error is due to alignment: conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc': conf/domain_event.c:1198:30: error: cast increases required alignment of target type [-Werror=cast-align] conf/domain_event.c:1314:34: error: cast increases required alignment of target type [-Werror=cast-align] cc1: all warnings being treated as errors Using ATTRIBUTE_PACKED we force a structure to not follow architecture and compiler best alignments. --- src/conf/domain_event.c | 20 ++-- src/conf/network_event.c|2 +- src/conf/object_event_private.h |2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 64035f7..7d58367 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -76,7 +76,7 @@ struct _virDomainEventLifecycle { int type; int detail; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventLifecycle virDomainEventLifecycle; typedef virDomainEventLifecycle *virDomainEventLifecyclePtr; @@ -84,7 +84,7 @@ struct _virDomainEventRTCChange { virDomainEvent parent; long long offset; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventRTCChange virDomainEventRTCChange; typedef virDomainEventRTCChange *virDomainEventRTCChangePtr; @@ -92,7 +92,7 @@ struct _virDomainEventWatchdog { virDomainEvent parent; int action; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventWatchdog virDomainEventWatchdog; typedef virDomainEventWatchdog *virDomainEventWatchdogPtr; @@ -103,7 +103,7 @@ struct _virDomainEventIOError { char *devAlias; int action; char *reason; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventIOError virDomainEventIOError; typedef virDomainEventIOError *virDomainEventIOErrorPtr; @@ -113,7 +113,7 @@ struct _virDomainEventBlockJob { char *path; int type; int status; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventBlockJob virDomainEventBlockJob; typedef virDomainEventBlockJob *virDomainEventBlockJobPtr; @@ -125,7 +125,7 @@ struct _virDomainEventGraphics { virDomainEventGraphicsAddressPtr remote; char *authScheme; virDomainEventGraphicsSubjectPtr subject; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventGraphics virDomainEventGraphics; typedef virDomainEventGraphics *virDomainEventGraphicsPtr; @@ -136,7 +136,7 @@ struct _virDomainEventDiskChange { char *newSrcPath; char *devAlias; int reason; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventDiskChange virDomainEventDiskChange; typedef virDomainEventDiskChange *virDomainEventDiskChangePtr; @@ -145,7 +145,7 @@ struct _virDomainEventTrayChange { char *devAlias; int reason; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventTrayChange virDomainEventTrayChange; typedef virDomainEventTrayChange *virDomainEventTrayChangePtr; @@ -154,7 +154,7 @@ struct _virDomainEventBalloonChange { /* In unit of 1024 bytes */ unsigned long long actual; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventBalloonChange virDomainEventBalloonChange; typedef virDomainEventBalloonChange *virDomainEventBalloonChangePtr; @@ -162,7 +162,7 @@ struct _virDomainEventDeviceRemoved { virDomainEvent parent; char *devAlias; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventDeviceRemoved virDomainEventDeviceRemoved; typedef virDomainEventDeviceRemoved *virDomainEventDeviceRemovedPtr; diff --git a/src/conf/network_event.c b/src/conf/network_event.c index 4a02523..1ffcf6c 100644 --- a/src/conf/network_event.c +++ b/src/conf/network_event.c @@ -32,7 +32,7 @@ struct _virNetworkEventLifecycle { virObjectEvent parent; int type; -}; +} ATTRIBUTE_PACKED; typedef struct _virNetworkEventLifecycle virNetworkEventLifecycle; typedef virNetworkEventLifecycle *virNetworkEventLifecyclePtr; diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h index f41f432..34cd902 100644 --- a/src/conf/object_event_private.h +++ b/src/conf/object_event_private.h @@ -73,7 +73,7 @@ struct _virObjectEvent { virObject parent; int eventID; virObjectMeta meta; -}; +}ATTRIBUTE_PACKED; virClassPtr virClassForObjectEvent(void); -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt support for KVM/ARM on armv7l hardware
Hello, I'm working on it. All the basic features are already there, so it's possible to run a KVM-ARM vm using libvirt. The platform that I'm currently testing is TI OMAP5. Regards, Michele On 12/12/2013 12:33, Daniel P. Berrange wrote: On Mon, Dec 09, 2013 at 08:02:31PM +0800, 林佳緯 wrote: I have found Libvirt supporting for armv7b in http://libvirt.org/news.html. But armv7l is not found there. Does Libvirt support run KVM/ARM for hardware virtualization on armv7l platform? It might work - AFAIK no one has tested it, but unless something is radically different on armv7l vs armv7b i doubt anything will break. Daniel -- *Michele Paolino*, Virtualization RD Engineer Virtual Open Systems /Open Source KVM Virtualization Developments/ /Multicore Systems Virtualization Porting Services/ Web/:/www.virtualopensystems. http://www.virtualopensystems.com/com http://www.virtualopensystems.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [resend] fix libvirt alignment on arm platforms
With the changes added by the latest commits (e.g. 8a29ffcf9aee45b61a0a12ee45c656cfd52333e8) related to new events feature v2, we are unable to compile libvirt on ARM target (OMAP5). The error is due to alignment: conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc': conf/domain_event.c:1198:30: error: cast increases required alignment of target type [-Werror=cast-align] conf/domain_event.c:1314:34: error: cast increases required alignment of target type [-Werror=cast-align] cc1: all warnings being treated as errors Using ATTRIBUTE_PACKED we force a structure to not follow architecture and compiler best alignments. --- src/conf/domain_event.c | 20 ++-- src/conf/network_event.c|2 +- src/conf/object_event_private.h |2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 64035f7..7d58367 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -76,7 +76,7 @@ struct _virDomainEventLifecycle { int type; int detail; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventLifecycle virDomainEventLifecycle; typedef virDomainEventLifecycle *virDomainEventLifecyclePtr; @@ -84,7 +84,7 @@ struct _virDomainEventRTCChange { virDomainEvent parent; long long offset; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventRTCChange virDomainEventRTCChange; typedef virDomainEventRTCChange *virDomainEventRTCChangePtr; @@ -92,7 +92,7 @@ struct _virDomainEventWatchdog { virDomainEvent parent; int action; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventWatchdog virDomainEventWatchdog; typedef virDomainEventWatchdog *virDomainEventWatchdogPtr; @@ -103,7 +103,7 @@ struct _virDomainEventIOError { char *devAlias; int action; char *reason; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventIOError virDomainEventIOError; typedef virDomainEventIOError *virDomainEventIOErrorPtr; @@ -113,7 +113,7 @@ struct _virDomainEventBlockJob { char *path; int type; int status; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventBlockJob virDomainEventBlockJob; typedef virDomainEventBlockJob *virDomainEventBlockJobPtr; @@ -125,7 +125,7 @@ struct _virDomainEventGraphics { virDomainEventGraphicsAddressPtr remote; char *authScheme; virDomainEventGraphicsSubjectPtr subject; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventGraphics virDomainEventGraphics; typedef virDomainEventGraphics *virDomainEventGraphicsPtr; @@ -136,7 +136,7 @@ struct _virDomainEventDiskChange { char *newSrcPath; char *devAlias; int reason; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventDiskChange virDomainEventDiskChange; typedef virDomainEventDiskChange *virDomainEventDiskChangePtr; @@ -145,7 +145,7 @@ struct _virDomainEventTrayChange { char *devAlias; int reason; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventTrayChange virDomainEventTrayChange; typedef virDomainEventTrayChange *virDomainEventTrayChangePtr; @@ -154,7 +154,7 @@ struct _virDomainEventBalloonChange { /* In unit of 1024 bytes */ unsigned long long actual; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventBalloonChange virDomainEventBalloonChange; typedef virDomainEventBalloonChange *virDomainEventBalloonChangePtr; @@ -162,7 +162,7 @@ struct _virDomainEventDeviceRemoved { virDomainEvent parent; char *devAlias; -}; +} ATTRIBUTE_PACKED; typedef struct _virDomainEventDeviceRemoved virDomainEventDeviceRemoved; typedef virDomainEventDeviceRemoved *virDomainEventDeviceRemovedPtr; diff --git a/src/conf/network_event.c b/src/conf/network_event.c index 4a02523..1ffcf6c 100644 --- a/src/conf/network_event.c +++ b/src/conf/network_event.c @@ -32,7 +32,7 @@ struct _virNetworkEventLifecycle { virObjectEvent parent; int type; -}; +} ATTRIBUTE_PACKED; typedef struct _virNetworkEventLifecycle virNetworkEventLifecycle; typedef virNetworkEventLifecycle *virNetworkEventLifecyclePtr; diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h index f41f432..34cd902 100644 --- a/src/conf/object_event_private.h +++ b/src/conf/object_event_private.h @@ -73,7 +73,7 @@ struct _virObjectEvent { virObject parent; int eventID; virObjectMeta meta; -}; +}ATTRIBUTE_PACKED; virClassPtr virClassForObjectEvent(void); -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/34] network events feature v2
Sorry, I noticed just now that here you are discussing about the problem with the compilation of libvirt on ARM platforms. An alternative solution can be ATTRIBUTE_PACKED. I've sent right now to the ML a patch based on this. Michele On 12/12/2013 17:11, Daniel P. Berrange wrote: On Wed, Dec 11, 2013 at 12:28:32PM -0700, Eric Blake wrote: On 12/11/2013 12:15 PM, Eric Blake wrote: struct _virObjectEvent { virObject parent; int eventID; virObjectMeta meta; }; Only has alignment specified by virObject (which in turn is unsigned int, int, void*), struct _virObject { unsigned int magic; int refs; virClassPtr klass; }; I think one possible solution would be as simple as altering src/util/virobject.h to change 'magic' from 'unsigned int' to 'unsigned long long' - then ALL virObject structs will be forcefully aligned to the worst case between void* and long long, so that any subclass can use long long without requiring stricter alignment than the parent class, and so that downcasting code like domain_event.c no longer warns. But it does make every object consume more memory on 64-bit platforms (from 16 bytes into 24 bytes), is that okay? Or maybe even change _virObject to contain a union: struct _virObject { union { long long align; struct { unsigned int magic; int refs; } s; } u; virClassPtr klass; } which keeps the size at 16 bytes on 64-bit platform, keeps things at 12 bytes on 32-bit platforms that don't care about long long alignment, and for ARM (*) would change things from 12 to 16 bytes with 8-byte alignment for the long long. Yeah, that means using obj-u.s.refs instead of obj-refs, but most code shouldn't be directly mucking with object-internal fields, so hopefully the fallout would be limited to just virobject.c (if only C99 had anonymous struct/unions; C11 does, but we don't require that yet). (*) Am I correct that your platform with the compile failure is a 32-bit ARM platform with 4-byte pointers? Because if it were 64-bit, then I would have guessed that an 8-byte pointer would already be forcing 8-byte alignment, such that use of 'long long' in a subclass wouldn't be warning about changed alignment. That seems reasonable to me - it makes sense that we should have our base object type be nicely aligned, instead of trying to fix this in the events code (and potentially anywhere else using objects in the future). Daniel -- *Michele Paolino*, Virtualization RD Engineer Virtual Open Systems /Open Source KVM Virtualization Developments/ /Multicore Systems Virtualization Porting Services/ Web/:/www.virtualopensystems. http://www.virtualopensystems.com/com http://www.virtualopensystems.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: add support for error messages greater than 1024 characters
In libvirt, the default error message length is 1024 bytes. This is not enough for qemu to print long error messages such as the list of supported ARM machine models (more than 1700 chars). This is raised when the machine entry in the XML file is wrong, but there may be now or in future other verbose error messages. The above patch enables libvirt to print error messages 1024 for qemu. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- src/qemu/qemu_process.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bd9546e..c2e2136 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1904,10 +1904,25 @@ cleanup: * a possible read of the fd in the monitor code doesn't influence this * error delivery option */ ignore_value(lseek(logfd, pos, SEEK_SET)); -qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true); -virReportError(VIR_ERR_INTERNAL_ERROR, - _(process exited while connecting to monitor: %s), - buf); +len = qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true); + +/* virReportError error buffer is limited to 1024 byte*/ +if (len 1024){ +virReportError(VIR_ERR_INTERNAL_ERROR, +_(process exited while connecting to monitor: %s), +buf); +} else { + if (STRPREFIX(buf, Supported machines are:)) + virReportError(VIR_ERR_INTERNAL_ERROR, + _(process exited while connecting to monitor: + please check machine model)); + else + virReportError(VIR_ERR_INTERNAL_ERROR, + _(process exited while connecting to monitor)); + + VIR_ERROR(%s, buf); +} + ret = -1; } -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virSocketAddrIsWildcard: Drop IN6_IS_ADDR_UNSPECIFIED
Hi Michal, The IN6_IS_ADDR_UNSPECIFIED() function needs a struct in6_addr as argument. So changing the line in this way: -return IN6_IS_ADDR_UNSPECIFIED(addr-data.inet6.sin6_addr.s6_addr); +return IN6_IS_ADDR_UNSPECIFIED(addr-data.inet6.sin6_addr); it should work. Regards, On Mon, Jun 10, 2013 at 12:31 PM, Michal Privoznik mpriv...@redhat.comwrote: There's this macro IN6_IS_ADDR_UNSPECIFIED which seems to be portable, but it is not. On other architectures many errors are produced, e.g. on my ARM box I get: CC libvirt_util_la-virsocketaddr.lo util/virsocketaddr.c: In function 'virSocketAddrIsWildcard': util/virsocketaddr.c:244:16: error: cast increases required alignment of target type [-Werror=cast-align] util/virsocketaddr.c: At top level: cc1: error: unrecognized command line option -Wno-unused-command-line-argument [-Werror] cc1: all warnings being treated as errors Hence, we should drop its usage and move to memcmp instead. --- src/util/virsocketaddr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index e84c58e..1cefda7 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -241,7 +241,8 @@ virSocketAddrIsWildcard(const virSocketAddrPtr addr) return memcmp(addr-data.inet4.sin_addr.s_addr, tmp, sizeof(addr-data.inet4.sin_addr.s_addr)) == 0; case AF_INET6: -return IN6_IS_ADDR_UNSPECIFIED(addr-data.inet6.sin6_addr.s6_addr); +return memcmp(addr-data.inet6.sin6_addr.s6_addr, in6addr_any, + sizeof(addr-data.inet6.sin6_addr.s6_addr)) == 0; } return false; } -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- *Michele Paolino ** *Virtual Open Systems* **Open Source KVM Virtualization Developments Multicore Systems Virtualization Porting Services *Web*:* *www.virtualopensystems.com* -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ARM libvirt compiling error
Hello, I'm trying to set up a development environment on an Arndale (ARM Samsung Exynos 5250) board to work on sVirt. I'm using Debian 7.0, I've downloaded the source code from GIT and than: ./autogen.sh --prefix=$HOME/usr make but in the middle of make execution, the program fails with this error: conf/domain_conf.c: In function 'virDomainHostdevDefParseXML': conf/domain_conf.c:3915:36: error: 'next_unit' may be used uninitialized in this function [-Werror=uninitialized] conf/domain_conf.c:3886:9: note: 'next_unit' was declared here conf/domain_conf.c: At top level: cc1: error: unrecognized command line option -Wno-unused-command-line-argument [-Werror] I've solved this problem simply initializing the next_unit variable (file src/conf/domain_conf.c, line 3886). This is the diff between the original file and the modified one: 3886c3886 int next_unit; --- int next_unit = -1; Another way to solve is obviously with --disable-werror, but I guess this is not the best way. My gcc version is 4.6.3 (Debian 4.6.3-14), kernel version is 3.8.0-rc4. Maybe it's only a compiler problem, but can anyone confirm this? Is it worth to submit a new bug report/patch the source? Regards, Michele -- *Michele Paolino ** *Virtual Open Systems* **Open Source KVM Virtualization Developments Multicore Systems Virtualization Porting Services *Web*:* *www.virtualopensystems.com* -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list