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@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@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.