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>
[..]
> + 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 ?
> + goto cleanup;
> + }
> + }
> +
> + if ((msi = virXPathNode("./msi", ctxt))) {
> + def->msi.enabled = true;
> +
> + if ((tmp = virXMLPropString(msi, "vectors")) &&
> + virStrToLong_uip(tmp, NULL, 0, &def->msi.vectors) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("invalid number of vectors for shmem: '%s'"),
> + tmp);
> + goto cleanup;
> + }
> + VIR_FREE(tmp);
> +
> + if ((tmp = virXMLPropString(msi, "ioeventfd")) &&
> + (def->msi.ioeventfd = virTristateSwitchTypeFromString(tmp)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("invalid msi ioeventfd setting for shmem: '%s'"),
> + tmp);
> + goto cleanup;
> + }
> + VIR_FREE(tmp);
> + }
> +
> + /* msi option is only relevant with a server */
> + if (def->msi.enabled && !def->server.enabled) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("msi option is only supported with a server"));
> + goto cleanup;
If the msi node is inside the server node, i.e:
<server path='/tmp/socket-ivshmem0'>
<msi vectors='32' ioeventfd='on'/>
</server>
Then, this test can be removed.
Maxime
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list