[libvirt] [PATCH v2 1/3] docs, conf, schema: add support for shmem device

[Martin Kletzander]

On Fri, Sep 26, 2014 at 10:32:08AM +0200, Maxime Leroy wrote:
>On Thu, Sep 25, 2014 at 11:45 AM, Martin Kletzander <mkletzan at redhat.com> wrote:
>> This patch adds parsing/formatting code as well as documentation for
>> shared memory devices.  This will currently be only accessible in QEMU
>> using it's ivshmem device, but is designed as generic as possible to
>> allow future expansion for other hypervisors.
>>
>> In the devices section in the domain XML users may specify:
>>
>> - For shmem device using a server:
>>
>>  <shmem name='shmem0'>
>>    <server path='/tmp/socket-ivshmem0'/>
>>    <size unit='M'>32</size>
>>    <msi vectors='32' ioeventfd='on'/>
>>  </shmem>
>
>I would prefer:
>
><shmem name='shmem0'>
>   <server path='/tmp/socket-ivshmem0'>
>      <msi vectors='32' ioeventfd='on'/>
>   </server>
>   <size unit='M'>32</size>
></shmem>
>

Take that discussed generality into account.  What if there's another
implementation that we'll want to cover with the smem device (be it
qemu or another hypervisor) that allows MSI without any server
configuration.  It won't make sense then.

>[..]
>> +    if ((server = virXPathNode("./server", ctxt))) {
>> +        def->server.enabled = true;
>> +
>> +        if ((tmp = virXMLPropString(server, "path")))
>> +            def->server.path = virFileSanitizePath(tmp);
>> +        VIR_FREE(tmp);
>> +
>
>The server path is mandatory if the ivshmem server is not started by libvirt.
>
>If the user needs to start manually the ivshmem server, having a
>default path doesn't make sense anymore.
>

It is not.  Users can start the server themselves with the listening
socket residing in /var/lib/libvirt/shmem-<name>-sock and it will just
work.  I'm trying to keep that possibility available for the users.

>> +        /*
>> +         * We can always safely remove this check, but until the
>> +         * server starting part is in it is better to directly disable
>> +         * it rather then just ignoring it.
>> +         */
>> +        if ((tmp = virXMLPropString(server, "start"))) {
>> +            virReportError(VIR_ERR_XML_ERROR, "%s",
>> +                           _("Unsupported start attribute for "
>> +                             "shmem server element"));
>
>If an autostart ivshmem server is implemented, as we agree to
>implement for the V2 (see
>http://www.redhat.com/archives/libvir-list/2014-September/msg00124.html),
>there is no need for a "start" attribute in the server node.
>
>Why this 'dead' code ?
>

Simply, because I forgot about that, thanks for catching that, I'll
remove it ;)

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/20140926/eacad10b/attachment-0001.sig>