Part of a series of cleanups to use new accessor methods. * src/xenxs/xen_sxpr.c (xenParseSxprDisks, xenParseSxpr) (xenFormatSxprDisk, xenFormatSxpr): Use accessors. * src/xenxs/xen_xm.c (xenParseXM, xenFormatXMDisk, xenFormatXM): Likewise. Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> --- This one is a bit trickier to review, in that it is replacing in-place strndup over to calls to a function that mallocs a copy of the input string. While it compiles and passes 'make check', I don't actually have a xen setup to actually prove that it works. src/xenxs/xen_sxpr.c | 111 ++++++++++++++++++++++++++------------------------ src/xenxs/xen_xm.c | 113 +++++++++++++++++++++++++++------------------------ 2 files changed, 118 insertions(+), 106 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 01d1ca1..e03e254 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1,7 +1,7 @@ /* * xen_sxpr.c: Xen SEXPR parsing functions * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH * Copyright (C) 2005 Anthony Liguori <aliguori@xxxxxxxxxx> * @@ -401,24 +401,23 @@ xenParseSxprDisks(virDomainDefPtr def, if (sexpr_lookup(node, "device/tap2") && STRPREFIX(src, "tap:")) { - if (VIR_STRDUP(disk->driverName, "tap2") < 0) + if (virDomainDiskSetDriver(disk, "tap2") < 0) goto error; } else { - if (VIR_ALLOC_N(disk->driverName, (offset-src)+1) < 0) + char *tmp; + if (VIR_STRNDUP(tmp, src, offset - src) < 0) goto error; - if (virStrncpy(disk->driverName, src, offset-src, - (offset-src)+1) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Driver name %s too big for destination"), - src); + if (virDomainDiskSetDriver(disk, tmp) < 0) { + VIR_FREE(tmp); goto error; } + VIR_FREE(tmp); } src = offset + 1; - if (STREQ(disk->driverName, "tap") || - STREQ(disk->driverName, "tap2")) { + if (STREQ(virDomainDiskGetDriver(disk), "tap") || + STREQ(virDomainDiskGetDriver(disk), "tap2")) { char *driverType = NULL; offset = strchr(src, ':'); @@ -431,12 +430,12 @@ xenParseSxprDisks(virDomainDefPtr def, if (VIR_STRNDUP(driverType, src, offset - src) < 0) goto error; if (STREQ(driverType, "aio")) - disk->format = VIR_STORAGE_FILE_RAW; + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); else - disk->format = - virStorageFileFormatTypeFromString(driverType); + virDomainDiskSetFormat(disk, + virStorageFileFormatTypeFromString(driverType)); VIR_FREE(driverType); - if (disk->format <= 0) { + if (virDomainDiskGetFormat(disk) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown driver type %s"), src); goto error; @@ -448,17 +447,17 @@ xenParseSxprDisks(virDomainDefPtr def, so we assume common case here. If blktap becomes omnipotent, we can revisit this, perhaps stat()'ing the src file in question */ - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; - } else if (STREQ(disk->driverName, "phy")) { - disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - } else if (STREQ(disk->driverName, "file")) { - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); + } else if (STREQ(virDomainDiskGetDriver(disk), "phy")) { + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); + } else if (STREQ(virDomainDiskGetDriver(disk), "file")) { + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); } } else { /* No CDROM media so can't really tell. We'll just call if a FILE for now and update when media is inserted later */ - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); } if (STREQLEN(dst, "ioemu:", 6)) @@ -482,7 +481,7 @@ xenParseSxprDisks(virDomainDefPtr def, if (VIR_STRDUP(disk->dst, dst) < 0) goto error; - if (VIR_STRDUP(disk->src, src) < 0) + if (virDomainDiskSetSource(disk, src) < 0) goto error; if (STRPREFIX(disk->dst, "xvd")) @@ -1307,17 +1306,17 @@ xenParseSxpr(const struct sexpr *root, virDomainDiskDefPtr disk; if (VIR_ALLOC(disk) < 0) goto error; - if (VIR_STRDUP(disk->src, tmp) < 0) { + if (virDomainDiskSetSource(disk, tmp) < 0) { virDomainDiskDefFree(disk); goto error; } - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; if (VIR_STRDUP(disk->dst, "hdc") < 0) { virDomainDiskDefFree(disk); goto error; } - if (VIR_STRDUP(disk->driverName, "file") < 0) { + if (virDomainDiskSetDriver(disk, "file") < 0) { virDomainDiskDefFree(disk); goto error; } @@ -1342,17 +1341,17 @@ xenParseSxpr(const struct sexpr *root, virDomainDiskDefPtr disk; if (VIR_ALLOC(disk) < 0) goto error; - if (VIR_STRDUP(disk->src, tmp) < 0) { + if (virDomainDiskSetSource(disk, tmp) < 0) { VIR_FREE(disk); goto error; } - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); disk->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; if (VIR_STRDUP(disk->dst, fds[i]) < 0) { virDomainDiskDefFree(disk); goto error; } - if (VIR_STRDUP(disk->driverName, "file") < 0) { + if (virDomainDiskSetSource(disk, "file") < 0) { virDomainDiskDefFree(disk); goto error; } @@ -1722,6 +1721,9 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, int xendConfigVersion, int isAttach) { + const char *src = virDomainDiskGetSource(def); + const char *driver = virDomainDiskGetDriver(def); + /* Xend (all versions) put the floppy device config * under the hvm (image (os)) block */ @@ -1729,7 +1731,7 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { if (isAttach) { virReportError(VIR_ERR_INVALID_ARG, - _("Cannot directly attach floppy %s"), def->src); + _("Cannot directly attach floppy %s"), src); return -1; } return 0; @@ -1741,7 +1743,7 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, xendConfigVersion == XEND_CONFIG_VERSION_3_0_2) { if (isAttach) { virReportError(VIR_ERR_INVALID_ARG, - _("Cannot directly attach CDROM %s"), def->src); + _("Cannot directly attach CDROM %s"), src); return -1; } return 0; @@ -1753,9 +1755,9 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, /* Normally disks are in a (device (vbd ...)) block * but blktap disks ended up in a differently named * (device (tap ....)) block.... */ - if (def->driverName && STREQ(def->driverName, "tap")) { + if (STREQ_NULLABLE(driver, "tap")) { virBufferAddLit(buf, "(tap "); - } else if (def->driverName && STREQ(def->driverName, "tap2")) { + } else if (STREQ_NULLABLE(driver, "tap2")) { virBufferAddLit(buf, "(tap2 "); } else { virBufferAddLit(buf, "(vbd "); @@ -1778,36 +1780,39 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, virBufferEscapeSexpr(buf, "(dev '%s')", def->dst); } - if (def->src) { - if (def->driverName) { - if (STREQ(def->driverName, "tap") || - STREQ(def->driverName, "tap2")) { + if (src) { + if (driver) { + if (STREQ(driver, "tap") || + STREQ(driver, "tap2")) { const char *type; + int format = virDomainDiskGetFormat(def); - if (!def->format || def->format == VIR_STORAGE_FILE_RAW) + if (!format || format == VIR_STORAGE_FILE_RAW) type = "aio"; else - type = virStorageFileFormatTypeToString(def->format); - virBufferEscapeSexpr(buf, "(uname '%s:", def->driverName); + type = virStorageFileFormatTypeToString(format); + virBufferEscapeSexpr(buf, "(uname '%s:", driver); virBufferEscapeSexpr(buf, "%s:", type); - virBufferEscapeSexpr(buf, "%s')", def->src); + virBufferEscapeSexpr(buf, "%s')", src); } else { - virBufferEscapeSexpr(buf, "(uname '%s:", def->driverName); - virBufferEscapeSexpr(buf, "%s')", def->src); + virBufferEscapeSexpr(buf, "(uname '%s:", driver); + virBufferEscapeSexpr(buf, "%s')", src); } } else { - if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) { - virBufferEscapeSexpr(buf, "(uname 'file:%s')", def->src); - } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { - if (def->src[0] == '/') - virBufferEscapeSexpr(buf, "(uname 'phy:%s')", def->src); + int type = virDomainDiskGetType(def); + + if (type == VIR_DOMAIN_DISK_TYPE_FILE) { + virBufferEscapeSexpr(buf, "(uname 'file:%s')", src); + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + if (src[0] == '/') + virBufferEscapeSexpr(buf, "(uname 'phy:%s')", src); else virBufferEscapeSexpr(buf, "(uname 'phy:/dev/%s')", - def->src); + src); } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported disk type %s"), - virDomainDiskTypeToString(def->type)); + virDomainDiskTypeToString(type)); return -1; } } @@ -2313,23 +2318,23 @@ xenFormatSxpr(virConnectPtr conn, /* some disk devices are defined here */ for (i = 0; i < def->ndisks; i++) { + const char *src = virDomainDiskGetSource(def->disks[i]); + switch (def->disks[i]->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: /* Only xend <= 3.0.2 wants cdrom config here */ if (xendConfigVersion != XEND_CONFIG_VERSION_3_0_2) break; - if (!STREQ(def->disks[i]->dst, "hdc") || - def->disks[i]->src == NULL) + if (!STREQ(def->disks[i]->dst, "hdc") || !src) break; - virBufferEscapeSexpr(&buf, "(cdrom '%s')", - def->disks[i]->src); + virBufferEscapeSexpr(&buf, "(cdrom '%s')", src); break; case VIR_DOMAIN_DISK_DEVICE_FLOPPY: /* all xend versions define floppies here */ virBufferEscapeSexpr(&buf, "(%s ", def->disks[i]->dst); - virBufferEscapeSexpr(&buf, "'%s')", def->disks[i]->src); + virBufferEscapeSexpr(&buf, "'%s')", src); break; default: diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index a70c5e3..d74e427 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1,7 +1,7 @@ /* * xen_xm.c: Xen XM parsing functions * - * Copyright (C) 2006-2007, 2009-2010, 2012-2013 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2014 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH * Copyright (C) 2006 Daniel P. Berrange * @@ -472,6 +472,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, char *head; char *offset; char *tmp; + const char *src; if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) goto skipdisk; @@ -493,17 +494,16 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, goto skipdisk; if (offset == head) { - disk->src = NULL; /* No source file given, eg CDROM with no media */ + /* No source file given, eg CDROM with no media */ + ignore_value(virDomainDiskSetSource(disk, NULL)); } else { - if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0) + if (VIR_STRNDUP(tmp, head, offset - head) < 0) goto cleanup; - if (virStrncpy(disk->src, head, offset - head, - (offset - head) + 1) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Source file %s too big for destination"), - head); + if (virDomainDiskSetSource(disk, tmp) < 0) { + VIR_FREE(tmp); goto cleanup; } + VIR_FREE(tmp); } head = offset + 1; @@ -524,65 +524,68 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, } head = offset + 1; - /* Extract source driver type */ - if (disk->src) { + src = virDomainDiskGetSource(disk); + if (src) { + size_t len; /* The main type phy:, file:, tap: ... */ - if ((tmp = strchr(disk->src, ':')) != NULL) { - if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0) + if ((tmp = strchr(src, ':')) != NULL) { + len = tmp - src; + if (VIR_STRNDUP(tmp, src, len) < 0) goto cleanup; - if (virStrncpy(disk->driverName, disk->src, - (tmp - disk->src), - (tmp - disk->src) + 1) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Driver name %s too big for destination"), - disk->src); + if (virDomainDiskSetDriver(disk, tmp) < 0) { + VIR_FREE(tmp); goto cleanup; } + VIR_FREE(tmp); /* Strip the prefix we found off the source file name */ - memmove(disk->src, disk->src+(tmp-disk->src)+1, - strlen(disk->src)-(tmp-disk->src)); + if (virDomainDiskSetSource(disk, src + len + 1) < 0) + goto cleanup; + src = virDomainDiskGetSource(disk); } /* And the sub-type for tap:XXX: type */ - if (disk->driverName && - STREQ(disk->driverName, "tap")) { + if (STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap")) { char *driverType; - if (!(tmp = strchr(disk->src, ':'))) + if (!(tmp = strchr(src, ':'))) goto skipdisk; + len = tmp - src; - if (VIR_STRNDUP(driverType, disk->src, tmp - disk->src) < 0) + if (VIR_STRNDUP(driverType, src, len) < 0) goto cleanup; if (STREQ(driverType, "aio")) - disk->format = VIR_STORAGE_FILE_RAW; + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); else - disk->format = - virStorageFileFormatTypeFromString(driverType); + virDomainDiskSetFormat(disk, + virStorageFileFormatTypeFromString(driverType)); VIR_FREE(driverType); - if (disk->format <= 0) { + if (virDomainDiskGetFormat(disk) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown driver type %s"), - disk->src); + src); goto cleanup; } /* Strip the prefix we found off the source file name */ - memmove(disk->src, disk->src+(tmp-disk->src)+1, - strlen(disk->src)-(tmp-disk->src)); + if (virDomainDiskSetSource(disk, src + len + 1) < 0) + goto cleanup; + src = virDomainDiskGetSource(disk); } } /* No source, or driver name, so fix to phy: */ - if (!disk->driverName && - VIR_STRDUP(disk->driverName, "phy") < 0) + if (!virDomainDiskGetDriver(disk) && + virDomainDiskSetDriver(disk, "phy") < 0) goto cleanup; /* phy: type indicates a block device */ - disk->type = STREQ(disk->driverName, "phy") ? - VIR_DOMAIN_DISK_TYPE_BLOCK : VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(disk, + STREQ(virDomainDiskGetDriver(disk), "phy") ? + VIR_DOMAIN_DISK_TYPE_BLOCK : + VIR_DOMAIN_DISK_TYPE_FILE); /* Check for a :cdrom/:disk postfix */ disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; @@ -624,11 +627,11 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (VIR_ALLOC(disk) < 0) goto cleanup; - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; - if (VIR_STRDUP(disk->driverName, "file") < 0) + if (virDomainDiskSetDriver(disk, "file") < 0) goto cleanup; - if (VIR_STRDUP(disk->src, str) < 0) + if (virDomainDiskSetSource(disk, str) < 0) goto cleanup; if (VIR_STRDUP(disk->dst, "hdc") < 0) goto cleanup; @@ -1176,27 +1179,31 @@ int xenXMConfigSetString(virConfPtr conf, const char *setting, const char *str) } -static int xenFormatXMDisk(virConfValuePtr list, - virDomainDiskDefPtr disk, - int hvm, - int xendConfigVersion) +static int +xenFormatXMDisk(virConfValuePtr list, + virDomainDiskDefPtr disk, + int hvm, + int xendConfigVersion) { virBuffer buf = VIR_BUFFER_INITIALIZER; virConfValuePtr val, tmp; + const char *src = virDomainDiskGetSource(disk); + int format = virDomainDiskGetFormat(disk); + const char *driver = virDomainDiskGetDriver(disk); - if (disk->src) { - if (disk->format) { + if (src) { + if (format) { const char *type; - if (disk->format == VIR_STORAGE_FILE_RAW) + if (format == VIR_STORAGE_FILE_RAW) type = "aio"; else - type = virStorageFileFormatTypeToString(disk->format); - virBufferAsprintf(&buf, "%s:", disk->driverName); - if (STREQ(disk->driverName, "tap")) + type = virStorageFileFormatTypeToString(format); + virBufferAsprintf(&buf, "%s:", driver); + if (STREQ(driver, "tap")) virBufferAsprintf(&buf, "%s:", type); } else { - switch (disk->type) { + switch (virDomainDiskGetType(disk)) { case VIR_DOMAIN_DISK_TYPE_FILE: virBufferAddLit(&buf, "file:"); break; @@ -1206,11 +1213,11 @@ static int xenFormatXMDisk(virConfValuePtr list, default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported disk type %s"), - virDomainDiskTypeToString(disk->type)); + virDomainDiskTypeToString(virDomainDiskGetType(disk))); goto cleanup; } } - virBufferAdd(&buf, disk->src, -1); + virBufferAdd(&buf, src, -1); } virBufferAddLit(&buf, ","); if (hvm && xendConfigVersion == XEND_CONFIG_VERSION_3_0_2) @@ -1602,9 +1609,9 @@ virConfPtr xenFormatXM(virConnectPtr conn, if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM && def->disks[i]->dst && STREQ(def->disks[i]->dst, "hdc") && - def->disks[i]->src) { + virDomainDiskGetSource(def->disks[i])) { if (xenXMConfigSetString(conf, "cdrom", - def->disks[i]->src) < 0) + virDomainDiskGetSource(def->disks[i])) < 0) goto cleanup; break; } -- 1.8.5.3 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list