On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan@xxxxxxxxxx> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list