[libvirt] [PATCH 2/3] conf: Rework virDomainDeviceDefParse

Peter Krempa pkrempa at redhat.com
Thu Jul 11 12:08:37 UTC 2013


On 07/11/13 13:29, Michal Privoznik wrote:
> When adding a new domain device, it is fairly easy to forget to add
> corresponding piece into virDomainDeviceDefParse. However, if the
> internal structure is changed to one bit switch() the compiler will warn
> about not handled enum item.
> ---
>   src/conf/domain_conf.c | 96 ++++++++++++++++++++++++++++----------------------
>   1 file changed, 54 insertions(+), 42 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d0c87b2..764ca6e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9336,94 +9336,106 @@ virDomainDeviceDefParse(const char *xmlStr,
>       if (VIR_ALLOC(dev) < 0)
>           goto error;
>
> -    if (xmlStrEqual(node->name, BAD_CAST "disk")) {
> -        dev->type = VIR_DOMAIN_DEVICE_DISK;
> +    if ((dev->type = virDomainDeviceTypeFromString((const char *) node->name)) < 0) {
> +        /* Some crazy mapping of serial, parallel, console and channel to
> +         * VIR_DOMAIN_DEVICE_CHR. */

hmmm, this isn't very nice, but ...

> +        if (xmlStrEqual(node->name, BAD_CAST "channel") ||
> +            xmlStrEqual(node->name, BAD_CAST "console") ||
> +            xmlStrEqual(node->name, BAD_CAST "parallel") ||
> +            xmlStrEqual(node->name, BAD_CAST "serial")) {
> +            dev->type = VIR_DOMAIN_DEVICE_CHR;
> +        } else {
> +            virReportError(VIR_ERR_XML_ERROR, _("unknown device type '%s'"), node->name);

This line is probably too long. Please break it.

> +            goto error;
> +        }
> +    }
> +
> +    switch ((virDomainDeviceType) dev->type) {
> +    case VIR_DOMAIN_DEVICE_DISK:
>           if (!(dev->data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
>                                                           NULL, def->seclabels,
>                                                           def->nseclabels,
>                                                           flags)))
>               goto error;
> -    } else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
> -        dev->type = VIR_DOMAIN_DEVICE_LEASE;
> +        break;
> +    case VIR_DOMAIN_DEVICE_LEASE:
>           if (!(dev->data.lease = virDomainLeaseDefParseXML(node)))
>               goto error;
> -    } else if (xmlStrEqual(node->name, BAD_CAST "filesystem")) {
> -        dev->type = VIR_DOMAIN_DEVICE_FS;
> +        break;

... this pattern looks better and as the commit message states, it lets 
the compiler warn us.

ACK, with long line broken.

Peter




More information about the libvir-list mailing list