4:21 a.m.
On Thu, Aug 07, 2014 at 05:34:35PM +0200, Maxime Leroy wrote:
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.
No need to change anything in QEMU. That name is because libvirt
supports more than just QEMU. For example if we add support for that
in lxc/xen/bhyve/whatever, it might not be called ivshmem and we
should be generic.
> 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.
It is "supported" in a sense that it will migrate. But that doesn't
mean it will work. The data on destination host might not be there
and there might even be different data. The guest must count with
that, but that's another issue.
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 ?
There was a suggestion of implementing the non-server variant first
and expand it to the variant with server afterwards. That might be
the best solution because we'll have bit more time to see the
re-factoring differences in QEMU as well. And we can concentrate on
other details.
>> - 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?
I wasn't going to and I won't be in near future. I'm also missing
someone else's (from the maiainers) opinion, so I can't even know
if this makes sense or if it's totally exaggerated.
If someone is willing to code this in, I'll gladly review it, though.
> 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