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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list