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

On Thu, Sep 25, 2014 at 11:45 AM, Martin Kletzander <mkletzan(a)redhat.com&gt; 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>

[..] > + 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.

> + /* > + * 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 ?