On Wed, May 28, 2014 at 10:46:27AM +0200, Luke Gorrie wrote:
> vhost-user is a networking backend based on unix domain sockets
> instead of tap devices and ioctl(). This makes it possible for
> userspace networking stacks (vswitches) to provide vhost-networking to
> guests.
>
> Signed-off-by: Luke Gorrie <luke@xxxxxxxx>
> ---
> docs/schemas/domaincommon.rng | 23 ++++++++
Also need to update docs/formatdomain.html.in to describe the
new XML syntax
> src/conf/domain_conf.c | 42 ++++++++++++++
> src/conf/domain_conf.h | 5 ++
> src/lxc/lxc_process.c | 1 +
> src/qemu/qemu_command.c | 66 +++++++++++++++++++++-
> src/uml/uml_conf.c | 5 ++
> src/xenxs/xen_sxpr.c | 1 +
> tests/qemuxml2argvdata/qemuxml2argv-hugepages.args | 2 +-
> 8 files changed, 142 insertions(+), 3 deletions(-)
You need to introduce a new data file in tests/qemuxml2argvdata/
and add it to tests/qemuxml2argvtest.c and tests/qemuxml2xmltest.c
to validate correct XML parsing, formatting and QEMU CLI arg
generation for the new NIC type.
> @@ -6804,6 +6817,25 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
> }
> break;
>
> + case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
> + if (vhostuser_socket == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("No <socket> 'path' attribute "
_( should be aligned with the 'VIR_' on the line above - it indented
2 more spaces.
> + "specified with <interface type='vhostuser'/>"));
> + goto error;
Indentation is wonky for the goto.
> + }
> + if (vhostuser_mode == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("No <socket> 'mode' attribute "
And here too
> + "specified with <interface type='vhostuser'/>"));
> + goto error;
And here too
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1d5bce6..755bf9d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7007,6 +7007,62 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
> }
>
> static int
> +qemuBuildVhostuserCommandLine(virCommandPtr cmd,
> + virDomainDefPtr def,
> + virDomainNetDefPtr net,
> + virQEMUCapsPtr qemuCaps)
> +{
> + virBuffer buf1 = VIR_BUFFER_INITIALIZER;
> + virBuffer buf2 = VIR_BUFFER_INITIALIZER;
> +
> + char* nic = NULL;
> +
> + if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("Netdev support unavailable"));
> + goto error;
> + }
> +
> + if (!qemuDomainSupportsNicdev(def, qemuCaps, net)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("Nicdev support unavailable"));
> + goto error;
> + }
> +
> + virBufferAsprintf(&buf1, "socket,id=char%s,path=%s%s",
> + net->info.alias, net->data.vhostuser.socket,
> + STRCASEEQ(net->data.vhostuser.mode, "server") ? ",server" : "");
Again these trailing lines should be aligned with the '&buf1' - eg as
is done in the line below:
> + virBufferAsprintf(&buf2, "type=vhost-user,id=host%s,chardev=char%s",
> + net->info.alias, net->info.alias);
> +
> + if (virBufferError(&buf1) || virBufferError(&buf2)) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + virCommandAddArgList(cmd, "-chardev", virBufferContentAndReset(&buf1),
> + NULL);
> + virCommandAddArgList(cmd, "-netdev", virBufferContentAndReset(&buf2),
> + 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);
> +
> + return 0;
> +
> +error:
> + virBufferFreeAndReset(&buf1);
> + virBufferFreeAndReset(&buf2);
> +
> + return -1;
> +}
> +
> +static int
> qemuBuildInterfaceCommandLine(virCommandPtr cmd,
> virQEMUDriverPtr driver,
> virConnectPtr conn,
> @@ -7029,6 +7085,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
> int actualType = virDomainNetGetActualType(net);
> size_t i;
>
> + if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
> + return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps);
> + }
No need for '{...}' on single line if()s
> +
> if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> /* NET_TYPE_HOSTDEV devices are really hostdev devices, so
> * their commandlines are constructed with other hostdevs.
> @@ -7374,8 +7434,10 @@ qemuBuildCommandLine(virConnectPtr conn,
> def->emulator);
> goto error;
> }
> - virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path",
> - cfg->hugepagePath, NULL);
> + virCommandAddArg(cmd, "-mem-path");
> + virCommandAddArgFormat(cmd, "%s,prealloc=on,share=%s",
> + cfg->hugepagePath,
> + def->mem.nosharepages ? "off" : "on");
This is totally bogus and should be removed from the patch.
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages.args
> index d42d9fc..5f8df71 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages.args
> @@ -1,5 +1,5 @@
> LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
> /usr/bin/qemu -S -M \
> -pc -m 214 -mem-prealloc -mem-path /dev/hugepages/libvirt/qemu -smp 1 \
> +pc -m 214 -mem-path /dev/hugepages/libvirt/qemu,prealloc=on,share=on -smp 1 \
> -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
> /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
This change is also bogus.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list