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@xxxxxxxxxx> 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list