[libvirt] [PATCHv2 04/17] conf: add virDomainControllerDefNew()

John Ferlan jferlan at redhat.com
Wed Jul 22 19:11:28 UTC 2015



On 07/17/2015 02:43 PM, Laine Stump wrote:
> There are some non-0 default values in virDomainControllerDef (and
> will soon be more) that are easier to not forget if the remembering is
> done by a single initializer function (rather than inline code after
> allocating the obejct with generic VIR_ALLOC().
> ---
> new in V2
> 
>  src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5a9a88d..8dd4bf0 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1527,6 +1527,37 @@ virDomainDiskSetFormat(virDomainDiskDefPtr def, int format)
>  }
>  
>  
> +static virDomainControllerDefPtr
> +virDomainControllerDefNew(virDomainControllerType type)
> +{
> +    virDomainControllerDefPtr def;
> +
> +    if (VIR_ALLOC(def) < 0)
> +        return NULL;
> +
> +    def->type = type;
> +
> +    /* initialize anything that has a non-0 default */
> +    switch ((virDomainControllerType) def->type) {
> +    case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
> +        def->opts.vioserial.ports = -1;
> +        def->opts.vioserial.vectors = -1;
> +        break;
> +    case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> +    case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> +    case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
> +    case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> +    case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
> +    case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
> +    case VIR_DOMAIN_CONTROLLER_TYPE_USB:
> +    case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
> +        break;
> +    }
> +
> +    return def;
> +}
> +
> +
>  void virDomainControllerDefFree(virDomainControllerDefPtr def)
>  {
>      if (!def)
> @@ -7597,9 +7628,10 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>                                 xmlXPathContextPtr ctxt,
>                                 unsigned int flags)
>  {
> -    virDomainControllerDefPtr def;
> +    virDomainControllerDefPtr def = NULL;
> +    int type = 0;

Should we make this?

    type = VIR_DOMAIN_CONTROLLER_TYPE_IDE;

Not that it perhaps matters too much, but does perhaps point to
where/why an IDE controller got created if "type='xxx'"

ACK - whether or not you make that change

John
>      xmlNodePtr cur = NULL;
> -    char *type = NULL;
> +    char *typeStr = NULL;
>      char *idx = NULL;
>      char *model = NULL;
>      char *queues = NULL;
> @@ -7610,18 +7642,18 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>  
>      ctxt->node = node;
>  
> -    if (VIR_ALLOC(def) < 0)
> -        return NULL;
> -
> -    type = virXMLPropString(node, "type");
> -    if (type) {
> -        if ((def->type = virDomainControllerTypeFromString(type)) < 0) {
> +    typeStr = virXMLPropString(node, "type");
> +    if (typeStr) {
> +        if ((type = virDomainControllerTypeFromString(typeStr)) < 0) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("Unknown controller type '%s'"), type);
> +                           _("Unknown controller type '%s'"), typeStr);
>              goto error;
>          }
>      }
>  
> +    if (!(def = virDomainControllerDefNew(type)))
> +        return NULL;
> +
>      idx = virXMLPropString(node, "index");
>      if (idx) {
>          if (virStrToLong_ui(idx, NULL, 10, &def->idx) < 0 ||
> @@ -7692,8 +7724,6 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>                  VIR_FREE(ports);
>                  goto error;
>              }
> -        } else {
> -            def->opts.vioserial.ports = -1;
>          }
>          VIR_FREE(ports);
>  
> @@ -7707,8 +7737,6 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>                  VIR_FREE(vectors);
>                  goto error;
>              }
> -        } else {
> -            def->opts.vioserial.vectors = -1;
>          }
>          VIR_FREE(vectors);
>          break;
> @@ -7780,7 +7808,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>  
>   cleanup:
>      ctxt->node = saved;
> -    VIR_FREE(type);
> +    VIR_FREE(typeStr);
>      VIR_FREE(idx);
>      VIR_FREE(model);
>      VIR_FREE(queues);
> @@ -13960,18 +13988,12 @@ virDomainDefMaybeAddController(virDomainDefPtr def,
>              return 0;
>      }
>  
> -    if (VIR_ALLOC(cont) < 0)
> +    if (!(cont = virDomainControllerDefNew(type)))
>          return -1;
>  
> -    cont->type = type;
>      cont->idx = idx;
>      cont->model = model;
>  
> -    if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) {
> -        cont->opts.vioserial.ports = -1;
> -        cont->opts.vioserial.vectors = -1;
> -    }
> -
>      if (VIR_APPEND_ELEMENT(def->controllers, def->ncontrollers, cont) < 0) {
>          VIR_FREE(cont);
>          return -1;
> 




More information about the libvir-list mailing list