[libvirt] [PATCH v1 0/6] ivshmem support

[Martin Kletzander]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140903/2d194a99/attachment-0001.sig>