On Wed, Apr 21, 2010 at 12:01:18PM -0400, Chris Lalancette wrote:
> Implement the qemu hooks for XML namespace data. This
> allows us to specify a qemu XML namespace, and then
> specify:
>
> <qemu:commandline>
> <qemu:arg>arg</qemu:arg>
> <qemu:env name='name' value='value'/>
> </qemu:commandline>
Now I see it, it looks a little odd to use the element content
for qemu:arg, while having an element attributes for qemu:env.
Since changing qemu:env to use content isn't practical, I
think we could do this instead
<qemu:arg value='blah'/>
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index b2820f0..ab0a4a4 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -158,6 +158,17 @@ struct qemud_driver {
> typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
> typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr;
>
> +typedef struct _qemuDomainCmdlineDef qemuDomainCmdlineDef;
> +typedef qemuDomainCmdlineDef *qemuDomainCmdlineDefPtr;
> +struct _qemuDomainCmdlineDef {
> + unsigned int num_extra;
> + char **extra;
Lets call these 'num_args' and 'args' instead.
> +
> + unsigned int num_env;
> + char **env_name;
> + char **env_value;
> +};
> +
> /* Port numbers used for KVM migration. */
> # define QEMUD_MIGRATION_FIRST_PORT 49152
> # define QEMUD_MIGRATION_NUM_PORTS 64
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5f4adfd..0b297a7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -47,6 +47,8 @@
> #include <sys/ioctl.h>
> #include <sys/un.h>
>
> +#include <libxml/xpathInternals.h>
> +
> #ifdef __linux__
> # include <sys/vfs.h>
> # ifndef NFS_SUPER_MAGIC
> @@ -88,6 +90,9 @@
>
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
> +#define QEMU_NAMESPACE_HREF "http://libvirt.org/schemas/domain/qemu/1.0";
> +
> +
> /* Only 1 job is allowed at any time
> * A job includes *all* monitor commands, even those just querying
> * information, not merely actions */
> @@ -526,6 +531,144 @@ static void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virD
> }
> }
>
> +static void qemuDomainDefNamespaceFree(void *nsdata)
> +{
> + qemuDomainCmdlineDefPtr cmd = nsdata;
> + int i;
> +
> + if (!cmd)
> + return;
> +
> + for (i = 0; i < cmd->num_extra; i++)
> + VIR_FREE(cmd->extra[i]);
> + for (i = 0; i < cmd->num_env; i++) {
> + VIR_FREE(cmd->env_name[i]);
> + VIR_FREE(cmd->env_value[i]);
> + }
> + VIR_FREE(cmd->extra);
> + VIR_FREE(cmd->env_name);
> + VIR_FREE(cmd->env_value);
> + VIR_FREE(cmd);
> +}
> +
> +static int qemuDomainDefNamespaceParse(xmlDocPtr xml,
> + xmlNodePtr root,
> + xmlXPathContextPtr ctxt,
> + void **data)
> +{
> + qemuDomainCmdlineDefPtr cmd = NULL;
> + xmlNsPtr ns;
> + xmlNodePtr *nodes = NULL;
> + int n, i;
> + xmlNodePtr oldnode;
> +
> + ns = xmlSearchNs(xml, root, BAD_CAST "qemu");
> + if (!ns)
> + /* this is fine; it just means there was no qemu namespace listed */
> + return 0;
> +
> + if (STRNEQ((const char *)ns->href, QEMU_NAMESPACE_HREF)) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Found namespace '%s' doesn't match expected '%s'"),
> + ns->href, QEMU_NAMESPACE_HREF);
> + return -1;
> + }
> +
> + if (xmlXPathRegisterNs(ctxt, ns->prefix, ns->href) < 0) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to register xml namespace '%s'"), ns->href);
> + return -1;
> + }
> +
> + if (VIR_ALLOC(cmd) < 0) {
> + virReportOOMError();
> + return -1;
> + }
> +
> + /* first handle the extra command-line arguments */
> + n = virXPathNodeSet("./qemu:commandline/qemu:arg", ctxt, &nodes);
> + if (n < 0)
> + /* virXPathNodeSet already set the error */
> + goto error;
> +
> + if (n && VIR_ALLOC_N(cmd->extra, n) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + for (i = 0; i < n; i++) {
> + oldnode = ctxt->node;
> + ctxt->node = nodes[i];
> + cmd->extra[cmd->num_extra] = virXPathString("string(.)", ctxt);
> + if (cmd->extra[cmd->num_extra] == NULL)
> + goto error;
> + cmd->num_extra++;
> + ctxt->node = oldnode;
> + }
> +
> + /* now handle the extra environment variables */
> + n = virXPathNodeSet("./qemu:commandline/qemu:env", ctxt, &nodes);
> + if (n < 0)
> + /* virXPathNodeSet already set the error */
> + goto error;
> +
> + if (n && VIR_ALLOC_N(cmd->env_name, n) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + if (n && VIR_ALLOC_N(cmd->env_value, n) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + for (i = 0; i < n; i++) {
> + oldnode = ctxt->node;
> + ctxt->node = nodes[i];
> + cmd->env_name[cmd->num_env] = virXPathString("string(./@name)", ctxt);
> + if (cmd->env_name[cmd->num_env] == NULL)
> + goto error;
> + cmd->env_value[cmd->num_env] = virXPathString("string(./@value)", ctxt);
> + /* a NULL value for command is allowed, since it might be empty */
> + cmd->num_env++;
> + ctxt->node = oldnode;
> + }
Using xpath for getting immediate attributes is somewhat overkill. You can
just avoid touching ctxt at all, and use
virXMLPropString(nodes[i], "name");
virXMLPropString(nodes[i], "value");
> +static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf,
> + void *nsdata)
> +{
> + qemuDomainCmdlineDefPtr cmd = nsdata;
> + int i;
> +
> + if (cmd->num_extra || cmd->num_env)
> + virBufferAddLit(buf, " <qemu:commandline>\n");
> + for (i = 0; i < cmd->num_extra; i++)
> + virBufferVSprintf(buf, " <qemu:arg>%s</qemu:arg>\n", cmd->extra[i]);
> + for (i = 0; i < cmd->num_env; i++) {
> + virBufferVSprintf(buf, " <qemu:env name='%s'", cmd->env_name[i]);
> + if (cmd->env_value[i])
> + virBufferVSprintf(buf, " value='%s'", cmd->env_value[i]);
> + virBufferAddLit(buf, "/>\n");
> + }
Should use escaping for all the attributes here
> + if (cmd->num_extra || cmd->num_env)
> + virBufferAddLit(buf, " </qemu:commandline>\n");
> +
> + return 0;
> +}
> +
> +static const char *qemuDomainDefNamespaceHref(void)
> +{
> + return "xmlns:qemu='" QEMU_NAMESPACE_HREF "'";
> +}
>
> static int qemuCgroupControllerActive(struct qemud_driver *driver,
> int controller)
> @@ -1316,6 +1459,14 @@ qemuCreateCapabilities(virCapsPtr oldcaps,
> caps->privateDataXMLFormat = qemuDomainObjPrivateXMLFormat;
> caps->privateDataXMLParse = qemuDomainObjPrivateXMLParse;
>
> + /* Domain Namespace XML parser hooks */
> + if (VIR_ALLOC(caps->ns) < 0)
> + goto no_memory;
> +
> + caps->ns->parse = qemuDomainDefNamespaceParse;
> + caps->ns->free = qemuDomainDefNamespaceFree;
> + caps->ns->format = qemuDomainDefNamespaceFormatXML;
> + caps->ns->href = qemuDomainDefNamespaceHref;
Could avoid needing to allocate this, by just statically
declaring the callback table.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list