On 01/27/2012 10:22 AM, Peter Krempa wrote:
This patch adds support for the new api into the qemu driver to
support
modification and retireval of domain description and title. This patch
does not add support for modifying the <metadata> element.
---
+
+ /* validate title */
+ if (type == VIR_DOMAIN_METADATA_TITLE && metadata) {
+ if (strchr(metadata, '\n')) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Domain title can't contain newlines"));
+ goto cleanup;
+ }
+ if (strlen(metadata) > VIR_DOMAIN_TITLE_LENGTH) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Domain title too long"));
+ goto cleanup;
+ }
+ }
Since title is relatively new, we may want to do this validation in both
domain_conf (when parsing XML) and in libvirt.c (when calling
virDomainSetMetadata), so that hypervisors don't have to repeat this code.
+ case VIR_DOMAIN_METADATA_ELEMENT:
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("QEmu driver does not support modifying"
+ "<metadata> element"));
I'd use VIR_ERR_ARGUMENT_UNSUPPORTED here. All of the arguments were
valid, we just don't know how to act on them.
+static char *
+qemuDomainGetMetadata(virDomainPtr dom,
+ int type,
+ const char *uri ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ struct qemud_driver *driver = dom->conn->privateData;
+
+ if (flags &
+ (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) {
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("Can't specify both LIVE and CONFIG flags"));
+ goto cleanup;
+ }
Drop this check. It should live in libvirt.c. And as written, you
didn't do the check you wanted - you are rejecting any use of LIVE or
CONFIG, rather than the intended simultaneous use of both flags.
Looks like a good implementation for 2 of the 3 fields, and again, I'm
okay if we don't get <metadata> implemented until a later patch, as long
as we're happy that the API will let us do it right.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org