On Wed, Sep 03, 2014 at 12:56:00PM +0200, Maxime Leroy wrote:
Hello Martin, Let me summarize the points we need for the next patchset version: 1) merge patches - doc: schema, conf: Parse and format and tests ( for xml2xml ) into doc: schema: conf: Add shmem node - qemu: Build comand, qemu: Add cap flag CAPS_IVSHMEM, and tests (for xml2argv) into qemu: add ivshmem support 2) parsing shmem - remove <shmem> 'model' attribute - use _uip instand of ui - check size >= 1M
I don't know how and where we ended up with this, but feel free to keep this in the parsing code, we can make it lax (or move it to qemu.*PortParse() any time in the future.
- remove loop to parse childreen nodes - path attribute is optional, default path is /var/run/libvirt/ivshmem-server/<name>-sock 3) tests: - add pci addresses in the XML 4) xml format - no ivshmem server: <shmem name='shmem0'> (name is a mandatory attribute) <size unit='M'>32</size> (optionnal, default value: 4MB, size >= 1M and power of 2)
I also had problems running qemu with size > 4MB, but I don't know where the problem was. We should do anything we can so that qemu doesn't fail (if there's the possibility). Just add the restrictions into the documentation, please.
</shmem>
- ivshmem server with no path to the socket file
<shmem name='shmem0'> (name is a mandatory attribute)
<server>
<msi vectors='32' ioeventfd='on'/> (optionnal)
</server>
<size unit='M'>32</size> (optionnal)
</shmem>
default path is : /var/run/libvirt/ivshmem-server/<name>-sock
- ivshmem server with path to a specific socket file
<shmem name='shmem0'>
<server path='/tmp/shmem0-sock'>
<msi vectors='32' ioeventfd='on'/> (optionnal)
</server>
<size unit='M'>32</size> (optionnal)
</shmem>
The name attribute is only needed if libvirt starts the ivshmem server.
In the case of the ivshmem-server is already running, the path
attribute is enough.
To simplify, I think the name attribute should be always mandatory ?
That's fine with me, we can always make it optional later on.
What do you think about this new xml format ?
Looks great, I'd just add one little bit, which I probably mentioned it earlier. I think that whatever the last patch (ivshmem server starting) will enable in the XML (e.g. <server start="yes">) should be disabled in the first part (probably during parsing). Would that be OK with you?
5) ivshmem server - remame virStartIvshmemServer to virIvshmemServerStart - call virIvshmemServerStart in qemuProcessStart instead to qemuBuild*CommandLine - rename IVSHMEMSERVER to IVSHMEM_SERVER - autostart/stop ivshmem server I really like the idea to use the new option '-q' to stop automatically the server and the new option to pass the fd to ivshmem-server and qemu. It's an elegant solution. ;)
Good to hear that.
Anyway, I need to wait to see if theses options can be integrated in qemu before to submit a new patchset for libvirt.
Feel free to leave the server part for later if you want to concentrate on the first shmem part only now. Everything looks fine, I think the next version might get in (unless somebody finds something both of us missed) :) Have a nice day, Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list