[libvirt] [RFC PATCH 0/6] New API to normalize the device XML
The initial purpose was to fix a regression for detaching device, (introduced by commit ea7182c29). There was a patch posted to resolve the problem: https://www.redhat.com/archives/libvir-list/2011-December/msg00818.html But as Eric suggested, it's not the ideal way to go, we never known how many stuffs like <address> will be involved in future. So new API to invoke the internal parsing functions might be a right way then. However, things are not that simple (an API without internal driver support, invoking the parsing functions directly). As the domain conf is a neccessary argument to parse the device XML. (e.g. for an Input device). Although we can bypass that by modification on virDomainDeviceDefParse, it could be trap as we will parse the device XML in another way which is different with the real parsing. So finally I choosed to implement the driver support for the new API. There might be something I didn't take into consideration, (e.g. Do we need some flags for the XML parsing and formating?). But it can demo the thought good enough. On the other hand, I'm wondering if it's deserved to introduce such an API, comparing it's usage, is it two heavy if we need the internal drivers support for such an API? Any thoughts and feedback is welcomed. [PATCH 1/6] normalize_xml: Define the new API [PATCH 2/6] normalize_xml: Implement the new API [PATCH 3/6] normalize_xml: Wire up the remote protocol [PATCH 4/6] normalize_xml: New internal API to format device XML [PATCH 5/6] normalize_xml: Implement qemu driver support [PATCH 6/6] normalize_xml: New virsh command to normalize device XML Regards, Osier
--- include/libvirt/libvirt.h.in | 3 +++ src/driver.h | 5 +++++ src/libvirt_public.syms | 5 +++++ 3 files changed, 13 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad6fcce..5f2c46b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3600,6 +3600,9 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count); +char *virDomainNormalizeDeviceXML(virDomainPtr dom, + const char *device_xml, + unsigned int flags); #ifdef __cplusplus } #endif diff --git a/src/driver.h b/src/driver.h index ec4abf3..301960d 100644 --- a/src/driver.h +++ b/src/driver.h @@ -793,6 +793,10 @@ typedef int int *nparams, unsigned int flags); +typedef char * + (*virDrvDomainNormalizeDeviceXML)(virDomainPtr dom, + const char *device_xml, + unsigned int flags); /** * _virDriver: * @@ -961,6 +965,7 @@ struct _virDriver { virDrvNodeSuspendForDuration nodeSuspendForDuration; virDrvDomainSetBlockIoTune domainSetBlockIoTune; virDrvDomainGetBlockIoTune domainGetBlockIoTune; + virDrvDomainNormalizeDeviceXML domainNormalizeDeviceXML; }; typedef int diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ca7216..79edc13 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -516,4 +516,9 @@ LIBVIRT_0.9.9 { virDomainSetNumaParameters; } LIBVIRT_0.9.8; +LIBVIRT_0.9.10 { + global: + virDomainNormalizeDeviceXML; +} LIBVIRT_0.9.9; + # .... define new API here using predicted next version number .... -- 1.7.7.3
On 01/09/2012 07:29 AM, Osier Yang wrote:
---
Mention the API name in the commit message, so that grepping 'git log' makes it easier to find when the API was added.
include/libvirt/libvirt.h.in | 3 +++ src/driver.h | 5 +++++ src/libvirt_public.syms | 5 +++++ 3 files changed, 13 insertions(+), 0 deletions(-)
I would combine this patch with 2/6 - that is, the declaration and the documentation should belong in the same patch (otherwise, I don't know if the declaration makes sense).
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad6fcce..5f2c46b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3600,6 +3600,9 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count);
+char *virDomainNormalizeDeviceXML(virDomainPtr dom,
This should _not_ be tied to a specific domain, but should be a general-purpose virConnectPtr operation. After all, it has no impact on the state of current libvirt objects in use; it is merely a way to expose the mapping of XML into conf/*_conf.c C structures and back into XML format. See also my comments on 0/6 about making it more generic, to cover more than just <domain>. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/libvirt.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 896d151..223f07b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17867,3 +17867,55 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainNormalizeDeviceXML: + * @dom: Pointer to domain object + * @device_xml: Description of the device XML to be normalized + * @flags: currently unused, pass 0 + * + * Normalize the incoming device XML, and returned the formated XML. + * + * The mainly use of this function is to format the incoming device + * XML as what the device is represented internally. + * + * Returns NULL in case of error, or the formated XML in case of success. + */ +char * +virDomainNormalizeDeviceXML(virDomainPtr dom, + const char *device_xml, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "device_xml=%p, flags=%x", + NULLSTR(device_xml), flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + if (device_xml == NULL) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + conn = dom->conn; + + if (conn->driver->domainNormalizeDeviceXML) { + char *ret = NULL; + ret = conn->driver->domainNormalizeDeviceXML(dom, device_xml, flags); + if (!ret) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return NULL; +} -- 1.7.7.3
On 01/09/2012 07:29 AM, Osier Yang wrote:
--- src/libvirt.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 896d151..223f07b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17867,3 +17867,55 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainNormalizeDeviceXML: + * @dom: Pointer to domain object + * @device_xml: Description of the device XML to be normalized + * @flags: currently unused, pass 0
We should definitely support VIR_DOMAIN_XML_INACTIVE off the bat, as that affects the output that you will get from conf/domain_conf.c. And since VIR_DOMAIN_XML_INACTIVE != VIR_INTERFACE_XML_INACTIVE, we have to be careful how we represent things.
+ * + * Normalize the incoming device XML, and returned the formated XML.
s/formated/formatted/
+ * + * The mainly use of this function is to format the incoming device
s/mainly/main/
+ * XML as what the device is represented internally. + * + * Returns NULL in case of error, or the formated XML in case of success.
s/formated/formatted/ See my proposal in my reply to 0/6 for what I think would make a more powerful API. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 9 +++++++++ 3 files changed, 21 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7580477..74225af 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4739,6 +4739,7 @@ static virDriver remote_driver = { .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ + .domainNormalizeDeviceXML = remoteDomainNormalizeDeviceXML, /* 0.9.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index ca739ff..e6a51fb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2348,6 +2348,15 @@ struct remote_node_suspend_for_duration_args { unsigned int flags; }; +struct remote_domain_normalize_device_xml_args { + remote_nonnull_domain dom; + remote_nonnull_string device_xml; + unsigned int flags; +}; + +struct remote_domain_normalize_device_xml_ret { + remote_nonnull_string xml; +}; /*----- Protocol. -----*/ @@ -2653,7 +2662,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_NORMALIZE_DEVICE_XML = 258 /* autogen autogen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 2758315..1bc5a59 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1832,6 +1832,14 @@ struct remote_node_suspend_for_duration_args { uint64_t duration; u_int flags; }; +struct remote_domain_normalize_device_xml_args { + remote_nonnull_domain dom; + remote_nonnull_string xml; + u_int flags; +} +struct remote_domain_normalize_device_xml_ret { + remote_nonnull_string xml; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2090,4 +2098,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, + REMOTE_PROC_DOMAIN_NORMALIZE_DEVICE_XML = 258, }; -- 1.7.7.3
On 01/09/2012 07:29 AM, Osier Yang wrote:
--- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 9 +++++++++ 3 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7580477..74225af 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4739,6 +4739,7 @@ static virDriver remote_driver = { .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ + .domainNormalizeDeviceXML = remoteDomainNormalizeDeviceXML, /* 0.9.10 */ };
static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index ca739ff..e6a51fb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2348,6 +2348,15 @@ struct remote_node_suspend_for_duration_args { unsigned int flags; };
+struct remote_domain_normalize_device_xml_args { + remote_nonnull_domain dom;
Obvious changes here if we go with a different API.
+ remote_nonnull_string device_xml; + unsigned int flags; +}; + +struct remote_domain_normalize_device_xml_ret { + remote_nonnull_string xml; +};
/*----- Protocol. -----*/
@@ -2653,7 +2662,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_NORMALIZE_DEVICE_XML = 258 /* autogen autogen priority:high */
This part looks fine - it shouldn't be making any monitor calls, so using the high-priority queue won't starve anything. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/conf/domain_conf.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + 3 files changed, 73 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0190a81..f0ed479 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11832,6 +11832,76 @@ virDomainDefFormat(virDomainDefPtr def, unsigned int flags) return virBufferContentAndReset(&buf); } +char * +virDomainDeviceDefFormat(virDomainDeviceDefPtr dev, unsigned int flags) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virCheckFlags(DUMPXML_FLAGS, NULL); + + switch(dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + if (virDomainDiskDefFormat(&buf, dev->data.disk, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_LEASE: + if (virDomainLeaseDefFormat(&buf, dev->data.lease) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_FS: + if (virDomainFSDefFormat(&buf, dev->data.fs, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_NET: + if (virDomainNetDefFormat(&buf, dev->data.net, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_INPUT: + if (virDomainInputDefFormat(&buf, dev->data.input, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_SOUND: + if (virDomainSoundDefFormat(&buf, dev->data.sound, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_VIDEO: + if (virDomainVideoDefFormat(&buf, dev->data.video, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + if (virDomainHostdevDefFormat(&buf, dev->data.hostdev, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_WATCHDOG: + if (virDomainWatchdogDefFormat(&buf, dev->data.watchdog, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + if (virDomainControllerDefFormat(&buf, dev->data.controller, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_GRAPHICS: + if (virDomainGraphicsDefFormat(&buf, dev->data.graphics, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_HUB: + if (virDomainHubDefFormat(&buf, dev->data.hub, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_REDIRDEV: + if (virDomainRedirdevDefFormat(&buf, dev->data.redirdev, flags) < 0) + goto cleanup; + break; + default: + /* Shouldn't hit here. */ + break; + } + return virBufferContentAndReset(&buf); + +cleanup: + virBufferFreeAndReset(&buf); + return NULL; +} static char *virDomainObjFormat(virCapsPtr caps, virDomainObjPtr obj, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 03aa5b6..9d726b2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1780,6 +1780,8 @@ int virDomainDefFormatInternal(virDomainDefPtr def, unsigned int flags, virBufferPtr buf); +char *virDomainDeviceDefFormat(virDomainDeviceDefPtr def, + unsigned int flags); int virDomainCpuSetParse(const char *str, char sep, char *cpuset, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ac2c52e..5dd4006 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -282,6 +282,7 @@ virDomainDeviceAddressIsValid; virDomainDeviceAddressPciMultiTypeFromString; virDomainDeviceAddressPciMultiTypeToString; virDomainDeviceAddressTypeToString; +virDomainDeviceDefFormat; virDomainDeviceDefFree; virDomainDeviceDefParse; virDomainDeviceInfoIterate; -- 1.7.7.3
On 01/09/2012 07:29 AM, Osier Yang wrote:
---
More details in the commit message might be nice.
src/conf/domain_conf.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + 3 files changed, 73 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0190a81..f0ed479 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11832,6 +11832,76 @@ virDomainDefFormat(virDomainDefPtr def, unsigned int flags) return virBufferContentAndReset(&buf); }
+char * +virDomainDeviceDefFormat(virDomainDeviceDefPtr dev, unsigned int flags) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virCheckFlags(DUMPXML_FLAGS, NULL); + + switch(dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + if (virDomainDiskDefFormat(&buf, dev->data.disk, flags) < 0) + goto cleanup; + break; + case VIR_DOMAIN_DEVICE_LEASE: + if (virDomainLeaseDefFormat(&buf, dev->data.lease) < 0) + goto cleanup; + break;
ACK - this looks like a useful helper method for implementing _part_ of the new functionality. But if you make things more generic, you probably also want a helper function at a higher level that delegates between domain_conf.c, interface_conf.c, and so forth, all according to the type parameter tied to the XML being normalized. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
If the domain is running, the live def is used to parse the device XML, otherwise persistent def is used. --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2c4d071..f350deb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11824,6 +11824,52 @@ cleanup: return ret; } +static char * +qemuDomainNormalizeDeviceXML(virDomainPtr dom, + const char *device_xml, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDefPtr persistentDef = NULL; + virDomainDefPtr def = NULL; + virDomainDeviceDefPtr dev = NULL; + char *ret = NULL; + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + def = vm->def; + } else { + def = persistentDef; + } + + dev = virDomainDeviceDefParse(driver->caps, def, device_xml, 0); + if (!dev) + goto cleanup; + + ret = virDomainDeviceDefFormat(dev, 0); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = "QEMU", @@ -11976,6 +12022,7 @@ static virDriver qemuDriver = { .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */ .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */ .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */ + .domainNormalizeDeviceXML = qemuDomainNormalizeDeviceXML, /* 0.9.10 */ }; -- 1.7.7.3
On 01/09/2012 07:29 AM, Osier Yang wrote:
If the domain is running, the live def is used to parse the device XML, otherwise persistent def is used. --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-)
Either the XML is valid on its own (it can be parsed into a domain_conf structure and output again) or it is not; it should not depend on whether it is also a subset of a specific domain, so I don't see how deciding between live or persistent domain XML should come into play.
+static char * +qemuDomainNormalizeDeviceXML(virDomainPtr dom, + const char *device_xml, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDefPtr persistentDef = NULL; + virDomainDefPtr def = NULL; + virDomainDeviceDefPtr dev = NULL; + char *ret = NULL;
Missing a virCheckFlags().
+ + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver);
Again, I don't see the point of a vm lookup; this should operate on just the connection level.
+ + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + def = vm->def; + } else { + def = persistentDef; + }
and that means that we should _not_ be checking for VIR_DOMAIN_AFFECT_*. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The command here is just to demo the effect of the API and for one wants to play with it, it's not much useful as a virsh command IMO, so it's not for final commit. --- tools/virsh.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 50 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e4b812e..0b15c46 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6050,6 +6050,54 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd) } /* + * "normalize-device-xml" command + */ +static const vshCmdInfo info_normalize_device_xml[] = { + {"help", N_("normalize the incoming device XML")}, + {"desc", N_("Output the normalized device XML.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_normalize_device_xml[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("Device XML file")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdNormalizeDeviceXML(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *xmlfile = NULL; + bool ret = false; + char *buffer = NULL; + char *output = NULL; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "file", &xmlfile) <= 0) + return false; + + if (virFileReadAll(xmlfile, VIRSH_MAX_XML_FILE, &buffer) < 0) + return false; + + output = virDomainNormalizeDeviceXML(dom, buffer, 0); + if (output) { + vshPrint(ctl, "%s", output); + VIR_FREE(output); + ret = true; + } + + VIR_FREE(buffer); + virDomainFree(dom); + return ret; +} + +/* * "domxml-from-native" command */ static const vshCmdInfo info_domxmlfromnative[] = { @@ -15812,6 +15860,8 @@ static const vshCmdDef domManagementCmds[] = { opts_migrate_setspeed, info_migrate_setspeed, 0}, {"migrate-getspeed", cmdMigrateGetMaxSpeed, opts_migrate_getspeed, info_migrate_getspeed, 0}, + {"normalize-device-xml", cmdNormalizeDeviceXML, + opts_normalize_device_xml, info_normalize_device_xml, 0}, {"numatune", cmdNumatune, opts_numatune, info_numatune, 0}, {"reboot", cmdReboot, opts_reboot, info_reboot, 0}, {"reset", cmdReset, opts_reset, info_reset, 0}, -- 1.7.7.3
On 01/09/2012 07:29 AM, Osier Yang wrote:
The command here is just to demo the effect of the API and for one wants to play with it, it's not much useful as a virsh command IMO, so it's not for final commit.
On the contrary, I think that every libvirt API should be exposed via virsh commands, so this patch should be polished and included in the final series (that is, add virsh.pod documentation). As is, providing this command allows for unit tests that our API works, whether or not anyone else finds another use for the virsh API.
--- tools/virsh.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index e4b812e..0b15c46 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6050,6 +6050,54 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd) }
/* + * "normalize-device-xml" command + */ +static const vshCmdInfo info_normalize_device_xml[] = { + {"help", N_("normalize the incoming device XML")}, + {"desc", N_("Output the normalized device XML.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_normalize_device_xml[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("Device XML file")},
Given my thoughts in 0/6, I think we also need an optional --type=... parameter. By default, if --type is not specified, virsh should read the given XML and guess a type itself (that is, --type=domain is redundant if xmlfile started with <domain>); but there are cases where it is a required argument (that is, a top-level <interface> in the user's file could either match --type=interface for use in commands like iface-define [using conf/interface_conf.c], or --type=domain-device for use in commands like attach-device [using conf/domain_conf.c], with the two uses having quite a different effect). And definitely support whatever flags we come up with, such as --validate, as well as an intelligent --inactive (which maps to VIR_DOMAIN_XML_INACTIVE if type is domain or domain-device, and to VIR_INTERFACE_XML_INACTIVE if type is interface).
+static bool +cmdNormalizeDeviceXML(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *xmlfile = NULL; + bool ret = false; + char *buffer = NULL; + char *output = NULL; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "file", &xmlfile) <= 0) + return false; + + if (virFileReadAll(xmlfile, VIRSH_MAX_XML_FILE, &buffer) < 0) + return false; + + output = virDomainNormalizeDeviceXML(dom, buffer, 0); + if (output) { + vshPrint(ctl, "%s", output); + VIR_FREE(output); + ret = true; + } + + VIR_FREE(buffer); + virDomainFree(dom);
'dom' shouldn't be needed if you make the API more generic.
@@ -15812,6 +15860,8 @@ static const vshCmdDef domManagementCmds[] = { opts_migrate_setspeed, info_migrate_setspeed, 0}, {"migrate-getspeed", cmdMigrateGetMaxSpeed, opts_migrate_getspeed, info_migrate_getspeed, 0}, + {"normalize-device-xml", cmdNormalizeDeviceXML, + opts_normalize_device_xml, info_normalize_device_xml, 0},
I'd name it 'normalize-xml', and stick it under the "Host and Hypervisor" section alongside 'capabilities' and such. I'd also look at adding a 7/6, to make 'virsh edit' and friends learn how to re-open the editor if normalization fails validation, rather than completely discarding all the user's edits if the actual define API fails. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Jan 09, 2012 at 10:29:08PM +0800, Osier Yang wrote:
The initial purpose was to fix a regression for detaching device, (introduced by commit ea7182c29). There was a patch posted to resolve the problem:
https://www.redhat.com/archives/libvir-list/2011-December/msg00818.html
But as Eric suggested, it's not the ideal way to go, we never known how many stuffs like <address> will be involved in future. So new API to invoke the internal parsing functions might be a right way then.
However, things are not that simple (an API without internal driver support, invoking the parsing functions directly). As the domain conf is a neccessary argument to parse the device XML. (e.g. for an Input device). Although we can bypass that by modification on virDomainDeviceDefParse, it could be trap as we will parse the device XML in another way which is different with the real parsing. So finally I choosed to implement the driver support for the new API. There might be something I didn't take into consideration, (e.g. Do we need some flags for the XML parsing and formating?). But it can demo the thought good enough.
On the other hand, I'm wondering if it's deserved to introduce such an API, comparing it's usage, is it two heavy if we need the internal drivers support for such an API?
Any thoughts and feedback is welcomed.
I don't really understand what the use case is for this API, from the description above. Can you give an example of how, for example, virt-manager would use this API todo something ? Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/09/2012 07:37 AM, Daniel P. Berrange wrote:
On Mon, Jan 09, 2012 at 10:29:08PM +0800, Osier Yang wrote:
The initial purpose was to fix a regression for detaching device, (introduced by commit ea7182c29). There was a patch posted to resolve the problem:
https://www.redhat.com/archives/libvir-list/2011-December/msg00818.html
But as Eric suggested, it's not the ideal way to go, we never known how many stuffs like <address> will be involved in future. So new API to invoke the internal parsing functions might be a right way then.
However, things are not that simple (an API without internal driver support, invoking the parsing functions directly). As the domain conf is a neccessary argument to parse the device XML. (e.g. for an Input
s/neccessary/necessary/
device). Although we can bypass that by modification on virDomainDeviceDefParse, it could be trap as we will parse the device XML in another way which is different with the real parsing. So finally I choosed to implement the driver support for the new API. There might
s/choosed/chose/
be something I didn't take into consideration, (e.g. Do we need some flags for the XML parsing and formating?). But it can demo the thought
s/formating/formatting/
good enough.
On the other hand, I'm wondering if it's deserved to introduce such an API, comparing it's usage, is it two heavy if we need the internal drivers support for such an API?
Any thoughts and feedback is welcomed.
I don't really understand what the use case is for this API, from the description above. Can you give an example of how, for example, virt-manager would use this API todo something ?
I see several potential uses: 1. XML feature probe. For example, libvirt 0.9.9 just added the XML feature of having a <seclabel> attached to a specific <disk>; older libvirt silently ignores that label. Right now, I have no way to know if I use <seclabel> if I will get a per-disk override; running virt-xml-validate on my machine doesn't help if the libvirt on the remote server is at a different libvirt version, and if my only connection to the remote machine is via libvirt connections rather than a direct ssh, then I can't run virt-xml-validate on the destination machine. But with this new API, I could: construct a candidate XML run it through the API compare the resulting XML with my candidate if <seclabel> is still intact, then the feature is supported; if it was stripped, then I am talking to an older libvirt that didn't know the feature 2. XML subset matching. Things like 'virsh detach-device' want to be permissive in what the user passes in, by taking minimal XML from the user and expanding it to the exact XML present in the domain. But since we allow interleaves, it's not easy to do the comparison from user XML to the order that the XML is generated by 'virsh dumpxml'. With the new API, virsh could be coded to run any candidate XML through the normalization, prior to doing subset comparisons against the real domain XML, and have the assurance that all known elements and attributes appear in the same order. 3. XML validation. It seems pretty easy to expand this API into taking a flag that says to compare the incoming XML against the RNG, and return NULL on failure to validate. This would make implementation of 'virsh edit' smarter, by being able to validate the user's edits prior to trying to apply them. 4. libvirt-gconfig seems like an obvious client for taking user input and normalizing it into the same form that libvirt would output. However, I think the API needs to be a bit more flexible than what Osier proposed. It needs to be an API on the virConnectPtr, not a virDomainPtr, since it is NOT tied to a specific domain, but to the parsing abilities of the connection in general. I also think that the API can handle multiple XML types in a single API. (Oh yuck - I just realized that VIR_DOMAIN_XML_INACTIVE==2, but VIR_INTERFACE_XML_INACTIVE==1, so we _can't_ have a common VIR_NORMALIZE_XML_INACTIVE). I envision something more like: typedef enum { /* All top-level elements where we have a getXMLDesc method: */ VIR_NORMALIZE_XML_DOMAIN = 100, /* top-level is <domain> */ VIR_NORMALIZE_XML_SNAPSHOT = 200, /* top-level is <domainsnapshot> */ VIR_NORMALIZE_XML_NETWORK = 300, /* top-level is <network> */ ... /* Additionally, sub-level elements that are used in various APIs: */ VIR_NORMALIZE_XML_DOMAIN_DEVICE = 101, /* any element that fits under /domain/devices, such as <disk> or <interface> */ ... } virConnectNormalizeXMLTypes; typedef enum { /* Reserve bits 0, 1, 2 for virDomainXMLFlags */ /* Reserve bits 0 for virInterfaceXMLFlags */ ... VIR_NORMALIZE_XML_VALIDATE = (1 << 30), /* perform RNG validation */ } virConnectNormalizeXMLFlags; /** * virConnectNormalizeXML: * @conn: Pointer to connection * @xml: XML to be normalized * @type: one value from virConnectNormalizeXMLTypes, documenting * expected top-level element of @xml * @flags: bitwise-OR of virConnectNormalizeXMLFlags * * This function parses the incoming @xml, with root element * according to @type, and returns the same XML normalized to the * same form it would appear via the corresponding vir*GetXMLDesc * function if it had described a real libvirt object. No * machine state is checked or changed, so while the XML may be * valid, subsequent use of the XML might still fail for other * reasons such as an attempt to reuse a UUID. * * The normalization process may reorder attributes and elements, * and may add new elements describing default states. By default, * unrecognized elements and attributes are silently removed, but * if @flags includes VIR_NORMALIZE_XML_VALIDATE, an error is * raised if any unknown input is encountered. * * Depending on @type, additional bits in @flags may be understood. * For example, if @type specifies a domain or sub-element of a * domain, the flags can include virDomainXMLFlags such as * VIR_DOMAIN_XML_INACTIVE; whereas if @type specifies a host * interface, the flags can include virInterfaceXMLFlags. * * Returns the normalized XML (caller must free), or NULL if the * XML could not be normalized. */ char * virConnectNormalizeXML(virConnectPtr conn, const char *xml, unsigned int type, unsigned int flags); -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Jan 09, 2012 at 03:53:25PM -0700, Eric Blake wrote:
I don't really understand what the use case is for this API, from the description above. Can you give an example of how, for example, virt-manager would use this API todo something ?
I see several potential uses:
1. XML feature probe. For example, libvirt 0.9.9 just added the XML feature of having a <seclabel> attached to a specific <disk>; older libvirt silently ignores that label. Right now, I have no way to know if I use <seclabel> if I will get a per-disk override; running virt-xml-validate on my machine doesn't help if the libvirt on the remote server is at a different libvirt version, and if my only connection to the remote machine is via libvirt connections rather than a direct ssh, then I can't run virt-xml-validate on the destination machine. But with this new API, I could: construct a candidate XML run it through the API compare the resulting XML with my candidate if <seclabel> is still intact, then the feature is supported; if it was stripped, then I am talking to an older libvirt that didn't know the feature
2. XML subset matching. Things like 'virsh detach-device' want to be permissive in what the user passes in, by taking minimal XML from the user and expanding it to the exact XML present in the domain. But since we allow interleaves, it's not easy to do the comparison from user XML to the order that the XML is generated by 'virsh dumpxml'. With the new API, virsh could be coded to run any candidate XML through the normalization, prior to doing subset comparisons against the real domain XML, and have the assurance that all known elements and attributes appear in the same order.
These 2 things feel like overloading the API with two different semantics. On the one hand I see a simple XML -> struct -> XML transformation, which is driver indepedant and does not need to get any HV state. I call this canonicalization The other case I see XML -> struct -> munge struct info -> XML which makes changes based on the HV state. I call this XML enhancement. I'm not even convinced this is desirable. IMHO the way virsh detach device is going about this task is just plain wrong. It should be just requesting the full XML document, and then doing an XPath query on it, to pull out the <interface> element it needs. This removes the need to create any XML from scratch.
3. XML validation. It seems pretty easy to expand this API into taking a flag that says to compare the incoming XML against the RNG, and return NULL on failure to validate. This would make implementation of 'virsh edit' smarter, by being able to validate the user's edits prior to trying to apply them.
4. libvirt-gconfig seems like an obvious client for taking user input and normalizing it into the same form that libvirt would output.
libvirt-gconfig actually attempts to never re-write the XML it receives either from libvirt or the user. The code is designed so it just parses the XML doc it is given, and keeps the xmlDocPtr DOM in memory, and just reads/writes the DOM directly for all APIs. This way we preserve all data that the user has in the XML, including stuff we don't understand ourselves.
However, I think the API needs to be a bit more flexible than what Osier proposed. It needs to be an API on the virConnectPtr, not a virDomainPtr, since it is NOT tied to a specific domain, but to the parsing abilities of the connection in general. I also think that the API can handle multiple XML types in a single API. (Oh yuck - I just realized that VIR_DOMAIN_XML_INACTIVE==2, but VIR_INTERFACE_XML_INACTIVE==1, so we _can't_ have a common VIR_NORMALIZE_XML_INACTIVE). I envision something more like:
VIR_NORMALIZE_XML_VALIDATE = (1 << 30), /* perform RNG validation */ } virConnectNormalizeXMLFlags;
/** * virConnectNormalizeXML: * @conn: Pointer to connection * @xml: XML to be normalized * @type: one value from virConnectNormalizeXMLTypes, documenting * expected top-level element of @xml * @flags: bitwise-OR of virConnectNormalizeXMLFlags * * This function parses the incoming @xml, with root element * according to @type, and returns the same XML normalized to the * same form it would appear via the corresponding vir*GetXMLDesc * function if it had described a real libvirt object. No * machine state is checked or changed, so while the XML may be * valid, subsequent use of the XML might still fail for other * reasons such as an attempt to reuse a UUID. * * The normalization process may reorder attributes and elements, * and may add new elements describing default states. By default, * unrecognized elements and attributes are silently removed, but * if @flags includes VIR_NORMALIZE_XML_VALIDATE, an error is * raised if any unknown input is encountered. * * Depending on @type, additional bits in @flags may be understood. * For example, if @type specifies a domain or sub-element of a * domain, the flags can include virDomainXMLFlags such as * VIR_DOMAIN_XML_INACTIVE; whereas if @type specifies a host * interface, the flags can include virInterfaceXMLFlags. * * Returns the normalized XML (caller must free), or NULL if the * XML could not be normalized. */ char * virConnectNormalizeXML(virConnectPtr conn, const char *xml, unsigned int type, unsigned int flags);
This does look more useful, but as mentioned above, IMHO the normalization should be purely syntactic normalization, and not rely on any semantic state from the hypervisor drivers. ie, basically round trip virDomainParseDef+virDomainFormatDef + optional RNG validation. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/10/2012 09:55 AM, Daniel P. Berrange wrote:
These 2 things feel like overloading the API with two different semantics. On the one hand I see a simple XML -> struct -> XML transformation, which is driver indepedant and does not need to get any HV state. I call this canonicalization
We still want it to be connection-dependent, though, so that the canonicalization is tied to the version of the driver the user is connected to, and not to the version of the libvirt.so client, in the case where the driver and client are at different versions.
The other case I see XML -> struct -> munge struct info -> XML which makes changes based on the HV state. I call this XML enhancement. I'm not even convinced this is desirable. IMHO the way virsh detach device is going about this task is just plain wrong. It should be just requesting the full XML document, and then doing an XPath query on it, to pull out the <interface> element it needs. This removes the need to create any XML from scratch.
Agreed - the new API should _not_ do any munging of user info; any munging to solve the virsh situation should be done _after_ canonicalization, in virsh, and not by a hypervisor driver API. The XML that gets output by the new API should be semantically equivalent to the XML that was input. And the idea of converting a user's subset of XML into XPath queries against the domain's XML in order to extract the full device from the domain sounds like it might work.
* This function parses the incoming @xml, with root element * according to @type, and returns the same XML normalized to the * same form it would appear via the corresponding vir*GetXMLDesc * function if it had described a real libvirt object. No * machine state is checked or changed, so while the XML may be * valid, subsequent use of the XML might still fail for other * reasons such as an attempt to reuse a UUID. *
This does look more useful, but as mentioned above, IMHO the normalization should be purely syntactic normalization, and not rely on any semantic state from the hypervisor drivers.
I think we're in violent agreement - since "no machine state is checked or changed", the API is round-trip only, with no modification of the XML other than what happens as a result of virDomainParseDef+virDomainFormatDef (that is, canonicalizing the order of <interleave> elements, flattening integers to their canonical form, and adding in any default attributes/elements). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Jan 10, 2012 at 10:13:59AM -0700, Eric Blake wrote:
On 01/10/2012 09:55 AM, Daniel P. Berrange wrote:
These 2 things feel like overloading the API with two different semantics. On the one hand I see a simple XML -> struct -> XML transformation, which is driver indepedant and does not need to get any HV state. I call this canonicalization
We still want it to be connection-dependent, though, so that the canonicalization is tied to the version of the driver the user is connected to, and not to the version of the libvirt.so client, in the case where the driver and client are at different versions.
Oh sure, still needs to run server side - we can have 1 shared impl that can be pulled in by all drivers - as we do for nodeinfo stuff that's shared. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 2012年01月10日 06:53, Eric Blake wrote:
On 01/09/2012 07:37 AM, Daniel P. Berrange wrote:
On Mon, Jan 09, 2012 at 10:29:08PM +0800, Osier Yang wrote:
The initial purpose was to fix a regression for detaching device, (introduced by commit ea7182c29). There was a patch posted to resolve the problem:
https://www.redhat.com/archives/libvir-list/2011-December/msg00818.html
But as Eric suggested, it's not the ideal way to go, we never known how many stuffs like<address> will be involved in future. So new API to invoke the internal parsing functions might be a right way then.
However, things are not that simple (an API without internal driver support, invoking the parsing functions directly). As the domain conf is a neccessary argument to parse the device XML. (e.g. for an Input
s/neccessary/necessary/
device). Although we can bypass that by modification on virDomainDeviceDefParse, it could be trap as we will parse the device XML in another way which is different with the real parsing. So finally I choosed to implement the driver support for the new API. There might
s/choosed/chose/
be something I didn't take into consideration, (e.g. Do we need some flags for the XML parsing and formating?). But it can demo the thought
s/formating/formatting/
good enough.
On the other hand, I'm wondering if it's deserved to introduce such an API, comparing it's usage, is it two heavy if we need the internal drivers support for such an API?
Any thoughts and feedback is welcomed.
I don't really understand what the use case is for this API, from the description above. Can you give an example of how, for example, virt-manager would use this API todo something ?
I see several potential uses:
1. XML feature probe. For example, libvirt 0.9.9 just added the XML feature of having a<seclabel> attached to a specific<disk>; older libvirt silently ignores that label. Right now, I have no way to know if I use<seclabel> if I will get a per-disk override; running virt-xml-validate on my machine doesn't help if the libvirt on the remote server is at a different libvirt version, and if my only connection to the remote machine is via libvirt connections rather than a direct ssh, then I can't run virt-xml-validate on the destination machine. But with this new API, I could: construct a candidate XML run it through the API compare the resulting XML with my candidate if<seclabel> is still intact, then the feature is supported; if it was stripped, then I am talking to an older libvirt that didn't know the feature
2. XML subset matching. Things like 'virsh detach-device' want to be permissive in what the user passes in, by taking minimal XML from the user and expanding it to the exact XML present in the domain. But since we allow interleaves, it's not easy to do the comparison from user XML to the order that the XML is generated by 'virsh dumpxml'. With the new API, virsh could be coded to run any candidate XML through the normalization, prior to doing subset comparisons against the real domain XML, and have the assurance that all known elements and attributes appear in the same order.
3. XML validation. It seems pretty easy to expand this API into taking a flag that says to compare the incoming XML against the RNG, and return NULL on failure to validate. This would make implementation of 'virsh edit' smarter, by being able to validate the user's edits prior to trying to apply them.
4. libvirt-gconfig seems like an obvious client for taking user input and normalizing it into the same form that libvirt would output.
However, I think the API needs to be a bit more flexible than what Osier proposed. It needs to be an API on the virConnectPtr, not a virDomainPtr, since it is NOT tied to a specific domain, but to the parsing abilities of the connection in general. I also think that the API can handle multiple XML types in a single API. (Oh yuck - I just realized that VIR_DOMAIN_XML_INACTIVE==2, but VIR_INTERFACE_XML_INACTIVE==1, so we _can't_ have a common VIR_NORMALIZE_XML_INACTIVE). I envision something more like:
typedef enum { /* All top-level elements where we have a getXMLDesc method: */ VIR_NORMALIZE_XML_DOMAIN = 100, /* top-level is<domain> */ VIR_NORMALIZE_XML_SNAPSHOT = 200, /* top-level is<domainsnapshot> */ VIR_NORMALIZE_XML_NETWORK = 300, /* top-level is<network> */ ... /* Additionally, sub-level elements that are used in various APIs: */ VIR_NORMALIZE_XML_DOMAIN_DEVICE = 101, /* any element that fits under /domain/devices, such as<disk> or<interface> */ ... } virConnectNormalizeXMLTypes;
typedef enum { /* Reserve bits 0, 1, 2 for virDomainXMLFlags */ /* Reserve bits 0 for virInterfaceXMLFlags */ ... VIR_NORMALIZE_XML_VALIDATE = (1<< 30), /* perform RNG validation */ } virConnectNormalizeXMLFlags;
/** * virConnectNormalizeXML: * @conn: Pointer to connection * @xml: XML to be normalized * @type: one value from virConnectNormalizeXMLTypes, documenting * expected top-level element of @xml * @flags: bitwise-OR of virConnectNormalizeXMLFlags * * This function parses the incoming @xml, with root element * according to @type, and returns the same XML normalized to the * same form it would appear via the corresponding vir*GetXMLDesc * function if it had described a real libvirt object. No * machine state is checked or changed, so while the XML may be * valid, subsequent use of the XML might still fail for other * reasons such as an attempt to reuse a UUID. * * The normalization process may reorder attributes and elements, * and may add new elements describing default states. By default, * unrecognized elements and attributes are silently removed, but * if @flags includes VIR_NORMALIZE_XML_VALIDATE, an error is * raised if any unknown input is encountered. * * Depending on @type, additional bits in @flags may be understood. * For example, if @type specifies a domain or sub-element of a * domain, the flags can include virDomainXMLFlags such as * VIR_DOMAIN_XML_INACTIVE; whereas if @type specifies a host * interface, the flags can include virInterfaceXMLFlags. * * Returns the normalized XML (caller must free), or NULL if the * XML could not be normalized. */ char * virConnectNormalizeXML(virConnectPtr conn, const char *xml, unsigned int type, unsigned int flags);
The API should be defined as following if we tend to support XML validation: int virConnectNormalizeXML(virConnectPtr conn, const char *xmlin, char *xmlout, unsigned int type, unsigned int flags); Or we need a standalone API for XML validation? not mixing the normalization with validation together instead? Personally I want one could get what an API does easily from the its name. i.e. I prefer two APIs. (virConnectValidateXML). Osier
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Osier Yang