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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxx>
> *
> * 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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list