[libvirt] [PATCH 15/18] conf: use disk source accessors in vmx/

[Laine Stump]

On 03/19/2014 11:20 AM, Eric Blake wrote:
> Part of a series of cleanups to use new accessor methods.
>
> * src/vmx/vmx.c (virVMXHandleLegacySCSIDiskDriverName)
> (virVMXParseDisk, virVMXFormatDisk, virVMXFormatFloppy): Use
> accessors.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/vmx/vmx.c | 113 ++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 66 insertions(+), 47 deletions(-)
>
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index 1ebb0f9..341d1af 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -1,7 +1,7 @@
>  /*
>   * vmx.c: VMware VMX parsing/formatting functions
>   *
> - * Copyright (C) 2010-2013 Red Hat, Inc.
> + * Copyright (C) 2010-2014 Red Hat, Inc.
>   * Copyright (C) 2009-2010 Matthias Bolte <matthias.bolte at googlemail.com>
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -1060,22 +1060,27 @@ virVMXHandleLegacySCSIDiskDriverName(virDomainDefPtr def,
>      int model;
>      size_t i;
>      virDomainControllerDefPtr controller = NULL;
> +    const char *driver = virDomainDiskGetDriver(disk);
> +    char *copy;
>
> -    if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI || disk->driverName == NULL) {
> +    if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI || !driver) {
>          return 0;
>      }
>
> -    tmp = disk->driverName;
> +    if (VIR_STRDUP(copy, driver) < 0)
> +        return -1;
> +    tmp = copy;
>
>      for (; *tmp != '\0'; ++tmp) {
>          *tmp = c_tolower(*tmp);
>      }
>
> -    model = virDomainControllerModelSCSITypeFromString(disk->driverName);
> +    model = virDomainControllerModelSCSITypeFromString(copy);
> +    VIR_FREE(copy);
>
>      if (model < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Unknown driver name '%s'"), disk->driverName);
> +                       _("Unknown driver name '%s'"), driver);
>          return -1;
>      }
>
> @@ -1098,7 +1103,7 @@ virVMXHandleLegacySCSIDiskDriverName(virDomainDefPtr def,
>      } else if (controller->model != model) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Inconsistent SCSI controller model ('%s' is not '%s') "
> -                         "for SCSI controller index %d"), disk->driverName,
> +                         "for SCSI controller index %d"), driver,
>                         virDomainControllerModelSCSITypeToString(controller->model),
>                         controller->idx);
>          return -1;
> @@ -2179,15 +2184,18 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
>                  }
>              }
>
> -            (*def)->type = VIR_DOMAIN_DISK_TYPE_FILE;
> -            (*def)->src = ctx->parseFileName(fileName, ctx->opaque);
> +            virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE);
> +            if (virDomainDiskSetSource(*def,
> +                                       ctx->parseFileName(fileName,
> +                                                          ctx->opaque)) < 0)


The above will leak the string returned from ctx->parseFileName. There
is also at least one other place further down where the same thing occurs.


> +                goto cleanup;
>              (*def)->cachemode = writeThrough ? VIR_DOMAIN_DISK_CACHE_WRITETHRU
>                                               : VIR_DOMAIN_DISK_CACHE_DEFAULT;
>              if (mode)
>                  (*def)->transient = STRCASEEQ(mode,
>                                                "independent-nonpersistent");
>
> -            if ((*def)->src == NULL) {
> +            if (!virDomainDiskGetSource(*def)) {
>                  goto cleanup;
>              }
>          } else if (virFileHasSuffix(fileName, ".iso") ||
> @@ -2219,10 +2227,13 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
>                  }
>              }
>
> -            (*def)->type = VIR_DOMAIN_DISK_TYPE_FILE;
> -            (*def)->src = ctx->parseFileName(fileName, ctx->opaque);
> +            virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE);
> +            if (virDomainDiskSetSource(*def,
> +                                       ctx->parseFileName(fileName,
> +                                                          ctx->opaque)) < 0)
> +                goto cleanup;
>
> -            if ((*def)->src == NULL) {
> +            if (!virDomainDiskGetSource(*def)) {
>                  goto cleanup;
>              }
>          } else if (virFileHasSuffix(fileName, ".vmdk")) {
> @@ -2234,26 +2245,24 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
>               */
>              goto ignore;
>          } else if (STRCASEEQ(deviceType, "atapi-cdrom")) {
> -            (*def)->type = VIR_DOMAIN_DISK_TYPE_BLOCK;
> +            virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_BLOCK);
>
>              if (STRCASEEQ(fileName, "auto detect")) {
> -                (*def)->src = NULL;
> +                ignore_value(virDomainDiskSetSource(*def, NULL));
>                  (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
> -            } else {
> -                (*def)->src = fileName;
> -                fileName = NULL;
> +            } else if (virDomainDiskSetSource(*def, fileName) < 0) {
> +                goto cleanup;
>              }
>          } else if (STRCASEEQ(deviceType, "cdrom-raw")) {
>              /* Raw access CD-ROMs actually are device='lun' */
>              (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
> -            (*def)->type = VIR_DOMAIN_DISK_TYPE_BLOCK;
> +            virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_BLOCK);
>
>              if (STRCASEEQ(fileName, "auto detect")) {
> -                (*def)->src = NULL;
> +                ignore_value(virDomainDiskSetSource(*def, NULL));
>                  (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
> -            } else {
> -                (*def)->src = fileName;
> -                fileName = NULL;
> +            } else if (virDomainDiskSetSource(*def, fileName) < 0) {
> +                goto cleanup;
>              }
>          } else {
>              virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -2265,13 +2274,15 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
>          }
>      } else if (device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
>          if (fileType != NULL && STRCASEEQ(fileType, "device")) {
> -            (*def)->type = VIR_DOMAIN_DISK_TYPE_BLOCK;
> -            (*def)->src = fileName;
> -
> -            fileName = NULL;
> +            virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_BLOCK);
> +            if (virDomainDiskSetSource(*def, fileName) < 0)
> +                goto cleanup;
>          } else if (fileType != NULL && STRCASEEQ(fileType, "file")) {
> -            (*def)->type = VIR_DOMAIN_DISK_TYPE_FILE;
> -            (*def)->src = ctx->parseFileName(fileName, ctx->opaque);
> +            virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE);
> +            if (virDomainDiskSetSource(*def,
> +                                       ctx->parseFileName(fileName,
> +                                                          ctx->opaque)) < 0)

... here.