[libvirt] [PATCH 09/20] qemu: Save various defaults for shmem

[Peter Krempa]

On Thu, Sep 15, 2016 at 18:14:34 +0200, Martin Kletzander wrote:
> When we change the device used for the shared memory, it should not
> change the settings, so rather save them upfront then having problems
> later.
> 
> The only thing we are not saving is the role for old ivshmem and that's
> because we were not specifying it, which means role=auto and that is
> really bad idea to be using (due to various things).  However, we don't
> want to change the behaviour, so that's the reason for that.
> 
> Details for the defaults of the newer implementation 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>
> ---
>  src/qemu/qemu_domain.c                            | 33 +++++++++++++++++++++++
>  tests/qemuxml2argvdata/qemuxml2argv-shmem.args    |  2 +-
>  tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml |  3 ++-
>  3 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 3f16dbee2e6a..1fdbdf68fc8b 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2747,6 +2747,39 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>          }
>      }
> 
> +    if (dev->type == VIR_DOMAIN_DEVICE_SHMEM) {
> +        if (!dev->data.shmem->server.enabled) {
> +            /* The size is 4M if not specified */
> +            if (!dev->data.shmem->size)
> +                dev->data.shmem->size = 4 << 20;
> +            /* For old ivshmem we shouldn't change the role, the new
> +             * ones are peer by default, mostly to safeguard that
> +             * there should be no more than one master */
> +            if (!dev->data.shmem->role) {
> +                if (dev->data.shmem->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM)
> +                    dev->data.shmem->role = VIR_DOMAIN_SHMEM_ROLE_MASTER;
> +                else
> +                    dev->data.shmem->role = VIR_DOMAIN_SHMEM_ROLE_PEER;
> +            }
> +        } else {
> +            /* In QEMU, role doesn't make sense for server-side shmem */
> +            dev->data.shmem->role = VIR_DOMAIN_SHMEM_ROLE_DEFAULT;

Shouldn't this be VIR_DOMAIN_SHMEM_ROLE_PEER?

> +
> +            /* Defaults/Requirements for the newer device that we should save */
> +            if (dev->data.shmem->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL) {
> +                /* Size does not make much sense when claiming memory from
> +                 * the server and so the newer version doesn't support that */
> +                dev->data.shmem->size = 0;
> +
> +                /* Also they can only exist with MSI and ioeventfd is
> +                 * enabled unless specifically disabled */
> +                dev->data.shmem->msi.enabled = true;
> +                if (!dev->data.shmem->msi.ioeventfd)
> +                    dev->data.shmem->msi.ioeventfd = VIR_TRISTATE_SWITCH_ON;

This does not tweak the number of ioeventfd vectors, which will probably
remain 0.

> +            }
> +        }
> +    }

The above code does not check the following two incompatible configs:
1) device is VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN and server is enabled
2) device is VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL and server is disabled

> +
>      ret = 0;
> 

Peter