[libvirt] [PATCH v2 REPOST 0/8]: Add arbitrary qemu command-line and monitor commands
As we discussed previously, here is the patch series to add the ability to specify arbitrary qemu command-line parameters and environment variables, and also give arbitrary monitor commands to a guest. Because these extra arguments have a good shot at confusing libvirt, the use of them is not supported, but left available for advanced users and developers. They are also in a separate library and have a separate on-the-wire protocol. There is one bug left that I have not yet been able to fix. Because of the complicated way that virsh parses command-line arguments, it is not possible to pass through spaces and quotes when using the qemu-monitor-command. Unfortunately, the qemu monitor commands (and in particular when using QMP) depend heavily on quoting and spacing, so using virsh to send through command-lines is difficult. I'll have to think about how to better resolve this issue, but it should not hold up the rest of the series. I will point out one particular patch that could use some careful review, namely PATCH 6/8. In there, I change the remote_message_hdr to use an unsigned proc instead of a remote_procedure proc. Now, as far as I can tell the two sizes should be the same, so the wire protocol should be the same. Indeed, testing seems to show that older virsh can talk just fine to a libvirtd with this patch applied. However, the regenerated remote_protocol.c file for parsing remote_message_header has some strange bits in it that I don't quite understand. I'm hoping someone else can look at it and confirm that it is OK. Thanks to Dan Berrange and Eric Blake for their reviews already, and to DV for the Relax NG schema changes. Changes since v1 are listed in the individual patches.
This patch adds namespace XML parsers to be hooked into the main domain parser. This allows for individual hypervisor drivers to add per-namespace XML into the main domain XML. Changes since v1: - Use a statically declared table for caps->ns, removing the need to allocate/free it. Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/conf/capabilities.h | 17 +++++++++++++++++ src/conf/domain_conf.c | 38 +++++++++++++++++++++++++++++++------- src/conf/domain_conf.h | 3 +++ 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 9290c82..83bd3b7 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -115,6 +115,21 @@ struct _virCapsHost { unsigned char host_uuid[VIR_UUID_BUFLEN]; }; +typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr, + xmlXPathContextPtr, void **); +typedef void (*virDomainDefNamespaceFree)(void *); +typedef int (*virDomainDefNamespaceXMLFormat)(virBufferPtr, void *); +typedef const char *(*virDomainDefNamespaceHref)(void); + +typedef struct _virDomainXMLNamespace virDomainXMLNamespace; +typedef virDomainXMLNamespace *virDomainXMLNamespacePtr; +struct _virDomainXMLNamespace { + virDomainDefNamespaceParse parse; + virDomainDefNamespaceFree free; + virDomainDefNamespaceXMLFormat format; + virDomainDefNamespaceHref href; +}; + typedef struct _virCaps virCaps; typedef virCaps* virCapsPtr; struct _virCaps { @@ -128,6 +143,8 @@ struct _virCaps { int (*privateDataXMLFormat)(virBufferPtr, void *); int (*privateDataXMLParse)(xmlXPathContextPtr, void *); bool hasWideScsiBus; + + virDomainXMLNamespace ns; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 378c06e..653faf4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -738,6 +738,9 @@ void virDomainDefFree(virDomainDefPtr def) virCPUDefFree(def->cpu); + if (def->namespaceData && def->ns.free) + (def->ns.free)(def->namespaceData); + VIR_FREE(def); } @@ -3965,7 +3968,10 @@ static char *virDomainDefDefaultEmulator(virDomainDefPtr def, } static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, - xmlXPathContextPtr ctxt, int flags) + xmlDocPtr xml, + xmlNodePtr root, + xmlXPathContextPtr ctxt, + int flags) { xmlNodePtr *nodes = NULL, node = NULL; char *tmp = NULL; @@ -4633,6 +4639,16 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } + /* we have to make a copy of all of the callback pointers here since + * we won't have the virCaps structure available during free + */ + def->ns = caps->ns; + + if (def->ns.parse) { + if ((def->ns.parse)(xml, root, ctxt, &def->namespaceData) < 0) + goto error; + } + /* Auto-add any implied controllers which aren't present */ if (virDomainDefAddImplicitControllers(def) < 0) @@ -4653,6 +4669,7 @@ no_memory: static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps, + xmlDocPtr xml, xmlXPathContextPtr ctxt) { char *tmp = NULL; @@ -4672,7 +4689,7 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps, oldnode = ctxt->node; ctxt->node = config; - obj->def = virDomainDefParseXML(caps, ctxt, + obj->def = virDomainDefParseXML(caps, xml, config, ctxt, VIR_DOMAIN_XML_INTERNAL_STATUS); ctxt->node = oldnode; if (!obj->def) @@ -4763,7 +4780,7 @@ virDomainDefPtr virDomainDefParseNode(virCapsPtr caps, } ctxt->node = root; - def = virDomainDefParseXML(caps, ctxt, flags); + def = virDomainDefParseXML(caps, xml, root, ctxt, flags); cleanup: xmlXPathFreeContext(ctxt); @@ -4806,7 +4823,7 @@ virDomainObjPtr virDomainObjParseNode(virCapsPtr caps, } ctxt->node = root; - obj = virDomainObjParseXML(caps, ctxt); + obj = virDomainObjParseXML(caps, xml, ctxt); cleanup: xmlXPathFreeContext(ctxt); @@ -6029,10 +6046,12 @@ char *virDomainDefFormat(virDomainDefPtr def, if (def->id == -1) flags |= VIR_DOMAIN_XML_INACTIVE; + virBufferVSprintf(&buf, "<domain type='%s'", type); if (!(flags & VIR_DOMAIN_XML_INACTIVE)) - virBufferVSprintf(&buf, "<domain type='%s' id='%d'>\n", type, def->id); - else - virBufferVSprintf(&buf, "<domain type='%s'>\n", type); + virBufferVSprintf(&buf, " id='%d'", def->id); + if (def->namespaceData && def->ns.href) + virBufferVSprintf(&buf, " %s", (def->ns.href)()); + virBufferAddLit(&buf, ">\n"); virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); @@ -6289,6 +6308,11 @@ char *virDomainDefFormat(virDomainDefPtr def, } } + if (def->namespaceData && def->ns.format) { + if ((def->ns.format)(&buf, def->namespaceData) < 0) + goto cleanup; + } + virBufferAddLit(&buf, "</domain>\n"); if (virBufferError(&buf)) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 01da17e..39bb9a8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -872,6 +872,9 @@ struct _virDomainDef { virSecurityLabelDef seclabel; virDomainWatchdogDefPtr watchdog; virCPUDefPtr cpu; + + void *namespaceData; + virDomainXMLNamespace ns; }; /* Guest VM runtime state */ -- 1.6.6.1
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
This patch adds namespace XML parsers to be hooked into the main domain parser. This allows for individual hypervisor drivers to add per-namespace XML into the main domain XML.
Changes since v1: - Use a statically declared table for caps->ns, removing the need to allocate/free it.
Signed-off-by: Chris Lalancette <clalance@redhat.com>
+typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr, + xmlXPathContextPtr, void **); +typedef void (*virDomainDefNamespaceFree)(void *); +typedef int (*virDomainDefNamespaceXMLFormat)(virBufferPtr, void *); +typedef const char *(*virDomainDefNamespaceHref)(void);
Should virDomainDefNamespaceHref take a void* argument...
+ if (def->namespaceData && def->ns.href) + virBufferVSprintf(&buf, " %s", (def->ns.href)());
and pass def->namespaceData through to that function? I'm a little bit nervous about callback functions that cannot be extended, and passing a (unused, for now) void* pointer gives us some room for growth without changing the API, if need be.
+++ b/src/conf/domain_conf.c @@ -738,6 +738,9 @@ void virDomainDefFree(virDomainDefPtr def)
virCPUDefFree(def->cpu);
+ if (def->namespaceData && def->ns.free) + (def->ns.free)(def->namespaceData);
Style nit - you can omit the first set of parenthesis: def->ns.free(def->namespaceData) I don't know which style is more prevalent in existing code though, but if you choose to elide the parenthesis, there are several places in this patch that are affected.
static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, - xmlXPathContextPtr ctxt, int flags) + xmlDocPtr xml, + xmlNodePtr root, + xmlXPathContextPtr ctxt, + int flags)
As long as we are changing this API, should we change flags to unsigned int? And should we add a virCheckFlags(VIR_DOMAIN_XML_INACTIVE..., NULL)? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/01/10 - 03:23:07PM, Eric Blake wrote:
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
This patch adds namespace XML parsers to be hooked into the main domain parser. This allows for individual hypervisor drivers to add per-namespace XML into the main domain XML.
Changes since v1: - Use a statically declared table for caps->ns, removing the need to allocate/free it.
Signed-off-by: Chris Lalancette <clalance@redhat.com>
+typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr, + xmlXPathContextPtr, void **); +typedef void (*virDomainDefNamespaceFree)(void *); +typedef int (*virDomainDefNamespaceXMLFormat)(virBufferPtr, void *); +typedef const char *(*virDomainDefNamespaceHref)(void);
Should virDomainDefNamespaceHref take a void* argument...
+ if (def->namespaceData && def->ns.href) + virBufferVSprintf(&buf, " %s", (def->ns.href)());
and pass def->namespaceData through to that function? I'm a little bit nervous about callback functions that cannot be extended, and passing a (unused, for now) void* pointer gives us some room for growth without changing the API, if need be.
We could. It's an internal API, though, so we are free to change it at will later.
+++ b/src/conf/domain_conf.c @@ -738,6 +738,9 @@ void virDomainDefFree(virDomainDefPtr def)
virCPUDefFree(def->cpu);
+ if (def->namespaceData && def->ns.free) + (def->ns.free)(def->namespaceData);
Style nit - you can omit the first set of parenthesis:
def->ns.free(def->namespaceData)
I don't know which style is more prevalent in existing code though, but if you choose to elide the parenthesis, there are several places in this patch that are affected.
Yeah, true. I was actually following the style of other callbacks. I don't really care either way.
static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, - xmlXPathContextPtr ctxt, int flags) + xmlDocPtr xml, + xmlNodePtr root, + xmlXPathContextPtr ctxt, + int flags)
As long as we are changing this API, should we change flags to unsigned int? And should we add a virCheckFlags(VIR_DOMAIN_XML_INACTIVE..., NULL)?
Again, an internal API, so I guess I could change the flags. Minor change, though. As far as virCheckFlags() is concerned, my take on it (and it's usage in the tree up to this point) is always at driver entry points, not internal API's. While we could add it here, passing bogus flags here is a programming error, not a user error. -- Chris Lalancette
On 07/02/2010 06:54 AM, Chris Lalancette wrote:
Should virDomainDefNamespaceHref take a void* argument...
+ if (def->namespaceData && def->ns.href) + virBufferVSprintf(&buf, " %s", (def->ns.href)());
and pass def->namespaceData through to that function? I'm a little bit nervous about callback functions that cannot be extended, and passing a (unused, for now) void* pointer gives us some room for growth without changing the API, if need be.
We could. It's an internal API, though, so we are free to change it at will later.
Fair enough, since it is not exported.
static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, - xmlXPathContextPtr ctxt, int flags) + xmlDocPtr xml, + xmlNodePtr root, + xmlXPathContextPtr ctxt, + int flags)
As long as we are changing this API, should we change flags to unsigned int? And should we add a virCheckFlags(VIR_DOMAIN_XML_INACTIVE..., NULL)?
Again, an internal API, so I guess I could change the flags. Minor change, though. As far as virCheckFlags() is concerned, my take on it (and it's usage in the tree up to this point) is always at driver entry points, not internal API's. While we could add it here, passing bogus flags here is a programming error, not a user error.
Good point - there's a difference between a static function used only internally (we can check all callers in the same file) and a driver callback (where the interface is public, and we can't control the behavior of external programs that link to our library and pass arbitrary flags). So, with my questions addressed, ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Implement the qemu hooks for XML namespace data. This allows us to specify a qemu XML namespace, and then specify: <qemu:commandline> <qemu:arg value='arg'/> <qemu:env name='name' value='value'/> </qemu:commandline> In the domain XML. Changes since v1: - Change the <qemu:arg>arg</qemu:arg> XML to <qemu:arg value='arg'/> XML - Fix up some memory leaks in qemuDomainDefNamespaceParse - Rename num_extra and extra to num_args and args, respectively - Fixed up some error messages - Make sure to escape user-provided data in qemuDomainDefNamesapceFormatXML Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_conf.c | 14 +++++ src/qemu/qemu_conf.h | 11 ++++ src/qemu/qemu_driver.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ce42bd6..edbb2c1 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -4761,6 +4761,20 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT(current_snapshot->def->name); } + if (def->namespaceData) { + qemuDomainCmdlineDefPtr cmd; + + cmd = def->namespaceData; + for (i = 0; i < cmd->num_args; i++) + ADD_ARG_LIT(cmd->args[i]); + for (i = 0; i < cmd->num_env; i++) { + if (cmd->env_value[i]) + ADD_ENV_PAIR(cmd->env_name[i], cmd->env_value[i]); + else + ADD_ENV_PAIR(cmd->env_name[i], ""); + } + } + ADD_ARG(NULL); ADD_ENV(NULL); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index ab5f158..821ed57 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -165,6 +165,17 @@ struct qemud_driver { typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet; typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr; +typedef struct _qemuDomainCmdlineDef qemuDomainCmdlineDef; +typedef qemuDomainCmdlineDef *qemuDomainCmdlineDefPtr; +struct _qemuDomainCmdlineDef { + unsigned int num_args; + char **args; + + 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 487bfa3..4643731 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -46,6 +46,8 @@ #include <sys/ioctl.h> #include <sys/un.h> +#include <libxml/xpathInternals.h> + #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" @@ -84,6 +86,8 @@ #define QEMU_VNC_PORT_MIN 5900 #define QEMU_VNC_PORT_MAX 65535 +#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 */ @@ -531,6 +535,146 @@ static void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virD } } +static void qemuDomainDefNamespaceFree(void *nsdata) +{ + qemuDomainCmdlineDefPtr cmd = nsdata; + unsigned int i; + + if (!cmd) + return; + + for (i = 0; i < cmd->num_args; i++) + VIR_FREE(cmd->args[i]); + for (i = 0; i < cmd->num_env; i++) { + VIR_FREE(cmd->env_name[i]); + VIR_FREE(cmd->env_value[i]); + } + VIR_FREE(cmd->args); + VIR_FREE(cmd->env_name); + VIR_FREE(cmd->env_value); + VIR_FREE(cmd); +} + +static int qemuDomainDefNamespaceParse(xmlDocPtr xml, + xmlNodePtr root, + xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED, + void **data) +{ + qemuDomainCmdlineDefPtr cmd = NULL; + xmlNsPtr ns; + xmlNodePtr *nodes = NULL; + int n, i; + + 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->args, n) < 0) + goto no_memory; + + for (i = 0; i < n; i++) { + cmd->args[cmd->num_args] = virXMLPropString(nodes[i], "value"); + if (cmd->args[cmd->num_args] == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("No qemu command-line argument specified")); + goto error; + } + cmd->num_args++; + } + + VIR_FREE(nodes); + + /* 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) + goto no_memory; + + if (n && VIR_ALLOC_N(cmd->env_value, n) < 0) + goto no_memory; + + for (i = 0; i < n; i++) { + cmd->env_name[cmd->num_env] = virXMLPropString(nodes[i], "name"); + if (cmd->env_name[cmd->num_env] == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("No qemu environment name specified")); + goto error; + } + cmd->env_value[cmd->num_env] = virXMLPropString(nodes[i], "value"); + /* a NULL value for command is allowed, since it might be empty */ + cmd->num_env++; + } + + VIR_FREE(nodes); + + *data = cmd; + + return 0; + +no_memory: + virReportOOMError(); + +error: + VIR_FREE(nodes); + qemuDomainDefNamespaceFree(cmd); + return -1; +} + +static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, + void *nsdata) +{ + qemuDomainCmdlineDefPtr cmd = nsdata; + unsigned int i; + + if (cmd->num_args || cmd->num_env) + virBufferAddLit(buf, " <qemu:commandline>\n"); + for (i = 0; i < cmd->num_args; i++) + virBufferEscapeString(buf, " <qemu:arg value='%s'/>\n", + cmd->args[i]); + for (i = 0; i < cmd->num_env; i++) { + virBufferVSprintf(buf, " <qemu:env name='%s'", cmd->env_name[i]); + if (cmd->env_value[i]) + virBufferEscapeString(buf, " value='%s'", cmd->env_value[i]); + virBufferAddLit(buf, "/>\n"); + } + if (cmd->num_args || 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) @@ -1367,6 +1511,12 @@ qemuCreateCapabilities(virCapsPtr oldcaps, goto err_exit; } + /* Domain Namespace XML parser hooks */ + caps->ns.parse = qemuDomainDefNamespaceParse; + caps->ns.free = qemuDomainDefNamespaceFree; + caps->ns.format = qemuDomainDefNamespaceFormatXML; + caps->ns.href = qemuDomainDefNamespaceHref; + /* Security driver data */ if (driver->securityPrimaryDriver) { const char *doi, *model; -- 1.6.6.1
On 07/01/2010 02:59 PM, 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 value='arg'/> <qemu:env name='name' value='value'/> </qemu:commandline>
In the domain XML.
Changes since v1: - Change the <qemu:arg>arg</qemu:arg> XML to <qemu:arg value='arg'/> XML - Fix up some memory leaks in qemuDomainDefNamespaceParse - Rename num_extra and extra to num_args and args, respectively - Fixed up some error messages - Make sure to escape user-provided data in qemuDomainDefNamesapceFormatXML
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_conf.c | 14 +++++ src/qemu/qemu_conf.h | 11 ++++ src/qemu/qemu_driver.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 0 deletions(-)
+ if (def->namespaceData) { + qemuDomainCmdlineDefPtr cmd; + + cmd = def->namespaceData; + for (i = 0; i < cmd->num_args; i++) + ADD_ARG_LIT(cmd->args[i]);
It would be nice if we had the new task creation API stablized and implemented, but for now, this is reasonable.
+ for (i = 0; i < n; i++) { + cmd->env_name[cmd->num_env] = virXMLPropString(nodes[i], "name"); + if (cmd->env_name[cmd->num_env] == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("No qemu environment name specified")); + goto error;
Do we need to validate that the resulting name is valid (starts with a letter, and contains only alphanumeric and _)? arg and env_value can obviously be arbitrary strings, but not env_name.
+static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, + void *nsdata) +{ + qemuDomainCmdlineDefPtr cmd = nsdata; + unsigned int i; + + if (cmd->num_args || cmd->num_env) + virBufferAddLit(buf, " <qemu:commandline>\n");
Rather than check cmd->num_args and cmd->num_env here and again below, why not just do an early return 0 here?
+ for (i = 0; i < cmd->num_args; i++) + virBufferEscapeString(buf, " <qemu:arg value='%s'/>\n", + cmd->args[i]); + for (i = 0; i < cmd->num_env; i++) { + virBufferVSprintf(buf, " <qemu:env name='%s'", cmd->env_name[i]); + if (cmd->env_value[i]) + virBufferEscapeString(buf, " value='%s'", cmd->env_value[i]); + virBufferAddLit(buf, "/>\n"); + } + if (cmd->num_args || cmd->num_env) + virBufferAddLit(buf, " </qemu:commandline>\n"); + + return 0; +} + +static const char *qemuDomainDefNamespaceHref(void) +{ + return "xmlns:qemu='" QEMU_NAMESPACE_HREF "'"; +}
May need a tweak to add an ATTRIBUTE_IGNORED parameter depending on your reaction to my comment on the 1/8 patch. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/01/10 - 03:39:25PM, Eric Blake wrote:
src/qemu/qemu_conf.c | 14 +++++ src/qemu/qemu_conf.h | 11 ++++ src/qemu/qemu_driver.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 0 deletions(-)
+ if (def->namespaceData) { + qemuDomainCmdlineDefPtr cmd; + + cmd = def->namespaceData; + for (i = 0; i < cmd->num_args; i++) + ADD_ARG_LIT(cmd->args[i]);
It would be nice if we had the new task creation API stablized and implemented, but for now, this is reasonable.
+ for (i = 0; i < n; i++) { + cmd->env_name[cmd->num_env] = virXMLPropString(nodes[i], "name"); + if (cmd->env_name[cmd->num_env] == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("No qemu environment name specified")); + goto error;
Do we need to validate that the resulting name is valid (starts with a letter, and contains only alphanumeric and _)? arg and env_value can obviously be arbitrary strings, but not env_name.
Hm, interesting, I didn't know that rule about environment variable names. That is a good check to make, I'll add it.
+static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, + void *nsdata) +{ + qemuDomainCmdlineDefPtr cmd = nsdata; + unsigned int i; + + if (cmd->num_args || cmd->num_env) + virBufferAddLit(buf, " <qemu:commandline>\n");
Rather than check cmd->num_args and cmd->num_env here and again below, why not just do an early return 0 here?
Yeah, I can invert the logic and do that instead. Since I'll be respinning the patch for the above environment variable checking, I'll change it. -- Chris Lalancette
On 07/02/2010 07:01 AM, Chris Lalancette wrote:
Do we need to validate that the resulting name is valid (starts with a letter, and contains only alphanumeric and _)? arg and env_value can obviously be arbitrary strings, but not env_name.
Hm, interesting, I didn't know that rule about environment variable names. That is a good check to make, I'll add it.
Technically, any string that does not contain = can be inserted as an environment name, but then you can't access them from the shell. So it's best to restrict environment names to portable names (basically, the same set as shell variable names). Also, does the .rng relaxed schema have a way to express the limitation on valid env_names, for your patch 8/8? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/02/10 - 07:06:34AM, Eric Blake wrote:
On 07/02/2010 07:01 AM, Chris Lalancette wrote:
Do we need to validate that the resulting name is valid (starts with a letter, and contains only alphanumeric and _)? arg and env_value can obviously be arbitrary strings, but not env_name.
Hm, interesting, I didn't know that rule about environment variable names. That is a good check to make, I'll add it.
Technically, any string that does not contain = can be inserted as an environment name, but then you can't access them from the shell. So it's best to restrict environment names to portable names (basically, the same set as shell variable names).
Interesting point. Since we are directly invoking qemu with execve, in theory, qemu could access an environment name that doesn't conform to the shell's rules. That being said, since many people are invoking qemu through a shell, I doubt qemu would do something like that. So what do we think; add the restriction, or no?
Also, does the .rng relaxed schema have a way to express the limitation on valid env_names, for your patch 8/8?
Yeah, there is a way to add patterns to the RNG, if we decide to restrict the environment variables. -- Chris Lalancette
On 07/02/2010 07:29 AM, Chris Lalancette wrote:
Technically, any string that does not contain = can be inserted as an environment name, but then you can't access them from the shell. So it's best to restrict environment names to portable names (basically, the same set as shell variable names).
Interesting point. Since we are directly invoking qemu with execve, in theory, qemu could access an environment name that doesn't conform to the shell's rules. That being said, since many people are invoking qemu through a shell, I doubt qemu would do something like that.
So what do we think; add the restriction, or no?
I'm in favor of adding the restriction. Otherwise, we are allowing a backdoor where libvirt can make qemu do something that regular shell invocation of qemu cannot. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Now that we have the ability to specify arbitrary qemu command-line parameters in the XML, use it to handle unknown command-line parameters when doing a native-to-xml conversion. Changes since v1: - Rename num_extra to num_args - Fix up a memory leak on an error path Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_conf.c | 28 ++++++++++++++++++++++------ 1 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index edbb2c1..ecf21d3 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -5848,6 +5848,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, const char **nics = NULL; int video = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; int nvirtiodisk = 0; + qemuDomainCmdlineDefPtr cmd; if (!progargv[0]) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -5858,6 +5859,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, if (VIR_ALLOC(def) < 0) goto no_memory; + /* allocate the cmdlinedef up-front; if it's unused, we'll free it later */ + if (VIR_ALLOC(cmd) < 0) + goto no_memory; + virUUIDGenerate(def->uuid); def->id = -1; @@ -6282,12 +6287,15 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { - VIR_WARN("unknown QEMU argument '%s' during conversion", arg); -#if 0 - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown argument '%s'"), arg); - goto error; -#endif + /* something we can't yet parse. Add it to the qemu namespace + * cmdline/environment advanced options and hope for the best + */ + if (VIR_REALLOC_N(cmd->args, cmd->num_args+1) < 0) + goto no_memory; + cmd->args[cmd->num_args] = strdup(arg); + if (cmd->args[cmd->num_args] == NULL) + goto no_memory; + cmd->num_args++; } } @@ -6347,11 +6355,19 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, if (virDomainDefAddImplicitControllers(def) < 0) goto error; + if (cmd->num_args || cmd->num_env) { + def->ns = caps->ns; + def->namespaceData = cmd; + } + else + VIR_FREE(cmd); + return def; no_memory: virReportOOMError(); error: + VIR_FREE(cmd); virDomainDefFree(def); VIR_FREE(nics); return NULL; -- 1.6.6.1
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
Now that we have the ability to specify arbitrary qemu command-line parameters in the XML, use it to handle unknown command-line parameters when doing a native-to-xml conversion.
Changes since v1: - Rename num_extra to num_args - Fix up a memory leak on an error path
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_conf.c | 28 ++++++++++++++++++++++------ 1 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c } else { - VIR_WARN("unknown QEMU argument '%s' during conversion", arg); -#if 0 - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown argument '%s'"), arg); - goto error; -#endif + /* something we can't yet parse. Add it to the qemu namespace + * cmdline/environment advanced options and hope for the best + */
Let's still keep the VIR_WARN, as it may help in debugging. ACK, with that nit addressed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Add the library entry point for the new virDomainQemuMonitorCommand() entry point. Because this is not part of the "normal" libvirt API, it gets it's own header file, library file, and will eventually get it's own over-the-wire protocol later in the series. Changes since v1: - Go back to using the virDriver table for qemuDomainMonitorCommand, due to linking issues - Added versioning information to the libvirt-qemu.so Signed-off-by: Chris Lalancette <clalance@redhat.com> --- include/libvirt/Makefile.am | 1 + include/libvirt/libvirt-qemu.h | 30 ++++++++++++ src/Makefile.am | 8 +++- src/driver.h | 6 +++ src/esx/esx_driver.c | 1 + src/libvirt-qemu.c | 97 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 3 +- src/lxc/lxc_driver.c | 1 + src/opennebula/one_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + 19 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 include/libvirt/libvirt-qemu.h create mode 100644 src/libvirt-qemu.c diff --git a/include/libvirt/Makefile.am b/include/libvirt/Makefile.am index 8589dc5..b2c2b76 100644 --- a/include/libvirt/Makefile.am +++ b/include/libvirt/Makefile.am @@ -3,6 +3,7 @@ virincdir = $(includedir)/libvirt virinc_HEADERS = libvirt.h \ + libvirt-qemu.h \ virterror.h install-exec-hook: diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h new file mode 100644 index 0000000..1170196 --- /dev/null +++ b/include/libvirt/libvirt-qemu.h @@ -0,0 +1,30 @@ +/* -*- c -*- + * libvirt_qemu.h: + * Summary: qemu specific interfaces + * Description: Provides the interfaces of the libvirt library to handle + * qemu specific methods + * + * Copy: Copyright (C) 2010 Red Hat, Inc. + * + * See COPYING.LIB for the License of this software + * + * Author: Chris Lalancette <clalance@redhat.com> + */ + +#ifndef __VIR_QEMU_H__ +# define __VIR_QEMU_H__ + +# include "libvirt.h" + +# ifdef __cplusplus +extern "C" { +# endif + +int virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, + char **result, unsigned int flags); + +# ifdef __cplusplus +} +# endif + +#endif /* __VIR_QEMU_H__ */ diff --git a/src/Makefile.am b/src/Makefile.am index ece18a6..9cf9d67 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -32,7 +32,7 @@ if WITH_NETWORK UUID=$(shell uuidgen 2>/dev/null) endif -lib_LTLIBRARIES = libvirt.la +lib_LTLIBRARIES = libvirt.la libvirt-qemu.la moddir = $(libdir)/libvirt/drivers mod_LTLIBRARIES = @@ -1028,6 +1028,12 @@ libvirt_test_la_LIBADD = $(libvirt_la_LIBADD) libvirt_test_la_LDFLAGS = $(test_LDFLAGS) libvirt_test_la_CFLAGS = $(COVERAGE_CFLAGS) +libvirt_qemu_la_SOURCES = libvirt-qemu.c +libvirt_qemu_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \ + -version-info $(LIBVIRT_VERSION_INFO) \ + $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS) +libvirt_qemu_la_CFLAGS = $(COVERAGE_CFLAGS) +libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD) libexec_PROGRAMS = diff --git a/src/driver.h b/src/driver.h index 22e3db6..e443c1c 100644 --- a/src/driver.h +++ b/src/driver.h @@ -457,6 +457,11 @@ typedef int (*virDrvDomainSnapshotDelete)(virDomainSnapshotPtr snapshot, unsigned int flags); +typedef int + (*virDrvQemuDomainMonitorCommand)(virDomainPtr domain, const char *cmd, + char **result, unsigned int flags); + + /** * _virDriver: @@ -569,6 +574,7 @@ struct _virDriver { virDrvDomainSnapshotCurrent domainSnapshotCurrent; virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; + virDrvQemuDomainMonitorCommand qemuDomainMonitorCommand; }; typedef int diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index acf8908..227e6a9 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3759,6 +3759,7 @@ static virDriver esxDriver = { esxDomainSnapshotCurrent, /* domainSnapshotCurrent */ esxDomainRevertToSnapshot, /* domainRevertToSnapshot */ esxDomainSnapshotDelete, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c new file mode 100644 index 0000000..2d43e13 --- /dev/null +++ b/src/libvirt-qemu.c @@ -0,0 +1,97 @@ +#include <config.h> + +#include "virterror_internal.h" +#include "logging.h" +#include "datatypes.h" +#include "libvirt/libvirt-qemu.h" + +/** + * virLibConnError: + * @conn: the connection if available + * @error: the error number + * @info: extra information string + * + * Handle an error at the connection level + */ +static void +virLibConnError(virConnectPtr conn, virErrorNumber error, const char *info) +{ + const char *errmsg; + + if (error == VIR_ERR_OK) + return; + + errmsg = virErrorMsg(error, info); + virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, error, VIR_ERR_ERROR, + errmsg, info, NULL, 0, 0, errmsg, info); +} + +/** + * virLibDomainError: + * @domain: the domain if available + * @error: the error number + * @info: extra information string + * + * Handle an error at the connection level + */ +static void +virLibDomainError(virDomainPtr domain, virErrorNumber error, + const char *info) +{ + virConnectPtr conn = NULL; + const char *errmsg; + + if (error == VIR_ERR_OK) + return; + + errmsg = virErrorMsg(error, info); + if (error != VIR_ERR_INVALID_DOMAIN) { + conn = domain->conn; + } + virRaiseError(conn, domain, NULL, VIR_FROM_DOM, error, VIR_ERR_ERROR, + errmsg, info, NULL, 0, 0, errmsg, info); +} + +int +virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, + char **result, unsigned int flags) +{ + virConnectPtr conn; + + DEBUG("domain=%p, cmd=%s, result=%p, flags=%u", domain, cmd, result, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = domain->conn; + + if (result == NULL) { + virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->qemuDomainMonitorCommand) { + int ret; + ret = conn->driver->qemuDomainMonitorCommand(domain, cmd, result, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 778ceb1..c937367 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -21,6 +21,7 @@ virBufferContentAndReset; virBufferError; virBufferURIEncodeString; virBufferFreeAndReset; +virBufferStrcat; # caps.h @@ -731,6 +732,7 @@ virReportSystemErrorFull; virReportOOMErrorFull; virStrerror; virSetError; +virDispatchError; # xml.h diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 849c163..77035ff 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -393,16 +393,15 @@ LIBVIRT_0.8.0 { virDomainSnapshotFree; } LIBVIRT_0.7.7; - LIBVIRT_0.8.1 { global: virDomainGetBlockInfo; } LIBVIRT_0.8.0; - LIBVIRT_0.8.2 { global: virDomainCreateWithFlags; + virDomainQemuMonitorCommand; } LIBVIRT_0.8.1; # .... define new API here using predicted next version number .... diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 462bc9c..4fc1ecd 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2635,6 +2635,7 @@ static virDriver lxcDriver = { NULL, /* domainSnapshotCurrent */ NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; static virStateDriver lxcStateDriver = { diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c index 9d7b415..e70f17b 100644 --- a/src/opennebula/one_driver.c +++ b/src/opennebula/one_driver.c @@ -817,6 +817,7 @@ static virDriver oneDriver = { NULL, /* domainSnapshotCurrent */ NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; static virStateDriver oneStateDriver = { diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index f7da1be..98381fb 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1570,6 +1570,7 @@ static virDriver openvzDriver = { NULL, /* domainSnapshotCurrent */ NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; int openvzRegister(void) { diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index ee1e21b..e4afc5a 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3990,6 +3990,7 @@ static virDriver phypDriver = { NULL, /* domainSnapshotCurrent */ NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ + NULL, /* qemuMonitorCommand */ }; static virStorageDriver phypStorageDriver = { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4643731..5cc63e2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12521,6 +12521,7 @@ static virDriver qemuDriver = { qemuDomainSnapshotCurrent, /* domainSnapshotCurrent */ qemuDomainRevertToSnapshot, /* domainRevertToSnapshot */ qemuDomainSnapshotDelete, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7052bf1..18d2320 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -10302,6 +10302,7 @@ static virDriver remote_driver = { remoteDomainSnapshotCurrent, /* domainSnapshotCurrent */ remoteDomainRevertToSnapshot, /* domainRevertToSnapshot */ remoteDomainSnapshotDelete, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; static virNetworkDriver network_driver = { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5b6f47e..6c06cbc 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5322,6 +5322,7 @@ static virDriver testDriver = { NULL, /* domainSnapshotCurrent */ NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; static virNetworkDriver testNetworkDriver = { diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 110179e..90bf1b5 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1952,6 +1952,7 @@ static virDriver umlDriver = { NULL, /* domainSnapshotCurrent */ NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 0e0013b..edc7a72 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8248,6 +8248,7 @@ virDriver NAME(Driver) = { vboxDomainSnapshotCurrent, /* domainSnapshotCurrent */ vboxDomainRevertToSnapshot, /* domainRevertToSnapshot */ vboxDomainSnapshotDelete, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; virNetworkDriver NAME(NetworkDriver) = { diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 3dd673b..b55e494 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2004,6 +2004,7 @@ static virDriver xenUnifiedDriver = { NULL, /* domainSnapshotCurrent */ NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; /** diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index cefcf3b..e385648 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1814,6 +1814,7 @@ static virDriver xenapiDriver = { NULL, /* domainSnapshotCurrent */ NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ + NULL, /* qemuDomainMonitorCommand */ }; /** -- 1.6.6.1
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
Add the library entry point for the new virDomainQemuMonitorCommand() entry point. Because this is not part of the "normal" libvirt API, it gets it's own header file, library file, and will eventually get it's own over-the-wire protocol later in the series.
s/it's/its/
+ +int +virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, + char **result, unsigned int flags) +{ + virConnectPtr conn; + + DEBUG("domain=%p, cmd=%s, result=%p, flags=%u", domain, cmd, result, flags);
Do we want to use virCheckFlags(0, -1) here? ACK with those nits addressed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/01/10 - 04:02:42PM, Eric Blake wrote:
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
Add the library entry point for the new virDomainQemuMonitorCommand() entry point. Because this is not part of the "normal" libvirt API, it gets it's own header file, library file, and will eventually get it's own over-the-wire protocol later in the series.
s/it's/its/
Oops, I'll fix that :).
+ +int +virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, + char **result, unsigned int flags) +{ + virConnectPtr conn; + + DEBUG("domain=%p, cmd=%s, result=%p, flags=%u", domain, cmd, result, flags);
Do we want to use virCheckFlags(0, -1) here?
Again, this is a non-standard use of virDomainQemuMonitorCommand. I'm actually less against its use here, since this is an entry point with a user-provided flag parameter, but it's not standard practice at present to add it to these entry points. Indeed, in the general case it *has* to be pushed to the individual driver entry points, because we don't know what flags individual drivers support. The best we could do is to do the check twice; at the libvirt entry point, we could check that it is one of the valid flags, and then at the driver entry point, check that it is one of the flags this driver supports. The benefit to this scheme is that we can catch gratuitious errors early without doing an RPC call; the downside is that we are repeating the virCheckFlags() call twice. In any case, I think that would be a separate patch, so I would argue to leave this as-is for now (minus my grammatical mistake :). -- Chris Lalancette
On 07/02/10 - 09:45:04AM, Chris Lalancette wrote:
Do we want to use virCheckFlags(0, -1) here?
Again, this is a non-standard use of virDomainQemuMonitorCommand. I'm actual ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Err, I meant "virCheckFlags" here :)
-- Chris Lalancette
On 07/02/2010 07:45 AM, Chris Lalancette wrote:
Indeed, in the general case it *has* to be pushed to the individual driver entry points, because we don't know what flags individual drivers support. The best we could do is to do the check twice; at the libvirt entry point, we could check that it is one of the valid flags, and then at the driver entry point, check that it is one of the flags this driver supports. The benefit to this scheme is that we can catch gratuitious errors early without doing an RPC call; the downside is that we are repeating the virCheckFlags() call twice.
In any case, I think that would be a separate patch, so I would argue to leave this as-is for now (minus my grammatical mistake :).
Good argument - no change for now. And I _did_ see the virCheckFlags later on in 5/8 (it's just that I wrote my review immediately after reading 4/8, rather than reading the series). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Implement the qemu driver's virDomainQemuMonitorCommand and hook it into the API entry point. Changes since v1: - Rename the (external) qemuMonitorCommand to qemuDomainMonitorCommand - Add virCheckFlags to qemuDomainMonitorCommand Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_driver.c | 42 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_monitor.c | 13 +++++++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 28 ++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++++ src/qemu/qemu_monitor_text.c | 21 +++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 +++ 7 files changed, 112 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5cc63e2..cd4324b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12422,6 +12422,46 @@ cleanup: return ret; } +static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, + char **result, unsigned int flags ATTRIBUTE_UNUSED) +{ + struct qemud_driver *driver = domain->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(0, -1); + + VIR_WARN(_("Qemu monitor command '%s' executed; libvirt results may be unpredictable!"), cmd); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, domain->uuid); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(domain->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result); + qemuDomainObjExitMonitorWithDriver(driver, vm); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + static virDriver qemuDriver = { VIR_DRV_QEMU, "QEMU", @@ -12521,7 +12561,7 @@ static virDriver qemuDriver = { qemuDomainSnapshotCurrent, /* domainSnapshotCurrent */ qemuDomainRevertToSnapshot, /* domainRevertToSnapshot */ qemuDomainSnapshotDelete, /* domainSnapshotDelete */ - NULL, /* qemuDomainMonitorCommand */ + qemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b05032a..f1494ff 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1918,3 +1918,16 @@ int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name) ret = qemuMonitorTextDeleteSnapshot(mon, name); return ret; } + +int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char **reply) +{ + int ret; + + DEBUG("mon=%p, cmd=%s, reply=%p", mon, cmd, reply); + + if (mon->json) + ret = qemuMonitorJSONArbitraryCommand(mon, cmd, reply); + else + ret = qemuMonitorTextArbitraryCommand(mon, cmd, reply); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 459ccb7..48f4c20 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -389,4 +389,6 @@ int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name); +int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char **reply); + #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 01be86d..5ea71f2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2344,3 +2344,31 @@ int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name) virJSONValueFree(reply); return ret; } + +int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, + const char *cmd_str, + char **reply_str) +{ + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + int ret = -1; + + cmd = virJSONValueFromString(cmd_str); + if (!cmd) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + *reply_str = virJSONValueToString(reply); + if (!(*reply_str)) + goto cleanup; + + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 6fa8d83..94806c1 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -196,4 +196,8 @@ int qemuMonitorJSONCreateSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorJSONLoadSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name); +int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, + const char *cmd_str, + char **reply_str); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 569742a..69971a6 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2567,3 +2567,24 @@ cleanup: VIR_FREE(reply); return ret; } + +int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd, + char **reply) +{ + char *safecmd = NULL; + int ret; + + if (!(safecmd = qemuMonitorEscapeArg(cmd))) { + virReportOOMError(); + return -1; + } + + ret = qemuMonitorCommand(mon, safecmd, reply); + if (ret != 0) + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("failed to run cmd '%s'"), safecmd); + + VIR_FREE(safecmd); + + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 9926d34..c017509 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -194,4 +194,7 @@ int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name); +int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd, + char **reply); + #endif /* QEMU_MONITOR_TEXT_H */ -- 1.6.6.1
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
Implement the qemu driver's virDomainQemuMonitorCommand and hook it into the API entry point.
Changes since v1: - Rename the (external) qemuMonitorCommand to qemuDomainMonitorCommand - Add virCheckFlags to qemuDomainMonitorCommand
Signed-off-by: Chris Lalancette <clalance@redhat.com> +static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, + char **result, unsigned int flags ATTRIBUTE_UNUSED)
Drop ATTRIBUTE_UNUSED...
+{ + struct qemud_driver *driver = domain->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(0, -1);
...since this uses it. Other than that, ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Since we are adding a new "per-hypervisor" protocol, we make it so that the qemu remote protocol uses a new PROTOCOL and PROGRAM number. This allows us to easily distinguish it from the normal REMOTE protocol. This necessitates changing the proc in remote_message_header from a "remote_procedure" to an "unsigned", which should be the same size (and thus preserve the on-wire protocol). Changes since v1: - Fixed up a couple of script problems in remote_generate_stubs.pl - Switch an int flag to a bool in dispatch.c Signed-off-by: Chris Lalancette <clalance@redhat.com> --- cfg.mk | 2 +- daemon/Makefile.am | 32 +++++++++--- daemon/dispatch.c | 45 +++++++++++----- daemon/libvirtd.h | 1 + daemon/qemu_dispatch_args.h | 5 ++ daemon/qemu_dispatch_prototypes.h | 12 ++++ daemon/qemu_dispatch_ret.h | 5 ++ daemon/qemu_dispatch_table.h | 14 +++++ daemon/remote.c | 45 ++++++++++++++++ daemon/remote.h | 8 +++ daemon/remote_generate_stubs.pl | 62 ++++++++++++++-------- src/Makefile.am | 37 ++++++++++++- src/remote/qemu_protocol.c | 41 ++++++++++++++ src/remote/qemu_protocol.h | 57 ++++++++++++++++++++ src/remote/qemu_protocol.x | 46 ++++++++++++++++ src/remote/remote_driver.c | 105 +++++++++++++++++++++++++++---------- src/remote/remote_protocol.c | 50 +++++++++++++++++- src/remote/remote_protocol.h | 2 +- src/remote/remote_protocol.x | 2 +- 19 files changed, 494 insertions(+), 77 deletions(-) create mode 100644 daemon/qemu_dispatch_args.h create mode 100644 daemon/qemu_dispatch_prototypes.h create mode 100644 daemon/qemu_dispatch_ret.h create mode 100644 daemon/qemu_dispatch_table.h create mode 100644 src/remote/qemu_protocol.c create mode 100644 src/remote/qemu_protocol.h create mode 100644 src/remote/qemu_protocol.x diff --git a/cfg.mk b/cfg.mk index aefe34c..e12265e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -416,7 +416,7 @@ sc_prohibit_trailing_blank_lines: test $$found = 0 # Regex for grep -E that exempts generated files from style rules. -preprocessor_exempt = (remote_(driver|protocol)\.h)$$ +preprocessor_exempt = ((qemu|remote)_(driver|protocol)\.h)$$ # Enforce recommended preprocessor indentation style. sc_preprocessor_indentation: @if cppi --version >/dev/null 2>&1; then \ diff --git a/daemon/Makefile.am b/daemon/Makefile.am index df34ef1..cbebaa5 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -10,7 +10,8 @@ DAEMON_SOURCES = \ remote_dispatch_table.h \ remote_dispatch_args.h \ remote_dispatch_ret.h \ - ../src/remote/remote_protocol.c + ../src/remote/remote_protocol.c \ + ../src/remote/qemu_protocol.c AVAHI_SOURCES = \ mdns.c mdns.h @@ -79,7 +80,7 @@ libvirtd_LDADD = \ $(SASL_LIBS) \ $(POLKIT_LIBS) -libvirtd_LDADD += ../src/libvirt_util.la +libvirtd_LDADD += ../src/libvirt_util.la ../src/libvirt-qemu.la if WITH_DRIVER_MODULES libvirtd_LDADD += ../src/libvirt_driver.la @@ -170,21 +171,38 @@ endif remote.c: remote_dispatch_prototypes.h \ remote_dispatch_table.h \ remote_dispatch_args.h \ - remote_dispatch_ret.h + remote_dispatch_ret.h \ + qemu_dispatch_prototypes.h \ + qemu_dispatch_table.h \ + qemu_dispatch_args.h \ + qemu_dispatch_ret.h REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x +QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x remote_dispatch_prototypes.h: $(srcdir)/remote_generate_stubs.pl $(REMOTE_PROTOCOL) - $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -p $(REMOTE_PROTOCOL) > $@ + $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -c -p remote $(REMOTE_PROTOCOL) > $@ remote_dispatch_table.h: $(srcdir)/remote_generate_stubs.pl $(REMOTE_PROTOCOL) - $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -t $(REMOTE_PROTOCOL) > $@ + $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -c -t remote $(REMOTE_PROTOCOL) > $@ remote_dispatch_args.h: $(srcdir)/remote_generate_stubs.pl $(REMOTE_PROTOCOL) - $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -a $(REMOTE_PROTOCOL) > $@ + $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -c -a remote $(REMOTE_PROTOCOL) > $@ remote_dispatch_ret.h: $(srcdir)/remote_generate_stubs.pl $(REMOTE_PROTOCOL) - $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -r $(REMOTE_PROTOCOL) > $@ + $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -c -r remote $(REMOTE_PROTOCOL) > $@ + +qemu_dispatch_prototypes.h: $(srcdir)/remote_generate_stubs.pl $(QEMU_PROTOCOL) + $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -p qemu $(QEMU_PROTOCOL) > $@ + +qemu_dispatch_table.h: $(srcdir)/remote_generate_stubs.pl $(QEMU_PROTOCOL) + $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -t qemu $(QEMU_PROTOCOL) > $@ + +qemu_dispatch_args.h: $(srcdir)/remote_generate_stubs.pl $(QEMU_PROTOCOL) + $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -a qemu $(QEMU_PROTOCOL) > $@ + +qemu_dispatch_ret.h: $(srcdir)/remote_generate_stubs.pl $(QEMU_PROTOCOL) + $(AM_V_GEN)perl -w $(srcdir)/remote_generate_stubs.pl -r qemu $(QEMU_PROTOCOL) > $@ LOGROTATE_CONFS = libvirtd.qemu.logrotate libvirtd.lxc.logrotate \ libvirtd.uml.logrotate diff --git a/daemon/dispatch.c b/daemon/dispatch.c index 8f55eaa..2b21111 100644 --- a/daemon/dispatch.c +++ b/daemon/dispatch.c @@ -23,6 +23,8 @@ #include <config.h> +#include <stdbool.h> + #include "dispatch.h" #include "remote.h" @@ -336,10 +338,11 @@ cleanup: } -int +static int remoteDispatchClientCall (struct qemud_server *server, struct qemud_client *client, - struct qemud_client_message *msg); + struct qemud_client_message *msg, + bool qemu_protocol); /* @@ -356,12 +359,13 @@ remoteDispatchClientCall (struct qemud_server *server, * Returns 0 if the message was dispatched, -1 upon fatal error */ int -remoteDispatchClientRequest (struct qemud_server *server, - struct qemud_client *client, - struct qemud_client_message *msg) +remoteDispatchClientRequest(struct qemud_server *server, + struct qemud_client *client, + struct qemud_client_message *msg) { int ret; remote_error rerr; + bool qemu_call; DEBUG("prog=%d ver=%d type=%d status=%d serial=%d proc=%d", msg->hdr.prog, msg->hdr.vers, msg->hdr.type, @@ -370,22 +374,33 @@ remoteDispatchClientRequest (struct qemud_server *server, memset(&rerr, 0, sizeof rerr); /* Check version, etc. */ - if (msg->hdr.prog != REMOTE_PROGRAM) { + if (msg->hdr.prog == REMOTE_PROGRAM) + qemu_call = false; + else if (msg->hdr.prog == QEMU_PROGRAM) + qemu_call = true; + else { remoteDispatchFormatError (&rerr, - _("program mismatch (actual %x, expected %x)"), - msg->hdr.prog, REMOTE_PROGRAM); + _("program mismatch (actual %x, expected %x or %x)"), + msg->hdr.prog, REMOTE_PROGRAM, QEMU_PROGRAM); goto error; } - if (msg->hdr.vers != REMOTE_PROTOCOL_VERSION) { + + if (!qemu_call && msg->hdr.vers != REMOTE_PROTOCOL_VERSION) { remoteDispatchFormatError (&rerr, _("version mismatch (actual %x, expected %x)"), msg->hdr.vers, REMOTE_PROTOCOL_VERSION); goto error; } + else if (qemu_call && msg->hdr.vers != QEMU_PROTOCOL_VERSION) { + remoteDispatchFormatError (&rerr, + _("version mismatch (actual %x, expected %x)"), + msg->hdr.vers, QEMU_PROTOCOL_VERSION); + goto error; + } switch (msg->hdr.type) { case REMOTE_CALL: - return remoteDispatchClientCall(server, client, msg); + return remoteDispatchClientCall(server, client, msg, qemu_call); case REMOTE_STREAM: /* Since stream data is non-acked, async, we may continue to received @@ -427,10 +442,11 @@ error: * * Returns 0 if the reply was sent, or -1 upon fatal error */ -int +static int remoteDispatchClientCall (struct qemud_server *server, struct qemud_client *client, - struct qemud_client_message *msg) + struct qemud_client_message *msg, + bool qemu_protocol) { XDR xdr; remote_error rerr; @@ -469,7 +485,10 @@ remoteDispatchClientCall (struct qemud_server *server, } } - data = remoteGetDispatchData(msg->hdr.proc); + if (qemu_protocol) + data = qemuGetDispatchData(msg->hdr.proc); + else + data = remoteGetDispatchData(msg->hdr.proc); if (!data) { remoteDispatchFormatError (&rerr, _("unknown procedure: %d"), diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 4d8e7e2..3f13fb1 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -45,6 +45,7 @@ # include <rpc/types.h> # include <rpc/xdr.h> # include "remote_protocol.h" +# include "qemu_protocol.h" # include "logging.h" # include "threads.h" diff --git a/daemon/qemu_dispatch_args.h b/daemon/qemu_dispatch_args.h new file mode 100644 index 0000000..e278fa4 --- /dev/null +++ b/daemon/qemu_dispatch_args.h @@ -0,0 +1,5 @@ +/* Automatically generated by remote_generate_stubs.pl. + * Do not edit this file. Any changes you make will be lost. + */ + + qemu_monitor_command_args val_qemu_monitor_command_args; diff --git a/daemon/qemu_dispatch_prototypes.h b/daemon/qemu_dispatch_prototypes.h new file mode 100644 index 0000000..e6c83ba --- /dev/null +++ b/daemon/qemu_dispatch_prototypes.h @@ -0,0 +1,12 @@ +/* Automatically generated by remote_generate_stubs.pl. + * Do not edit this file. Any changes you make will be lost. + */ + +static int qemuDispatchMonitorCommand( + struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *err, + qemu_monitor_command_args *args, + qemu_monitor_command_ret *ret); diff --git a/daemon/qemu_dispatch_ret.h b/daemon/qemu_dispatch_ret.h new file mode 100644 index 0000000..492dcf9 --- /dev/null +++ b/daemon/qemu_dispatch_ret.h @@ -0,0 +1,5 @@ +/* Automatically generated by remote_generate_stubs.pl. + * Do not edit this file. Any changes you make will be lost. + */ + + qemu_monitor_command_ret val_qemu_monitor_command_ret; diff --git a/daemon/qemu_dispatch_table.h b/daemon/qemu_dispatch_table.h new file mode 100644 index 0000000..c196a3c --- /dev/null +++ b/daemon/qemu_dispatch_table.h @@ -0,0 +1,14 @@ +/* Automatically generated by remote_generate_stubs.pl. + * Do not edit this file. Any changes you make will be lost. + */ + +{ /* (unused) => 0 */ + .fn = NULL, + .args_filter = (xdrproc_t) xdr_void, + .ret_filter = (xdrproc_t) xdr_void, +}, +{ /* MonitorCommand => 1 */ + .fn = (dispatch_fn) qemuDispatchMonitorCommand, + .args_filter = (xdrproc_t) xdr_qemu_monitor_command_args, + .ret_filter = (xdrproc_t) xdr_qemu_monitor_command_ret, +}, diff --git a/daemon/remote.c b/daemon/remote.c index cb9e83d..118654c 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -57,6 +57,7 @@ #include "memory.h" #include "util.h" #include "stream.h" +#include "libvirt/libvirt-qemu.h" #define VIR_FROM_THIS VIR_FROM_REMOTE #define REMOTE_DEBUG(fmt, ...) DEBUG(fmt, __VA_ARGS__) @@ -81,11 +82,16 @@ static void make_nonnull_domain_snapshot (remote_nonnull_domain_snapshot *snapsh #include "remote_dispatch_prototypes.h" +#include "qemu_dispatch_prototypes.h" static const dispatch_data const dispatch_table[] = { #include "remote_dispatch_table.h" }; +static const dispatch_data const qemu_dispatch_table[] = { +#include "qemu_dispatch_table.h" +}; + const dispatch_data const *remoteGetDispatchData(int proc) { if (proc >= ARRAY_CARDINALITY(dispatch_table) || @@ -96,6 +102,16 @@ const dispatch_data const *remoteGetDispatchData(int proc) return &(dispatch_table[proc]); } +const dispatch_data const *qemuGetDispatchData(int proc) +{ + if (proc >= ARRAY_CARDINALITY(qemu_dispatch_table) || + qemu_dispatch_table[proc].fn == NULL) { + return NULL; + } + + return &(qemu_dispatch_table[proc]); +} + /* Prototypes */ static void remoteDispatchDomainEventSend (struct qemud_client *client, @@ -6564,6 +6580,35 @@ remoteDispatchDomainGetBlockInfo (struct qemud_server *server ATTRIBUTE_UNUSED, return 0; } +static int +qemuDispatchMonitorCommand (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + qemu_monitor_command_args *args, + qemu_monitor_command_ret *ret) +{ + virDomainPtr domain; + + domain = get_nonnull_domain(conn, args->domain); + if (domain == NULL) { + remoteDispatchConnError(rerr, conn); + return -1; + } + + if (virDomainQemuMonitorCommand(domain, args->cmd, &ret->result, + args->flags) == -1) { + virDomainFree(domain); + remoteDispatchConnError(rerr, conn); + return -1; + } + + virDomainFree(domain); + + return 0; +} + /*----- Helpers. -----*/ diff --git a/daemon/remote.h b/daemon/remote.h index 9db7432..fd8d381 100644 --- a/daemon/remote.h +++ b/daemon/remote.h @@ -35,6 +35,13 @@ typedef union { # include "remote_dispatch_ret.h" } dispatch_ret; +typedef union { +# include "qemu_dispatch_args.h" +} qemu_dispatch_args; + +typedef union { +# include "qemu_dispatch_ret.h" +} qemu_dispatch_ret; @@ -67,6 +74,7 @@ typedef struct { const dispatch_data const *remoteGetDispatchData(int proc); +const dispatch_data const *qemuGetDispatchData(int proc); diff --git a/daemon/remote_generate_stubs.pl b/daemon/remote_generate_stubs.pl index 8b26e3d..a8c4e6d 100755 --- a/daemon/remote_generate_stubs.pl +++ b/daemon/remote_generate_stubs.pl @@ -1,7 +1,16 @@ #!/usr/bin/perl -w # -# This script parses remote_protocol.x and produces lots of boilerplate -# code for both ends of the remote connection. +# This script parses remote_protocol.x or qemu_protocol.x and produces lots of +# boilerplate code for both ends of the remote connection. +# +# The first non-option argument specifies the prefix to be searched for, and +# output to, the boilerplate code. The second non-option argument is the +# file you want to operate on. For instance, to generate the dispatch table +# for both remote_protocol.x and qemu_protocol.x, you would run the +# following: +# +# remote_generate_stubs.pl -c -t remote ../src/remote/remote_protocol.x +# remote_generate_stubs.pl -t qemu ../src/remote/qemu_protocol.x # # By Richard Jones <rjones@redhat.com> @@ -10,8 +19,12 @@ use strict; use Getopt::Std; # Command line options. -our ($opt_p, $opt_t, $opt_a, $opt_r, $opt_d); -getopts ('ptard'); +our ($opt_p, $opt_t, $opt_a, $opt_r, $opt_d, $opt_c); +getopts ('ptardc'); + +my $structprefix = $ARGV[0]; +my $procprefix = uc $structprefix; +shift; # Convert name_of_call to NameOfCall. sub name_to_ProcName { @@ -25,47 +38,50 @@ sub name_to_ProcName { # opinion about the name, args and return type of each RPC. my ($name, $ProcName, $id, %calls, @calls); -# REMOTE_PROC_CLOSE has no args or ret. -$calls{close} = { - name => "close", - ProcName => "Close", - UC_NAME => "CLOSE", - args => "void", - ret => "void", -}; +# only generate a close method if -c was passed +if ($opt_c) { + # REMOTE_PROC_CLOSE has no args or ret. + $calls{close} = { + name => "close", + ProcName => "Close", + UC_NAME => "CLOSE", + args => "void", + ret => "void", + }; +} while (<>) { - if (/^struct remote_(.*)_args/) { + if (/^struct ${structprefix}_(.*)_args/) { $name = $1; $ProcName = name_to_ProcName ($name); - die "duplicate definition of remote_${name}_args" + die "duplicate definition of ${structprefix}_${name}_args" if exists $calls{$name}; $calls{$name} = { name => $name, ProcName => $ProcName, UC_NAME => uc $name, - args => "remote_${name}_args", + args => "${structprefix}_${name}_args", ret => "void", }; - } elsif (/^struct remote_(.*)_ret/) { + } elsif (/^struct ${structprefix}_(.*)_ret/) { $name = $1; $ProcName = name_to_ProcName ($name); if (exists $calls{$name}) { - $calls{$name}->{ret} = "remote_${name}_ret"; + $calls{$name}->{ret} = "${structprefix}_${name}_ret"; } else { $calls{$name} = { name => $name, ProcName => $ProcName, UC_NAME => uc $name, args => "void", - ret => "remote_${name}_ret" + ret => "${structprefix}_${name}_ret" } } - } elsif (/^struct remote_(.*)_msg/) { + } elsif (/^struct ${structprefix}_(.*)_msg/) { $name = $1; $ProcName = name_to_ProcName ($name); @@ -73,9 +89,9 @@ while (<>) { name => $name, ProcName => $ProcName, UC_NAME => uc $name, - msg => "remote_${name}_msg" + msg => "${structprefix}_${name}_msg" } - } elsif (/^\s*REMOTE_PROC_(.*?)\s+=\s+(\d+),?$/) { + } elsif (/^\s*${procprefix}_PROC_(.*?)\s+=\s+(\d+),?$/) { $name = lc $1; $id = $2; $ProcName = name_to_ProcName ($name); @@ -111,7 +127,7 @@ elsif ($opt_p) { # Skip things which are REMOTE_MESSAGE next if $calls{$_}->{msg}; - print "static int remoteDispatch$calls{$_}->{ProcName}(\n"; + print "static int ${structprefix}Dispatch$calls{$_}->{ProcName}(\n"; print " struct qemud_server *server,\n"; print " struct qemud_client *client,\n"; print " virConnectPtr conn,\n"; @@ -152,7 +168,7 @@ elsif ($opt_t) { for ($id = 0 ; $id <= $#calls ; $id++) { if (defined $calls[$id] && !$calls[$id]->{msg}) { print "{ /* $calls[$id]->{ProcName} => $id */\n"; - print " .fn = (dispatch_fn) remoteDispatch$calls[$id]->{ProcName},\n"; + print " .fn = (dispatch_fn) ${structprefix}Dispatch$calls[$id]->{ProcName},\n"; if ($calls[$id]->{args} ne "void") { print " .args_filter = (xdrproc_t) xdr_$calls[$id]->{args},\n"; } else { diff --git a/src/Makefile.am b/src/Makefile.am index 9cf9d67..1f993a8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -155,7 +155,9 @@ REMOTE_DRIVER_SOURCES = \ gnutls_1_0_compat.h \ remote/remote_driver.c remote/remote_driver.h \ remote/remote_protocol.c \ - remote/remote_protocol.h + remote/remote_protocol.h \ + remote/qemu_protocol.c \ + remote/qemu_protocol.h EXTRA_DIST += remote/remote_protocol.x remote/rpcgen_fix.pl @@ -489,7 +491,7 @@ if HAVE_RPCGEN # Support for non-GLIB rpcgen is here as a convenience for # non-Linux people needing to test changes during dev. # -rpcgen: +rpcgen-normal: rm -f rp.c-t rp.h-t rp.c-t1 rp.c-t2 rp.h-t1 $(RPCGEN) -h -o rp.h-t $(srcdir)/remote/remote_protocol.x $(RPCGEN) -c -o rp.c-t $(srcdir)/remote/remote_protocol.x @@ -506,6 +508,37 @@ else mv -f rp.h-t $(srcdir)/remote/remote_protocol.h mv -f rp.c-t $(srcdir)/remote/remote_protocol.c endif + +rpcgen-qemu: + rm -f rp_qemu.c-t rp_qemu.h-t rp_qemu.c-t1 rp_qemu.c-t2 rp_qemu.h-t1 + $(RPCGEN) -h -o rp_qemu.h-t $(srcdir)/remote/qemu_protocol.x + $(RPCGEN) -c -o rp_qemu.c-t $(srcdir)/remote/qemu_protocol.x +if HAVE_GLIBC_RPCGEN + perl -w $(srcdir)/remote/rpcgen_fix.pl rp_qemu.h-t > rp_qemu.h-t1 + perl -w $(srcdir)/remote/rpcgen_fix.pl rp_qemu.c-t > rp_qemu.c-t1 + (echo '#include <config.h>'; cat rp_qemu.c-t1) > rp_qemu.c-t2 + chmod 0444 rp_qemu.c-t2 rp_qemu.h-t1 + mv -f rp_qemu.h-t1 $(srcdir)/remote/qemu_protocol.h + mv -f rp_qemu.c-t2 $(srcdir)/remote/qemu_protocol.c + rm -f rp_qemu.c-t rp_qemu.h-t rp_qemu.c-t1 +else + chmod 0444 rp_qemu.c-t rp_qemu.h-t + mv -f rp_qemu.h-t $(srcdir)/remote/qemu_protocol.h + mv -f rp_qemu.c-t $(srcdir)/remote/qemu_protocol.c +endif + +# +# Maintainer-only target for re-generating the derived .c/.h source +# files, which are actually derived from the .x file. +# +# For committing protocol changes to GIT, the GLIBC rpcgen *must* +# be used. +# +# Support for non-GLIB rpcgen is here as a convenience for +# non-Linux people needing to test changes during dev. +# +rpcgen: rpcgen-normal rpcgen-qemu + endif remote/remote_protocol.c: remote/remote_protocol.h diff --git a/src/remote/qemu_protocol.c b/src/remote/qemu_protocol.c new file mode 100644 index 0000000..81916ed --- /dev/null +++ b/src/remote/qemu_protocol.c @@ -0,0 +1,41 @@ +#include <config.h> +/* + * Please do not edit this file. + * It was generated using rpcgen. + */ + +#include "./remote/qemu_protocol.h" +#include "internal.h" +#include "remote_protocol.h" +#include <arpa/inet.h> + +bool_t +xdr_qemu_monitor_command_args (XDR *xdrs, qemu_monitor_command_args *objp) +{ + + if (!xdr_remote_nonnull_domain (xdrs, &objp->domain)) + return FALSE; + if (!xdr_remote_nonnull_string (xdrs, &objp->cmd)) + return FALSE; + if (!xdr_int (xdrs, &objp->flags)) + return FALSE; + return TRUE; +} + +bool_t +xdr_qemu_monitor_command_ret (XDR *xdrs, qemu_monitor_command_ret *objp) +{ + + if (!xdr_remote_nonnull_string (xdrs, &objp->result)) + return FALSE; + return TRUE; +} + +bool_t +xdr_qemu_procedure (XDR *xdrs, qemu_procedure *objp) +{ + + if (!xdr_enum (xdrs, (enum_t *) objp)) + return FALSE; + return TRUE; +} diff --git a/src/remote/qemu_protocol.h b/src/remote/qemu_protocol.h new file mode 100644 index 0000000..b822187 --- /dev/null +++ b/src/remote/qemu_protocol.h @@ -0,0 +1,57 @@ +/* + * Please do not edit this file. + * It was generated using rpcgen. + */ + +#ifndef _RP_QEMU_H_RPCGEN +#define _RP_QEMU_H_RPCGEN + +#include <rpc/rpc.h> + + +#ifdef __cplusplus +extern "C" { +#endif + +#include "internal.h" +#include "remote_protocol.h" +#include <arpa/inet.h> + +struct qemu_monitor_command_args { + remote_nonnull_domain domain; + remote_nonnull_string cmd; + int flags; +}; +typedef struct qemu_monitor_command_args qemu_monitor_command_args; + +struct qemu_monitor_command_ret { + remote_nonnull_string result; +}; +typedef struct qemu_monitor_command_ret qemu_monitor_command_ret; +#define QEMU_PROGRAM 0x20008087 +#define QEMU_PROTOCOL_VERSION 1 + +enum qemu_procedure { + QEMU_PROC_MONITOR_COMMAND = 1, +}; +typedef enum qemu_procedure qemu_procedure; + +/* the xdr functions */ + +#if defined(__STDC__) || defined(__cplusplus) +extern bool_t xdr_qemu_monitor_command_args (XDR *, qemu_monitor_command_args*); +extern bool_t xdr_qemu_monitor_command_ret (XDR *, qemu_monitor_command_ret*); +extern bool_t xdr_qemu_procedure (XDR *, qemu_procedure*); + +#else /* K&R C */ +extern bool_t xdr_qemu_monitor_command_args (); +extern bool_t xdr_qemu_monitor_command_ret (); +extern bool_t xdr_qemu_procedure (); + +#endif /* K&R C */ + +#ifdef __cplusplus +} +#endif + +#endif /* !_RP_QEMU_H_RPCGEN */ diff --git a/src/remote/qemu_protocol.x b/src/remote/qemu_protocol.x new file mode 100644 index 0000000..1d07895 --- /dev/null +++ b/src/remote/qemu_protocol.x @@ -0,0 +1,46 @@ +/* -*- c -*- + * qemu_protocol.x: private protocol for communicating between + * remote_internal driver and libvirtd. This protocol is + * internal and may change at any time. + * + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Chris Lalancette <clalance@redhat.com> + */ + +%#include "internal.h" +%#include "remote_protocol.h" +%#include <arpa/inet.h> + +/*----- Protocol. -----*/ +struct qemu_monitor_command_args { + remote_nonnull_domain domain; + remote_nonnull_string cmd; + int flags; +}; + +struct qemu_monitor_command_ret { + remote_nonnull_string result; +}; + +/* Define the program number, protocol version and procedure numbers here. */ +const QEMU_PROGRAM = 0x20008087; +const QEMU_PROTOCOL_VERSION = 1; + +enum qemu_procedure { + QEMU_PROC_MONITOR_COMMAND = 1 +}; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 18d2320..10bd5ef 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -77,6 +77,7 @@ #include "qparams.h" #include "remote_driver.h" #include "remote_protocol.h" +#include "qemu_protocol.h" #include "memory.h" #include "util.h" #include "event.h" @@ -199,8 +200,9 @@ struct private_data { }; enum { - REMOTE_CALL_IN_OPEN = 1, - REMOTE_CALL_QUIET_MISSING_RPC = 2, + REMOTE_CALL_IN_OPEN = (1 << 0), + REMOTE_CALL_QUIET_MISSING_RPC = (1 << 1), + REMOTE_QEMU_CALL = (1 << 2), }; @@ -8859,9 +8861,49 @@ done: /*----------------------------------------------------------------------*/ +static int +remoteQemuDomainMonitorCommand (virDomainPtr domain, const char *cmd, + char **result, unsigned int flags) +{ + int rv = -1; + qemu_monitor_command_args args; + qemu_monitor_command_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.domain, domain); + args.cmd = (char *)cmd; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (domain->conn, priv, REMOTE_QEMU_CALL, QEMU_PROC_MONITOR_COMMAND, + (xdrproc_t) xdr_qemu_monitor_command_args, (char *) &args, + (xdrproc_t) xdr_qemu_monitor_command_ret, (char *) &ret) == -1) + goto done; + + *result = strdup(ret.result); + if (*result == NULL) { + + virReportOOMError(); + goto cleanup; + } + + rv = 0; + +cleanup: + xdr_free ((xdrproc_t) xdr_qemu_monitor_command_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + +/*----------------------------------------------------------------------*/ static struct remote_thread_call * prepareCall(struct private_data *priv, + int flags, int proc_nr, xdrproc_t args_filter, char *args, xdrproc_t ret_filter, char *ret) @@ -8889,8 +8931,14 @@ prepareCall(struct private_data *priv, rv->ret = ret; rv->want_reply = 1; - hdr.prog = REMOTE_PROGRAM; - hdr.vers = REMOTE_PROTOCOL_VERSION; + if (flags & REMOTE_QEMU_CALL) { + hdr.prog = QEMU_PROGRAM; + hdr.vers = QEMU_PROTOCOL_VERSION; + } + else { + hdr.prog = REMOTE_PROGRAM; + hdr.vers = REMOTE_PROTOCOL_VERSION; + } hdr.proc = proc_nr; hdr.type = REMOTE_CALL; hdr.serial = rv->serial; @@ -9237,7 +9285,6 @@ remoteIODecodeMessageLength(struct private_data *priv) { static int processCallDispatchReply(virConnectPtr conn, struct private_data *priv, - int in_open, remote_message_header *hdr, XDR *xdr); @@ -9249,18 +9296,19 @@ processCallDispatchMessage(virConnectPtr conn, struct private_data *priv, static int processCallDispatchStream(virConnectPtr conn, struct private_data *priv, - int in_open, remote_message_header *hdr, XDR *xdr); static int processCallDispatch(virConnectPtr conn, struct private_data *priv, - int in_open) { + int flags) { XDR xdr; struct remote_message_header hdr; int len = priv->bufferLength - 4; int rv = -1; + int expectedprog; + int expectedvers; /* Length word has already been read */ priv->bufferOffset = 4; @@ -9274,35 +9322,40 @@ processCallDispatch(virConnectPtr conn, struct private_data *priv, priv->bufferOffset += xdr_getpos(&xdr); + expectedprog = REMOTE_PROGRAM; + expectedvers = REMOTE_PROTOCOL_VERSION; + if (flags & REMOTE_QEMU_CALL) { + expectedprog = QEMU_PROGRAM; + expectedvers = QEMU_PROTOCOL_VERSION; + } + /* Check program, version, etc. are what we expect. */ - if (hdr.prog != REMOTE_PROGRAM) { + if (hdr.prog != expectedprog) { remoteError(VIR_ERR_RPC, _("unknown program (received %x, expected %x)"), - hdr.prog, REMOTE_PROGRAM); + hdr.prog, expectedprog); return -1; } - if (hdr.vers != REMOTE_PROTOCOL_VERSION) { + if (hdr.vers != expectedvers) { remoteError(VIR_ERR_RPC, _("unknown protocol version (received %x, expected %x)"), - hdr.vers, REMOTE_PROTOCOL_VERSION); + hdr.vers, expectedvers); return -1; } switch (hdr.type) { case REMOTE_REPLY: /* Normal RPC replies */ - rv = processCallDispatchReply(conn, priv, in_open, - &hdr, &xdr); + rv = processCallDispatchReply(conn, priv, &hdr, &xdr); break; case REMOTE_MESSAGE: /* Async notifications */ - rv = processCallDispatchMessage(conn, priv, in_open, + rv = processCallDispatchMessage(conn, priv, flags & REMOTE_CALL_IN_OPEN, &hdr, &xdr); break; case REMOTE_STREAM: /* Stream protocol */ - rv = processCallDispatchStream(conn, priv, in_open, - &hdr, &xdr); + rv = processCallDispatchStream(conn, priv, &hdr, &xdr); break; default: @@ -9321,7 +9374,6 @@ processCallDispatch(virConnectPtr conn, struct private_data *priv, static int processCallDispatchReply(virConnectPtr conn ATTRIBUTE_UNUSED, struct private_data *priv, - int in_open ATTRIBUTE_UNUSED, remote_message_header *hdr, XDR *xdr) { struct remote_thread_call *thecall; @@ -9441,7 +9493,6 @@ processCallDispatchMessage(virConnectPtr conn, struct private_data *priv, static int processCallDispatchStream(virConnectPtr conn ATTRIBUTE_UNUSED, struct private_data *priv, - int in_open ATTRIBUTE_UNUSED, remote_message_header *hdr, XDR *xdr) { struct private_stream_data *privst; @@ -9547,7 +9598,7 @@ processCallDispatchStream(virConnectPtr conn ATTRIBUTE_UNUSED, static int remoteIOHandleInput(virConnectPtr conn, struct private_data *priv, - int in_open) + int flags) { /* Read as much data as is available, until we get * EAGAIN @@ -9574,7 +9625,7 @@ remoteIOHandleInput(virConnectPtr conn, struct private_data *priv, * next iteration. */ } else { - ret = processCallDispatch(conn, priv, in_open); + ret = processCallDispatch(conn, priv, flags); priv->bufferOffset = priv->bufferLength = 0; /* * We've completed one call, so return even @@ -9597,7 +9648,7 @@ remoteIOHandleInput(virConnectPtr conn, struct private_data *priv, static int remoteIOEventLoop(virConnectPtr conn, struct private_data *priv, - int in_open, + int flags, struct remote_thread_call *thiscall) { struct pollfd fds[2]; @@ -9687,7 +9738,7 @@ remoteIOEventLoop(virConnectPtr conn, } if (fds[0].revents & POLLIN) { - if (remoteIOHandleInput(conn, priv, in_open) < 0) + if (remoteIOHandleInput(conn, priv, flags) < 0) goto error; } @@ -9892,9 +9943,7 @@ remoteIO(virConnectPtr conn, if (priv->watch >= 0) virEventUpdateHandle(priv->watch, 0); - rv = remoteIOEventLoop(conn, priv, - flags & REMOTE_CALL_IN_OPEN ? 1 : 0, - thiscall); + rv = remoteIOEventLoop(conn, priv, flags, thiscall); if (priv->watch >= 0) virEventUpdateHandle(priv->watch, VIR_EVENT_HANDLE_READABLE); @@ -9966,14 +10015,14 @@ cleanup: */ static int call (virConnectPtr conn, struct private_data *priv, - int flags /* if we are in virConnectOpen */, + int flags, int proc_nr, xdrproc_t args_filter, char *args, xdrproc_t ret_filter, char *ret) { struct remote_thread_call *thiscall; - thiscall = prepareCall(priv, proc_nr, args_filter, args, + thiscall = prepareCall(priv, flags, proc_nr, args_filter, args, ret_filter, ret); if (!thiscall) { @@ -10302,7 +10351,7 @@ static virDriver remote_driver = { remoteDomainSnapshotCurrent, /* domainSnapshotCurrent */ remoteDomainRevertToSnapshot, /* domainRevertToSnapshot */ remoteDomainSnapshotDelete, /* domainSnapshotDelete */ - NULL, /* qemuDomainMonitorCommand */ + remoteQemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 33bbac5..59b18ae 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -3611,12 +3611,60 @@ xdr_remote_message_status (XDR *xdrs, remote_message_status *objp) bool_t xdr_remote_message_header (XDR *xdrs, remote_message_header *objp) { + register int32_t *buf; + + + if (xdrs->x_op == XDR_ENCODE) { + buf = (int32_t*)XDR_INLINE (xdrs, 3 * BYTES_PER_XDR_UNIT); + if (buf == NULL) { + if (!xdr_u_int (xdrs, &objp->prog)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->vers)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->proc)) + return FALSE; + + } else { + (void)IXDR_PUT_U_INT32(buf, objp->prog); + (void)IXDR_PUT_U_INT32(buf, objp->vers); + (void)IXDR_PUT_U_INT32(buf, objp->proc); + } + if (!xdr_remote_message_type (xdrs, &objp->type)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->serial)) + return FALSE; + if (!xdr_remote_message_status (xdrs, &objp->status)) + return FALSE; + return TRUE; + } else if (xdrs->x_op == XDR_DECODE) { + buf = (int32_t*)XDR_INLINE (xdrs, 3 * BYTES_PER_XDR_UNIT); + if (buf == NULL) { + if (!xdr_u_int (xdrs, &objp->prog)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->vers)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->proc)) + return FALSE; + + } else { + objp->prog = IXDR_GET_U_LONG(buf); + objp->vers = IXDR_GET_U_LONG(buf); + objp->proc = IXDR_GET_U_LONG(buf); + } + if (!xdr_remote_message_type (xdrs, &objp->type)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->serial)) + return FALSE; + if (!xdr_remote_message_status (xdrs, &objp->status)) + return FALSE; + return TRUE; + } if (!xdr_u_int (xdrs, &objp->prog)) return FALSE; if (!xdr_u_int (xdrs, &objp->vers)) return FALSE; - if (!xdr_remote_procedure (xdrs, &objp->proc)) + if (!xdr_u_int (xdrs, &objp->proc)) return FALSE; if (!xdr_remote_message_type (xdrs, &objp->type)) return FALSE; diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index cf8e2cb..1dff3fe 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -2249,7 +2249,7 @@ typedef enum remote_message_status remote_message_status; struct remote_message_header { u_int prog; u_int vers; - remote_procedure proc; + u_int proc; remote_message_type type; u_int serial; remote_message_status status; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index db412be..b1c6ccf 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2122,7 +2122,7 @@ const REMOTE_MESSAGE_HEADER_XDR_LEN = 4; struct remote_message_header { unsigned prog; /* REMOTE_PROGRAM */ unsigned vers; /* REMOTE_PROTOCOL_VERSION */ - remote_procedure proc; /* REMOTE_PROC_x */ + unsigned proc; /* REMOTE_PROC_x */ remote_message_type type; unsigned serial; /* Serial number of message. */ remote_message_status status; -- 1.6.6.1
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
Since we are adding a new "per-hypervisor" protocol, we make it so that the qemu remote protocol uses a new PROTOCOL and PROGRAM number. This allows us to easily distinguish it from the normal REMOTE protocol.
This necessitates changing the proc in remote_message_header from a "remote_procedure" to an "unsigned", which should be the same size (and thus preserve the on-wire protocol).
Changes since v1: - Fixed up a couple of script problems in remote_generate_stubs.pl - Switch an int flag to a bool in dispatch.c
ACK - the patch looked clean to me. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Changes since v1: - Add virsh.pod documentation Signed-off-by: Chris Lalancette <clalance@redhat.com> --- tools/Makefile.am | 1 + tools/virsh.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 16 +++++++++++++++ 3 files changed, 72 insertions(+), 0 deletions(-) diff --git a/tools/Makefile.am b/tools/Makefile.am index fd05e8b..c74df19 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -39,6 +39,7 @@ virsh_LDADD = \ $(STATIC_BINARIES) \ $(WARN_CFLAGS) \ ../src/libvirt.la \ + ../src/libvirt-qemu.la \ ../gnulib/lib/libgnu.la \ $(VIRSH_LIBS) virsh_CFLAGS = \ diff --git a/tools/virsh.c b/tools/virsh.c index e07fef3..6a08994 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -50,6 +50,7 @@ #include "util.h" #include "memory.h" #include "xml.h" +#include "libvirt/libvirt-qemu.h" static char *progname; @@ -9134,6 +9135,58 @@ cleanup: } /* + * "qemu-monitor-command" command + */ +static const vshCmdInfo info_qemu_monitor_command[] = { + {"help", N_("Qemu Monitor Command")}, + {"desc", N_("Qemu Monitor Command")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_qemu_monitor_command[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"cmd", VSH_OT_DATA, VSH_OFLAG_REQ, N_("command")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + int ret = FALSE; + char *monitor_cmd; + char *result = NULL; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + goto cleanup; + + dom = vshCommandOptDomain(ctl, cmd, NULL); + if (dom == NULL) + goto cleanup; + + monitor_cmd = vshCommandOptString(cmd, "cmd", NULL); + if (monitor_cmd == NULL) { + vshError(ctl, "%s", _("missing monitor command")); + goto cleanup; + } + + if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, 0) < 0) + goto cleanup; + + printf("%s\n", result); + + ret = TRUE; + +cleanup: + VIR_FREE(result); + if (dom) + virDomainFree(dom); + + return ret; +} + + +/* * Commands */ static const vshCmdDef commands[] = { @@ -9297,6 +9350,8 @@ static const vshCmdDef commands[] = { {"snapshot-list", cmdSnapshotList, opts_snapshot_list, info_snapshot_list}, {"snapshot-revert", cmdDomainSnapshotRevert, opts_snapshot_revert, info_snapshot_revert}, + {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, info_qemu_monitor_command}, + {NULL, NULL, NULL, NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 2b2227f..6718fb7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1026,6 +1026,22 @@ variables, and defaults to C<vi>. =back +=head1 QEMU-SPECIFIC COMMANDS + +NOTE: Use of the following commands is B<strongly> discouraged. They +can cause libvirt to become confused and do the wrong thing on subsequent +operations. If you use these commands, and something breaks, you get to +keep both pieces. + +=over 4 + +=item B<qemu-monitor-command> I<domain> I<command> + +Send an arbitrary monitor command I<command> to domain I<domain> through the +qemu monitor. The results of the command will be printed on stdout. + +=back + =head1 ENVIRONMENT The following environment variables can be set to alter the behaviour -- 1.6.6.1
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
Changes since v1: - Add virsh.pod documentation
Signed-off-by: Chris Lalancette <clalance@redhat.com>
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Thanks to DV for knocking together the Relax-NG changes quickly for me. Changes since v1: - Change the domain.rng to correspond to the new schema - Don't allocate caps->ns in testQemuCapsInit since it is a static table Signed-off-by: Chris Lalancette <clalance@redhat.com> --- docs/schemas/domain.rng | 26 +++++++++++++++++ src/qemu/qemu_driver.c | 16 +++++----- src/qemu/qemu_driver.h | 15 ++++++++++ tests/qemuargv2xmltest.c | 5 +++ .../qemuxml2argv-qemu-ns-no-env.args | 1 + .../qemuxml2argv-qemu-ns-no-env.xml | 28 ++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml | 30 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/testutilsqemu.c | 6 ++++ 10 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index fd57917..c1f5694 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -41,6 +41,9 @@ <optional> <ref name="seclabel"/> </optional> + <optional> + <ref name='qemucmdline'/> + </optional> </interleave> </element> </define> @@ -1658,6 +1661,29 @@ </define> <!-- + Optional hypervisor extensions in their own namespace: + QEmu + --> + <define name="qemucmdline"> + <element name="commandline" ns="http://libvirt.org/schemas/domain/qemu/1.0"> + <zeroOrMore> + <element name="arg"> + <attribute name='value'/> + </element> + </zeroOrMore> + <zeroOrMore> + <element name="env"> + <attribute name='name'/> + <optional> + <attribute name='value'/> + </optional> + <empty/> + </element> + </zeroOrMore> + </element> + </define> + + <!-- Type library Our unsignedInt doesn't allow a leading '+' in its lexical form diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cd4324b..13c43c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -535,7 +535,7 @@ static void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virD } } -static void qemuDomainDefNamespaceFree(void *nsdata) +void qemuDomainDefNamespaceFree(void *nsdata) { qemuDomainCmdlineDefPtr cmd = nsdata; unsigned int i; @@ -555,10 +555,10 @@ static void qemuDomainDefNamespaceFree(void *nsdata) VIR_FREE(cmd); } -static int qemuDomainDefNamespaceParse(xmlDocPtr xml, - xmlNodePtr root, - xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED, - void **data) +int qemuDomainDefNamespaceParse(xmlDocPtr xml, + xmlNodePtr root, + xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED, + void **data) { qemuDomainCmdlineDefPtr cmd = NULL; xmlNsPtr ns; @@ -648,8 +648,8 @@ error: return -1; } -static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, - void *nsdata) +int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, + void *nsdata) { qemuDomainCmdlineDefPtr cmd = nsdata; unsigned int i; @@ -671,7 +671,7 @@ static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, return 0; } -static const char *qemuDomainDefNamespaceHref(void) +const char *qemuDomainDefNamespaceHref(void) { return "xmlns:qemu='" QEMU_NAMESPACE_HREF "'"; } diff --git a/src/qemu/qemu_driver.h b/src/qemu/qemu_driver.h index 95b8bff..99e011f 100644 --- a/src/qemu/qemu_driver.h +++ b/src/qemu/qemu_driver.h @@ -27,6 +27,8 @@ # include <config.h> +# include <libxml/xpath.h> + # include "internal.h" # if HAVE_LINUX_KVM_H @@ -49,4 +51,17 @@ int qemuRegister(void); +int qemuMonitorCommand(virDomainPtr domain, const char *cmd, + char **result, unsigned int flags); + + +void qemuDomainDefNamespaceFree(void *nsdata); +int qemuDomainDefNamespaceParse(xmlDocPtr xml, + xmlNodePtr root, + xmlXPathContextPtr ctxt, + void **data); +int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, + void *nsdata); +const char *qemuDomainDefNamespaceHref(void); + #endif /* QEMUD_DRIVER_H */ diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index bd81018..501bc75 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -228,6 +228,11 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + /* it's not really possible to handle the environment variables in a + * generic way, so we run the qemu namespace test without them + */ + DO_TEST("qemu-ns-no-env", 0); + free(driver.stateDir); virCapabilitiesFree(driver.caps); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.args b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.args new file mode 100644 index 0000000..b02eb26 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -unknown parameter diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.xml b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.xml new file mode 100644 index 0000000..38399f0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.xml @@ -0,0 +1,28 @@ +<domain type='qemu' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + </devices> + <qemu:commandline> + <qemu:arg value='-unknown'/> + <qemu:arg value='parameter'/> + </qemu:commandline> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args new file mode 100644 index 0000000..b7055d5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test NS=ns BAR= /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -unknown parameter diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml new file mode 100644 index 0000000..c48c248 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml @@ -0,0 +1,30 @@ +<domain type='qemu' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + </devices> + <qemu:commandline> + <qemu:arg value='-unknown'/> + <qemu:arg value='parameter'/> + <qemu:env name='NS' value='ns'/> + <qemu:env name='BAR'/> + </qemu:commandline> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ad1379b..e16f3b2 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -368,6 +368,8 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + DO_TEST("qemu-ns", 0); + free(driver.stateDir); virCapabilitiesFree(driver.caps); diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 7fee21a..3659ff6 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -7,6 +7,7 @@ # include "testutils.h" # include "memory.h" # include "cpu_conf.h" +# include "qemu/qemu_driver.h" static virCapsGuestMachinePtr *testQemuAllocMachines(int *nmachines) { @@ -99,6 +100,11 @@ virCapsPtr testQemuCapsInit(void) { (machines = testQemuAllocMachines(&nmachines)) == NULL) goto cleanup; + caps->ns.parse = qemuDomainDefNamespaceParse; + caps->ns.free = qemuDomainDefNamespaceFree; + caps->ns.format = qemuDomainDefNamespaceFormatXML; + caps->ns.href = qemuDomainDefNamespaceHref; + if ((guest = virCapabilitiesAddGuest(caps, "hvm", "i686", 32, "/usr/bin/qemu", NULL, nmachines, machines)) == NULL) -- 1.6.6.1
On 07/01/2010 02:59 PM, Chris Lalancette wrote:
Thanks to DV for knocking together the Relax-NG changes quickly for me.
Changes since v1: - Change the domain.rng to correspond to the new schema - Don't allocate caps->ns in testQemuCapsInit since it is a static table
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- docs/schemas/domain.rng | 26 +++++++++++++++++
Can we also update docs/formatdomain.html.in to mention this?
src/qemu/qemu_driver.c | 16 +++++----- src/qemu/qemu_driver.h | 15 ++++++++++ tests/qemuargv2xmltest.c | 5 +++ .../qemuxml2argv-qemu-ns-no-env.args | 1 + .../qemuxml2argv-qemu-ns-no-env.xml | 28 ++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml | 30 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/testutilsqemu.c | 6 ++++ 10 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml
ACK to what is here. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Chris Lalancette -
Eric Blake