Re: [PATCH RFC] Add support for QEMU vhost-user feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]