[libvirt] [PATCH 6/7] qemu: Support newer ivshmem device variants
Daniel P. Berrange
berrange at redhat.com
Wed Sep 21 15:01:23 UTC 2016
On Wed, Sep 21, 2016 at 03:30:55PM +0200, Martin Kletzander wrote:
> QEMU added support for ivshmem-plain and ivshmem-doorbell. Those are
> reworked varians of legacy ivshmem that are compatible from the guest
> POV, but not from host's POV and have sane specification and handling.
>
> Details about the newer device type can be found in qemu's commit
> 5400c02b90bb:
>
> http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb
>
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
> docs/formatdomain.html.in | 5 +-
> src/qemu/qemu_command.c | 109 ++++++++++++++++++++-
> src/qemu/qemu_command.h | 10 ++
> .../qemuxml2argv-shmem-plain-doorbell.args | 46 +++++++++
> .../qemuxml2argv-shmem-plain-doorbell.xml | 1 +
> tests/qemuxml2argvtest.c | 3 +
> 6 files changed, 172 insertions(+), 2 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
> create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index b9163584d9f5..c614caf5f47f 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6740,7 +6740,10 @@ qemu-kvm -net nic,model=? /dev/null
>
> <p>
> A shared memory device allows to share a memory region between
> - different virtual machines and the host.
> + different virtual machines and the host. Note that memory in
> + this region will be migrated, if that's not desired, the device
> + needs to be unplugged before migration and plugged back again
> + after that.
> <span class="since">Since 1.2.10, QEMU and KVM only</span>
> </p>
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 2130a7e2e8cf..3f7dfc4a46b9 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8550,6 +8550,44 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def,
> return NULL;
> }
>
> +char *
> +qemuBuildShmemDevStr(virDomainDefPtr def,
> + virDomainShmemDefPtr shmem,
> + virQEMUCapsPtr qemuCaps)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + if (shmem->server.enabled) {
> + virBufferAddLit(&buf, "ivshmem-doorbell");
> + virBufferAsprintf(&buf, ",id=%s,chardev=char%s",
> + shmem->info.alias, shmem->info.alias);
> + if (shmem->msi.vectors)
> + virBufferAsprintf(&buf, ",vectors=%u", shmem->msi.vectors);
> + if (shmem->msi.ioeventfd)
> + virBufferAsprintf(&buf, ",ioeventfd=%s",
> + virTristateSwitchTypeToString(shmem->msi.ioeventfd));
> + } else {
> + virBufferAddLit(&buf, "ivshmem-plain");
> + virBufferAsprintf(&buf, ",id=%s,memdev=shmmem-%s",
> + shmem->info.alias, shmem->info.alias);
> +
> + /* For now we default to master=on, users are adviced to
> + * unplug the ivshmem if they don't want to migrate the data
> + * in the shmem (see docs). */
> + virBufferAddLit(&buf, ",master=on");
> + }
> +
> + if (qemuBuildDeviceAddressStr(&buf, def, &shmem->info, qemuCaps) < 0) {
> + virBufferFreeAndReset(&buf);
> + return NULL;
> + }
> +
> + if (virBufferCheckError(&buf) < 0)
> + return NULL;
> +
> + return virBufferContentAndReset(&buf);
> +}
> @@ -8579,6 +8662,18 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
> virQEMUCapsPtr qemuCaps)
> {
> char *devstr = NULL;
> + bool legacy = false;
> +
> + if (!qemuDomainSupportsNonLegacyShmem(qemuCaps, shmem)) {
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("shmem device is not supported with this "
> + "QEMU binary"));
> + return -1;
> + }
> +
> + legacy = true;
> + }
>
> if (shmem->size) {
> /*
> @@ -8606,8 +8701,14 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
> return -1;
> }
>
> - if (!(devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps)))
> + if (legacy)
> + devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps);
> + else
> + devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps);
I'm not a fan of the idea of silently picking a different device
for the guest behind the applications back. By not exposing the
different device types with a "model" attribute, we miss a way
to report to the application which models are supported by the
QEMU they're using - eg via domain capabilities.
This in turn means the application doesn't know whether they're
getting the new or old version, and so don't know if they're going
to have working migration or not.
If we expanded the XML to include model, then application can
explicitly request the new model (which supports migration) and
know that they'll get a startup failure if not supported, as
opposed to silently falling back to the non-migratable version.
Also, it makes life hard for us if the ivshmem-plain device wants
to support use of the 'server' attribute in the future, as we will
then not know which to create.
We've often been burnt in the past by trying todo magic like this,
instead of explicitly representing stuff in the XML, so I think we
should be being explicit about ivshmem models here.
Of course, if we do add <model> support, we have to allow for it
to be missing for sake of upgrades. So there's a question of which
model we should select as the default, if not seen in the XML.
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 :|
More information about the libvir-list
mailing list