[libvirt] [RFC PATCH 03/12] domain_conf: introduce cpu def helpers

[Peter Krempa]

On Wed, Jan 21, 2015 at 16:00:55 +0800, Zhu Guihua wrote:
> virDomainCPUDefFree - free memory allocated
> virDomainCPUDefParseXML - parse job type
> virDomainCPUDefFormat - output job type

This patch lacks addition to the RNG schemas that would describe the
elements that are added and the correct values.

Also lacks change to the docs/formatdomain.html.in


> 
> Signed-off-by: Zhu Guihua <zhugh.fnst at cn.fujitsu.com>
> ---
>  src/conf/domain_conf.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 100 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e036d75..1f05056 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

...

> +
> +static virDomainCPUDefPtr
> +virDomainCPUDefParseXML(xmlNodePtr node, virDomainDefPtr def)
> +{
> +    char *driver = NULL;
> +    char *apic_id = NULL;
> +    virDomainCPUDefPtr dev;
> +
> +    if (!(dev = virDomainCPUDefNew()))
> +        return NULL;
> +
> +    driver = virXMLPropString(node, "driver");

The driver should rather be an enum value that is parsed from the string
rather than stroing an inline string. This limits the namespace of
devices libvirt supports but simplifies all matching of the driver later
on.


> +    if (driver == NULL) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("missing cpu device driver"));
> +        goto error;
> +    }
> +    dev->driver = driver;
> +
> +    apic_id = virXMLPropString(node, "apic_id");
> +
> +    if (!apic_id)
> +        dev->apic_id = virDomainCPUGetFreeApicID(def);
> +    else
> +        dev->apic_id = atoi (apic_id);

Atoi is not allowed, use virStrToLong* instead. Also spaces after
function name is forbidden.

> +
> +    driver = NULL;
> +    apic_id = NULL;
> +
> + cleanup:
> +    VIR_FREE(driver);
> +    VIR_FREE(apic_id);
> +
> +    return dev;
> +
> + error:
> +    virDomainCPUDefFree(dev);
> +    dev = NULL;
> +    goto cleanup;
> +}
> +
>  virDomainDeviceDefPtr
>  virDomainDeviceDefParse(const char *xmlStr,
>                          const virDomainDef *def,
> @@ -11187,6 +11253,9 @@ virDomainDeviceDefParse(const char *xmlStr,
>              goto error;
>          break;
>      case VIR_DOMAIN_DEVICE_CPU:
> +        if (!(dev->data.cpu = virDomainCPUDefParseXML(node, (virDomainDefPtr)def)))
> +            goto error;
> +        break;
>      case VIR_DOMAIN_DEVICE_NONE:
>      case VIR_DOMAIN_DEVICE_LAST:
>          break;
> @@ -18142,6 +18211,31 @@ virDomainChrDefFormat(virBufferPtr buf,
>  }
>  
>  static int
> +virDomainCPUDefFormat(virBufferPtr buf,
> +                      virDomainCPUDefPtr def,
> +                      unsigned int flags)
> +{
> +    char *apic_id = NULL;
> +
> +    ignore_value(virAsprintf(&apic_id, "%d", def->apic_id));
> +
> +    virBufferAsprintf(buf, "<cpu driver='%s'", def->driver);
> +
> +    virBufferEscapeString(buf, " apic_id='%s'", apic_id);

Um? Why not virBufferAsprintf(buf, " apic_id='%d', apic_id)?

You won't need the intermediate string. Also it leaks the apic_id
string.


> +
> +    virBufferAddLit(buf, ">\n");
> +    virBufferAdjustIndent(buf, 2);
> +
> +    if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
> +        return -1;
> +
> +    virBufferAdjustIndent(buf, -2);
> +    virBufferAddLit(buf, "</cpu>\n");
> +
> +    return 0;
> +}
> +
> +static int

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150121/4a234786/attachment-0001.sig>