10:34 a.m.
On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan(a)redhat.com> wrote:
On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote:
>
> This patch documents XML elements used for support of ivshmem
> devices.
>
At first, I'd like to thank you for the proposal. There were numerous
requests for this, but since ivshmem is not known much, it's not easy
to get it in. I'll get to the details later.
You welcome. ;) Thanks for the review.
[...]
At first, we should be more generic about the name if we want other
hypervisors to benefit from it. The idea is that ivshmem is not
something new that nobody else than qemu can do. It's just a shared
memory, every other hypervisor out there can come up with something
similar to this, so we don't want this to be called ivshmem. One more
reason for that is that it doesn't just share memory between VMs, but
between host <-> guest too. Something like a 'shmem' should be fine,
I guess.
I agree with you, shmem is a better name for ivshmem.
In such a case, should ivshmem be renamed in QEMU ?
This would break compatibility with older versions of QEMU.
If we change ivshmem name in QEMU, we need to manage this case in libvirt.
So, I am not sure about this point.
I prolonged the paragraph to stress out this is not qemu-only
feature
(or might not be in the future) and we should be prepared for that.
Because of that, we should be more generic about more things than just
the name.
Another thing that bothers me (and bothered me with earlier
ivshmem-related proposals as well) is that role='(master|peer)' thing.
That thing does not mean squat for qemu and should be removed
completely. Looking at the code the only thing that role=peer
(non-default, by the way) does is that it disables migration...
That's it. That's another sign of the ivshmem code maturity inside
QEMU that we should keep in mind when designing the XML schema.
Ok. I added this role to be coherent with the initial proposals.
I agree with you, we should wait that live migration is properly
supported in ivshmem QEMU before adding any options related to that in
libvirt.
So I will remove this role to avoid adding confusions.
> Note: the ivshmem server needs to be launched before
> creating the VM. It's not done by libvirt.
>
This is a good temporary workaround, but keep in mind that libvirt
works remotely as well and for remote machines libvirt should be able
to do everything for you to be able to run the machine if necessary.
So even if it might not start the server now, it should in the future.
That should be at least differentiable by some parameter (maybe you do
it somewhere in the code somehow, I haven't got into that).
The new version of ivshmem server has not been accepted yet in QEMU.
I think it's too early to have an option to launch an ivshmem server or not.
I will prefer to focus on integrating these patches in libvirt first,
before adding a new feature to launch an ivhsmem server.
Are you ok with that ?
> - For ivshmem device not using an ivshmem server:
>
> <ivshmem use_server='no' role='peer'/>
> <source file='ivshmem1'/>
> <size unit='M'>32</size>
> </ivshmem>
>
> Note: the ivshmem shm needs to be created before
> creating the VM. It's not done by libvirt.
>
Isn't it done by qemu automatically? If it's not, the same as for the
ivshmem server applies.
Just checked the QEMU code, QEMU creates the file if it doesn't exist yet.
So my mistake ;)
I had one idea to deal with most of the problems (but adding a lot
of
code); let me outline that. From the global POV, we want something
that will fully work remotely, we want to be able to start it, stop
it, assign it to domains, etc. We might even migrate it in the
future, but that's another storyline. It must be generic enough, as
it can change a lot in the future. And so on. One way out of this
mess is to have yet another driver, let's call it shmem driver. That
driver would have APIs to define, undefine, ..., start and stop shared
memory regions. Those would have their own XML, and domain XML would
only have a device <shmem name='asdf_mem'/> or <shmem
uuid='...'/>
that would mean the domain will be started with a shared memory
region defined in the shmem driver. That way it could be shared even
between hypervisors.
It seems a nice idea to have a more generic way to share memory
between guests or host.
When can you share a spec or a RFC patch about it?
At first it seemed like an unreasonable solution, but the more I
think
about it the more I like it. It's not a requirement to get your
patches it, though, I just wanted to share this in case you (or anyone
else) find it compelling enough to try it.
I will keep focusing about ivshmem support as QEMU is supporting it for now.
I will be on holiday next week. Based on the the comments, I will send
a new version after my holidays.
Maxime