[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [RFC PATCH 1/5] doc: schema: Add basic documentation for the ivshmem support



On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]