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

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(a)redhat.com&gt; --- 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;

+ + /* 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;

+ } + } + }

+ ret = 0;