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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org