[libvirt] [PATCH 00/18] split virDomainDiskDef

I still have quite a bit of work to consolidate util/virStorageFile, conf/snapshot_conf.c, and the new struct in this series to all share the same common struct (basically, moving the struct created in this series over to util). But by refactoring the most common accesses to go through functions instead of direct field access, I've limited the amount of code that is actually impacted by using the new struct. In general, this series should not be changing any behavior. Eric Blake (18): conf: accessors for common source information conf: use disk source accessors in conf/ conf: use disk source accessors in bhyve/ conf: use disk source accessors in esx/ conf: use disk source accessors in libxl/ conf: use disk source accessors in locking/ conf: use disk source accessors in lxc/ conf: use disk source accessors in parallels/ conf: use disk source accessors in phyp/ conf: use disk source accessors in qemu/ conf: use disk source accessors in security/ conf: use disk source accessors in uml/ conf: use disk source accessors in vbox/ conf: use disk source accessors in vmware/ conf: use disk source accessors in vmx/ conf: use disk source accessors in xen/ conf: use disk source accessors in xenxs/ conf: prepare to track multiple host source files per <disk> src/bhyve/bhyve_command.c | 9 +- src/conf/domain_audit.c | 7 +- src/conf/domain_conf.c | 253 +++++++++++++++++++++----------- src/conf/domain_conf.h | 47 ++++-- src/conf/snapshot_conf.c | 2 +- src/esx/esx_driver.c | 26 ++-- src/libvirt_private.syms | 8 + src/libxl/libxl_conf.c | 50 ++++--- src/libxl/libxl_driver.c | 26 ++-- src/locking/domain_lock.c | 19 ++- src/lxc/lxc_cgroup.c | 6 +- src/lxc/lxc_controller.c | 77 +++++----- src/lxc/lxc_driver.c | 35 +++-- src/parallels/parallels_driver.c | 31 ++-- src/phyp/phyp_driver.c | 10 +- src/qemu/qemu_command.c | 308 ++++++++++++++++++++------------------- src/qemu/qemu_conf.c | 108 +++++++------- src/qemu/qemu_domain.c | 60 ++++---- src/qemu/qemu_driver.c | 203 +++++++++++++------------- src/qemu/qemu_hotplug.c | 116 ++++++++------- src/qemu/qemu_migration.c | 21 +-- src/qemu/qemu_process.c | 19 +-- src/security/security_dac.c | 17 ++- src/security/security_selinux.c | 20 +-- src/storage/storage_driver.c | 8 +- src/uml/uml_conf.c | 4 +- src/uml/uml_driver.c | 5 +- src/vbox/vbox_tmpl.c | 137 +++++++++-------- src/vmware/vmware_conf.c | 18 ++- src/vmware/vmware_conf.h | 6 +- src/vmx/vmx.c | 113 ++++++++------ src/xen/xend_internal.c | 11 +- src/xenxs/xen_sxpr.c | 111 +++++++------- src/xenxs/xen_xm.c | 113 +++++++------- tests/securityselinuxlabeltest.c | 8 +- 35 files changed, 1119 insertions(+), 893 deletions(-) -- 1.8.5.3

A future patch will split virDomainDiskDef, in order to track multiple host resources per guest <disk>. To reduce the size of that patch, I've factored out the four most common accesses into functions, so that I can incrementally upgrade the code base to use the accessors, and so that code that doesn't care about the distinction of per-file details won't have to be changed when the struct changes. * src/conf/domain_conf.h (virDomainDiskGetType) (virDomainDiskSetType, virDomainDiskGetSource) (virDomainDiskSetSource, virDomainDiskGetDriver) (virDomainDiskSetDriver, virDomainDiskGetFormat) (virDomainDiskSetFormat): New prototypes. * src/conf/domain_conf.c (virDomainDiskGetType) (virDomainDiskSetType, virDomainDiskGetSource) (virDomainDiskSetSource, virDomainDiskGetDriver) (virDomainDiskSetDriver, virDomainDiskGetFormat) (virDomainDiskSetFormat): Implement them. * src/libvirt_private.syms (domain_conf.h): Export them. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/domain_conf.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 +++++++ src/libvirt_private.syms | 8 ++++++ 3 files changed, 90 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89aa52c..d2724ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1355,6 +1355,20 @@ error: int +virDomainDiskGetType(virDomainDiskDefPtr def) +{ + return def->type; +} + + +void +virDomainDiskSetType(virDomainDiskDefPtr def, int type) +{ + def->type = type; +} + + +int virDomainDiskGetActualType(virDomainDiskDefPtr def) { if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME && def->srcpool) @@ -1364,6 +1378,64 @@ virDomainDiskGetActualType(virDomainDiskDefPtr def) } +const char * +virDomainDiskGetSource(virDomainDiskDefPtr def) +{ + return def->src; +} + + +int +virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src) +{ + int ret; + char *tmp = def->src; + + ret = VIR_STRDUP(def->src, src); + if (ret < 0) + def->src = tmp; + else + VIR_FREE(tmp); + return ret; +} + + +const char * +virDomainDiskGetDriver(virDomainDiskDefPtr def) +{ + return def->driverName; +} + + +int +virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) +{ + int ret; + char *tmp = def->driverName; + + ret = VIR_STRDUP(def->driverName, name); + if (ret < 0) + def->driverName = tmp; + else + VIR_FREE(tmp); + return ret; +} + + +int +virDomainDiskGetFormat(virDomainDiskDefPtr def) +{ + return def->format; +} + + +void +virDomainDiskSetFormat(virDomainDiskDefPtr def, int format) +{ + def->format = format; +} + + void virDomainControllerDefFree(virDomainControllerDefPtr def) { if (!def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 27f07e6..cc447b0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2256,7 +2256,17 @@ void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def); void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts); virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts, virDomainDiskHostDefPtr hosts); +int virDomainDiskGetType(virDomainDiskDefPtr def); +void virDomainDiskSetType(virDomainDiskDefPtr def, int type); int virDomainDiskGetActualType(virDomainDiskDefPtr def); +const char *virDomainDiskGetSource(virDomainDiskDefPtr def); +int virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src) + ATTRIBUTE_RETURN_CHECK; +const char *virDomainDiskGetDriver(virDomainDiskDefPtr def); +int virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) + ATTRIBUTE_RETURN_CHECK; +int virDomainDiskGetFormat(virDomainDiskDefPtr def); +void virDomainDiskSetFormat(virDomainDiskDefPtr def, int format); int virDomainDeviceFindControllerModel(virDomainDefPtr def, virDomainDeviceInfoPtr info, int controllerType); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3baf766..624b420 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -200,6 +200,10 @@ virDomainDiskFindByBusAndDst; virDomainDiskGeometryTransTypeFromString; virDomainDiskGeometryTransTypeToString; virDomainDiskGetActualType; +virDomainDiskGetDriver; +virDomainDiskGetFormat; +virDomainDiskGetSource; +virDomainDiskGetType; virDomainDiskHostDefClear; virDomainDiskHostDefCopy; virDomainDiskHostDefFree; @@ -214,6 +218,10 @@ virDomainDiskProtocolTransportTypeToString; virDomainDiskProtocolTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; +virDomainDiskSetDriver; +virDomainDiskSetFormat; +virDomainDiskSetSource; +virDomainDiskSetType; virDomainDiskSourceIsBlockType; virDomainDiskTypeFromString; virDomainDiskTypeToString; -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. Several places in domain_conf.c still open-code raw field access, but that code will be touched later with the diskDef struct split so I'm avoiding churn here. * src/conf/domain_audit.c (virDomainAuditStart): Use accessors. * src/conf/domain_conf.c (virDomainDiskIndexByName) (virDomainDiskPathByName, virDomainDiskDefForeachPath) (virDomainDiskSourceIsBlockType): Likewise. * src/conf/snapshot_conf.c (virDomainSnapshotAlignDisks): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/domain_audit.c | 7 ++++--- src/conf/domain_conf.c | 20 +++++++++++--------- src/conf/snapshot_conf.c | 2 +- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 69632b0..e8bd460 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -799,9 +799,10 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) size_t i; for (i = 0; i < vm->def->ndisks; i++) { - virDomainDiskDefPtr disk = vm->def->disks[i]; - if (disk->src) /* Skips CDROM without media initially inserted */ - virDomainAuditDisk(vm, NULL, disk->src, "start", true); + const char *src = virDomainDiskGetSource(vm->def->disks[i]); + + if (src) /* Skips CDROM without media initially inserted */ + virDomainAuditDisk(vm, NULL, src, "start", true); } for (i = 0; i < vm->def->nfss; i++) { diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d2724ca..a0e07c7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10233,8 +10233,7 @@ virDomainDiskIndexByName(virDomainDefPtr def, const char *name, if (*name != '/') { if (STREQ(vdisk->dst, name)) return i; - } else if (vdisk->src && - STREQ(vdisk->src, name)) { + } else if (STREQ_NULLABLE(virDomainDiskGetSource(vdisk), name)) { if (allow_ambiguous) return i; if (candidate >= 0) @@ -10253,7 +10252,7 @@ virDomainDiskPathByName(virDomainDefPtr def, const char *name) { int idx = virDomainDiskIndexByName(def, name, true); - return idx < 0 ? NULL : def->disks[idx]->src; + return idx < 0 ? NULL : virDomainDiskGetSource(def->disks[idx]); } int virDomainDiskInsert(virDomainDefPtr def, @@ -18532,14 +18531,16 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, int ret = -1; size_t depth = 0; virStorageFileMetadata *tmp; + const char *path = virDomainDiskGetSource(disk); + int type = virDomainDiskGetType(disk); - if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK || - (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && + if (!path || type == VIR_DOMAIN_DISK_TYPE_NETWORK || + (type == VIR_DOMAIN_DISK_TYPE_VOLUME && disk->srcpool && disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT)) return 0; - if (iter(disk, disk->src, 0, opaque) < 0) + if (iter(disk, path, 0, opaque) < 0) goto cleanup; tmp = disk->backingChain; @@ -19409,16 +19410,17 @@ virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) /* No reason to think the disk source is block type if * the source is empty */ - if (!def->src) + if (!virDomainDiskGetSource(def)) return false; - if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) + if (virDomainDiskGetType(def) == VIR_DOMAIN_DISK_TYPE_BLOCK) return true; /* For volume types, check the srcpool. * If it's a block type source pool, then it's possible */ - if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME && def->srcpool && + if (virDomainDiskGetType(def) == VIR_DOMAIN_DISK_TYPE_VOLUME && + def->srcpool && def->srcpool->voltype == VIR_STORAGE_VOL_BLOCK) { /* We don't think the volume accessed by remote URI is * block type source, since we can't/shouldn't manage it diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 6fa14ed..0509743 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -558,7 +558,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL && !disk->file) { - const char *original = def->dom->disks[i]->src; + const char *original = virDomainDiskGetSource(def->dom->disks[i]); const char *tmp; struct stat sb; -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/bhyve/bhyve_command.c (bhyveBuildDiskArgStr) (virBhyveProcessBuildLoadCmd): Use accessors. Signed-off-by: Eric Blake <eblake@redhat.com> --- I haven't actually compile-tested this one yet, but unless my grep was off, I think I got it correctly. src/bhyve/bhyve_command.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 15029cd..cfb577c 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -214,14 +214,15 @@ bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) return -1; } - if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) { + if (virDomainDiskGetType(disk) != VIR_DOMAIN_DISK_TYPE_FILE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("unsupported disk type")); return -1; } virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "2:0,%s,%s", bus_type, disk->src); + virCommandAddArgFormat(cmd, "2:0,%s,%s", bus_type, + virDomainDiskGetSource(disk)); return 0; } @@ -317,7 +318,7 @@ virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, return NULL; } - if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) { + if (virDomainDiskGetType(disk) != VIR_DOMAIN_DISK_TYPE_FILE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("unsupported disk type")); return NULL; @@ -332,7 +333,7 @@ virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, /* Image path */ virCommandAddArg(cmd, "-d"); - virCommandAddArg(cmd, disk->src); + virCommandAddArg(cmd, virDomainDiskGetSource(disk)); /* VM name */ virCommandAddArg(cmd, vm->def->name); -- 1.8.5.3

Eric Blake wrote:
Part of a series of cleanups to use new accessor methods.
* src/bhyve/bhyve_command.c (bhyveBuildDiskArgStr) (virBhyveProcessBuildLoadCmd): Use accessors.
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch builds fine for me. I also did a basic testing and didn't notice any regressions. Roman Bogorodskiy

Part of a series of cleanups to use new accessor methods. * src/esx/esx_driver.c (esxAutodetectSCSIControllerModel) (esxDomainDefineXML): Use accessors. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/esx/esx_driver.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index a0591f3..a08a69d 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1,7 +1,7 @@ /* * esx_driver.c: core driver functions for managing VMware ESX hosts * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2009-2013 Matthias Bolte <matthias.bolte@googlemail.com> * Copyright (C) 2009 Maximilian Wilhelm <max@rfc2324.org> * @@ -385,12 +385,12 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, esxVMX_Data *data = opaque; esxVI_FileInfo *fileInfo = NULL; esxVI_VmDiskFileInfo *vmDiskFileInfo = NULL; + const char *src = virDomainDiskGetSource(def); if (def->device != VIR_DOMAIN_DISK_DEVICE_DISK || def->bus != VIR_DOMAIN_DISK_BUS_SCSI || - def->type != VIR_DOMAIN_DISK_TYPE_FILE || - !def->src || - ! STRPREFIX(def->src, "[")) { + virDomainDiskGetType(def) != VIR_DOMAIN_DISK_TYPE_FILE || + !src || !STRPREFIX(src, "[")) { /* * This isn't a file-based SCSI disk device with a datastore related * source path => do nothing. @@ -398,7 +398,7 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, return 0; } - if (esxVI_LookupFileInfoByDatastorePath(data->ctx, def->src, + if (esxVI_LookupFileInfoByDatastorePath(data->ctx, src, false, &fileInfo, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; @@ -408,7 +408,7 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, if (!vmDiskFileInfo || !vmDiskFileInfo->controllerType) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not lookup controller model for '%s'"), def->src); + _("Could not lookup controller model for '%s'"), src); goto cleanup; } @@ -427,7 +427,7 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Found unexpected controller model '%s' for disk '%s'"), - vmDiskFileInfo->controllerType, def->src); + vmDiskFileInfo->controllerType, src); goto cleanup; } @@ -3045,6 +3045,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; virDomainPtr domain = NULL; + const char *src; memset(&data, 0, sizeof(data)); @@ -3121,7 +3122,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) for (i = 0; i < def->ndisks; ++i) { if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK && - def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE) { + virDomainDiskGetType(def->disks[i]) == VIR_DOMAIN_DISK_TYPE_FILE) { disk = def->disks[i]; break; } @@ -3134,22 +3135,23 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } - if (!disk->src) { + src = virDomainDiskGetSource(disk); + if (!src) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("First file-based harddisk has no source, cannot deduce " "datastore and path for VMX file")); goto cleanup; } - if (esxUtil_ParseDatastorePath(disk->src, &datastoreName, &directoryName, + if (esxUtil_ParseDatastorePath(src, &datastoreName, &directoryName, NULL) < 0) { goto cleanup; } - if (! virFileHasSuffix(disk->src, ".vmdk")) { + if (! virFileHasSuffix(src, ".vmdk")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting source '%s' of first file-based harddisk to " - "be a VMDK image"), disk->src); + "be a VMDK image"), src); goto cleanup; } -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/libxl/libxl_conf.c (libxlMakeDisk): Use accessors. * src/libxl/libxl_driver.c (libxlDomainChangeEjectableMedia) (libxlDomainAttachDeviceDiskLive): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++---------------------- src/libxl/libxl_driver.c | 26 ++++++++++--------------- 2 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index cfac847..cf110a3 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1,7 +1,7 @@ /* * libxl_conf.c: libxl configuration management * - * Copyright (C) 2012 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * Copyright (c) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany. * Copyright (C) 2011 Univention GmbH. * @@ -716,18 +716,22 @@ error: int libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) { + const char *driver; + int format; + libxl_device_disk_init(x_disk); - if (VIR_STRDUP(x_disk->pdev_path, l_disk->src) < 0) + if (VIR_STRDUP(x_disk->pdev_path, virDomainDiskGetSource(l_disk)) < 0) return -1; if (VIR_STRDUP(x_disk->vdev, l_disk->dst) < 0) return -1; - if (l_disk->driverName) { - if (STREQ(l_disk->driverName, "tap") || - STREQ(l_disk->driverName, "tap2")) { - switch (l_disk->format) { + driver = virDomainDiskGetDriver(l_disk); + format = virDomainDiskGetFormat(l_disk); + if (driver) { + if (STREQ(driver, "tap") || STREQ(driver, "tap2")) { + switch (format) { case VIR_STORAGE_FILE_QCOW: x_disk->format = LIBXL_DISK_FORMAT_QCOW; x_disk->backend = LIBXL_DISK_BACKEND_QDISK; @@ -750,13 +754,13 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight does not support disk format %s " "with disk driver %s"), - virStorageFileFormatTypeToString(l_disk->format), - l_disk->driverName); + virStorageFileFormatTypeToString(format), + driver); return -1; } - } else if (STREQ(l_disk->driverName, "qemu")) { + } else if (STREQ(driver, "qemu")) { x_disk->backend = LIBXL_DISK_BACKEND_QDISK; - switch (l_disk->format) { + switch (format) { case VIR_STORAGE_FILE_QCOW: x_disk->format = LIBXL_DISK_FORMAT_QCOW; break; @@ -775,30 +779,30 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight does not support disk format %s " "with disk driver %s"), - virStorageFileFormatTypeToString(l_disk->format), - l_disk->driverName); + virStorageFileFormatTypeToString(format), + driver); return -1; } - } else if (STREQ(l_disk->driverName, "file")) { - if (l_disk->format != VIR_STORAGE_FILE_NONE && - l_disk->format != VIR_STORAGE_FILE_RAW) { + } else if (STREQ(driver, "file")) { + if (format != VIR_STORAGE_FILE_NONE && + format != VIR_STORAGE_FILE_RAW) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight does not support disk format %s " "with disk driver %s"), - virStorageFileFormatTypeToString(l_disk->format), - l_disk->driverName); + virStorageFileFormatTypeToString(format), + driver); return -1; } x_disk->format = LIBXL_DISK_FORMAT_RAW; x_disk->backend = LIBXL_DISK_BACKEND_TAP; - } else if (STREQ(l_disk->driverName, "phy")) { - if (l_disk->format != VIR_STORAGE_FILE_NONE && - l_disk->format != VIR_STORAGE_FILE_RAW) { + } else if (STREQ(driver, "phy")) { + if (format != VIR_STORAGE_FILE_NONE && + format != VIR_STORAGE_FILE_RAW) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight does not support disk format %s " "with disk driver %s"), - virStorageFileFormatTypeToString(l_disk->format), - l_disk->driverName); + virStorageFileFormatTypeToString(format), + driver); return -1; } x_disk->format = LIBXL_DISK_FORMAT_RAW; @@ -806,7 +810,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight does not support disk driver %s"), - l_disk->driverName); + driver); return -1; } } else { diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a6ba10a..6e44567 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3255,11 +3255,9 @@ libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, goto cleanup; } - VIR_FREE(origdisk->src); - origdisk->src = disk->src; - disk->src = NULL; - origdisk->type = disk->type; - + if (virDomainDiskSetSource(origdisk, virDomainDiskGetSource(disk)) < 0) + goto cleanup; + virDomainDiskSetType(origdisk, virDomainDiskGetType(disk)); virDomainDiskDefFree(disk); @@ -3289,7 +3287,7 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv, goto cleanup; } - if (!l_disk->src) { + if (!virDomainDiskGetSource(l_disk)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("disk source path is missing")); goto cleanup; @@ -3779,16 +3777,12 @@ libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) goto cleanup; } - VIR_FREE(orig->src); - orig->src = disk->src; - orig->type = disk->type; - if (disk->driverName) { - VIR_FREE(orig->driverName); - orig->driverName = disk->driverName; - disk->driverName = NULL; - } - orig->format = disk->format; - disk->src = NULL; + if (virDomainDiskSetSource(orig, virDomainDiskGetSource(disk)) < 0) + goto cleanup; + virDomainDiskSetType(orig, virDomainDiskGetType(disk)); + virDomainDiskSetFormat(orig, virDomainDiskGetFormat(disk)); + if (virDomainDiskSetDriver(orig, virDomainDiskGetDriver(disk)) < 0) + goto cleanup; break; default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/locking/domain_lock.c (virDomainLockManagerAddDisk): Use accessors. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/locking/domain_lock.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index 643a875..ed37dd9 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -1,7 +1,7 @@ /* * domain_lock.c: Locking for domain lifecycle operations * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -72,12 +72,15 @@ static int virDomainLockManagerAddDisk(virLockManagerPtr lock, virDomainDiskDefPtr disk) { unsigned int diskFlags = 0; - if (!disk->src) + const char *src = virDomainDiskGetSource(disk); + int type = virDomainDiskGetType(disk); + + if (!src) return 0; - if (!(disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK || - disk->type == VIR_DOMAIN_DISK_TYPE_FILE || - disk->type == VIR_DOMAIN_DISK_TYPE_DIR)) + if (!(type == VIR_DOMAIN_DISK_TYPE_BLOCK || + type == VIR_DOMAIN_DISK_TYPE_FILE || + type == VIR_DOMAIN_DISK_TYPE_DIR)) return 0; if (disk->readonly) @@ -85,14 +88,14 @@ static int virDomainLockManagerAddDisk(virLockManagerPtr lock, if (disk->shared) diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED; - VIR_DEBUG("Add disk %s", disk->src); + VIR_DEBUG("Add disk %s", src); if (virLockManagerAddResource(lock, VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, - disk->src, + src, 0, NULL, diskFlags) < 0) { - VIR_DEBUG("Failed add disk %s", disk->src); + VIR_DEBUG("Failed add disk %s", src); return -1; } return 0; -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/lxc/lxc_cgroup.c (virLXCCgroupSetupDeviceACL): Use accessors. * src/lxc/lxc_controller.c (virLXCControllerSetupLoopDeviceDisk) (virLXCControllerSetupNBDDeviceDisk) (virLXCControllerSetupLoopDevices, virLXCControllerSetupDisk): Likewise. * src/lxc/lxc_driver.c (lxcDomainAttachDeviceDiskLive) (lxcDomainDetachDeviceDiskLive): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/lxc/lxc_cgroup.c | 6 ++--- src/lxc/lxc_controller.c | 69 +++++++++++++++++++++++++++--------------------- src/lxc/lxc_driver.c | 27 +++++++++++-------- 3 files changed, 58 insertions(+), 44 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 5a1718d..da5ccf5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_cgroup.c: LXC cgroup helpers @@ -372,11 +372,11 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, VIR_DEBUG("Allowing any disk block devs"); for (i = 0; i < def->ndisks; i++) { - if (def->disks[i]->type != VIR_DOMAIN_DISK_TYPE_BLOCK) + if (!virDomainDiskSourceIsBlockType(def->disks[i])) continue; if (virCgroupAllowDevicePath(cgroup, - def->disks[i]->src, + virDomainDiskGetSource(def->disks[i]), (def->disks[i]->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW) | diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 874e196..6ed13fb 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_controller.c: linux container process controller @@ -382,21 +382,24 @@ static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDefPtr disk) { int lofd; char *loname = NULL; + const char *src = virDomainDiskGetSource(disk); - if ((lofd = virFileLoopDeviceAssociate(disk->src, &loname)) < 0) + if ((lofd = virFileLoopDeviceAssociate(src, &loname)) < 0) return -1; VIR_DEBUG("Changing disk %s to use type=block for dev %s", - disk->src, loname); + src, loname); /* * We now change it into a block device type, so that * the rest of container setup 'just works' */ - disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - VIR_FREE(disk->src); - disk->src = loname; - loname = NULL; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); + if (virDomainDiskSetSource(disk, loname) < 0) { + VIR_FREE(loname); + return -1; + } + VIR_FREE(loname); return lofd; } @@ -435,28 +438,33 @@ static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs) static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk) { char *dev; + const char *src = virDomainDiskGetSource(disk); + int format = virDomainDiskGetFormat(disk); - if (disk->format <= VIR_STORAGE_FILE_NONE) { + if (format <= VIR_STORAGE_FILE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("An explicit disk format must be specified")); return -1; } - if (virFileNBDDeviceAssociate(disk->src, - disk->format, + if (virFileNBDDeviceAssociate(src, + format, disk->readonly, &dev) < 0) return -1; VIR_DEBUG("Changing disk %s to use type=block for dev %s", - disk->src, dev); + src, dev); /* * We now change it into a block device type, so that * the rest of container setup 'just works' */ - disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - VIR_FREE(disk->src); - disk->src = dev; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); + if (virDomainDiskSetSource(disk, dev) < 0) { + VIR_FREE(dev); + return -1; + } + VIR_FREE(dev); return 0; } @@ -519,23 +527,25 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) for (i = 0; i < ctrl->def->ndisks; i++) { virDomainDiskDefPtr disk = ctrl->def->disks[i]; int fd; + const char *driver = virDomainDiskGetDriver(disk); + int format = virDomainDiskGetFormat(disk); - if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) + if (virDomainDiskGetType(disk) != VIR_DOMAIN_DISK_TYPE_FILE) continue; /* If no driverName is set, we prefer 'loop' for * dealing with raw or undefined formats, otherwise * we use 'nbd'. */ - if (STREQ_NULLABLE(disk->driverName, "loop") || - (!disk->driverName && - (disk->format == VIR_STORAGE_FILE_RAW || - disk->format == VIR_STORAGE_FILE_NONE))) { - if (disk->format != VIR_STORAGE_FILE_RAW && - disk->format != VIR_STORAGE_FILE_NONE) { + if (STREQ_NULLABLE(driver, "loop") || + (!driver && + (format == VIR_STORAGE_FILE_RAW || + format == VIR_STORAGE_FILE_NONE))) { + if (format != VIR_STORAGE_FILE_RAW && + format != VIR_STORAGE_FILE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk format %s is not supported"), - virStorageFileFormatTypeToString(disk->format)); + virStorageFileFormatTypeToString(format)); goto cleanup; } @@ -553,8 +563,7 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) goto cleanup; } ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd; - } else if (STREQ_NULLABLE(disk->driverName, "nbd") || - !disk->driverName) { + } else if (!driver || STREQ(driver, "nbd")) { if (disk->cachemode != VIR_DOMAIN_DISK_CACHE_DEFAULT && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -567,7 +576,7 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk driver %s is not supported"), - disk->driverName); + driver); goto cleanup; } } @@ -1662,12 +1671,12 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, mode_t mode; char *tmpsrc = def->src; - if (def->type != VIR_DOMAIN_DISK_TYPE_BLOCK) { + if (virDomainDiskGetType(def) != VIR_DOMAIN_DISK_TYPE_BLOCK) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't setup disk for non-block device")); goto cleanup; } - if (def->src == NULL) { + if (!tmpsrc) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't setup disk without media")); goto cleanup; @@ -1679,14 +1688,14 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, if (stat(def->src, &sb) < 0) { virReportSystemError(errno, - _("Unable to access %s"), def->src); + _("Unable to access %s"), tmpsrc); goto cleanup; } if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Disk source %s must be a character/block device"), - def->src); + tmpsrc); goto cleanup; } @@ -1704,7 +1713,7 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, * to that normally implied by the device name */ VIR_DEBUG("Creating dev %s (%d,%d) from %s", - dst, major(sb.st_rdev), minor(sb.st_rdev), def->src); + dst, major(sb.st_rdev), minor(sb.st_rdev), tmpsrc); if (mknod(dst, mode, sb.st_rdev) < 0) { virReportSystemError(errno, _("Unable to create device %s"), diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1ae04c5..5546861 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4013,6 +4013,7 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, struct stat sb; char *file = NULL; int perms; + const char *src = NULL; if (!priv->initpid) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -4026,12 +4027,13 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, goto cleanup; } - if (def->type != VIR_DOMAIN_DISK_TYPE_BLOCK) { + if (!virDomainDiskSourceIsBlockType(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't setup disk for non-block device")); goto cleanup; } - if (def->src == NULL) { + src = virDomainDiskGetSource(def); + if (src == NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't setup disk without media")); goto cleanup; @@ -4043,16 +4045,16 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, goto cleanup; } - if (stat(def->src, &sb) < 0) { + if (stat(src, &sb) < 0) { virReportSystemError(errno, - _("Unable to access %s"), def->src); + _("Unable to access %s"), src); goto cleanup; } if (!S_ISBLK(sb.st_mode)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Disk source %s must be a block device"), - def->src); + src); goto cleanup; } @@ -4093,7 +4095,7 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, minor(sb.st_rdev), perms) < 0) VIR_WARN("cannot deny device %s for domain %s", - def->src, vm->def->name); + src, vm->def->name); goto cleanup; } @@ -4102,7 +4104,8 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, ret = 0; cleanup: - virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0); + if (src) + virDomainAuditDisk(vm, NULL, src, "attach", ret == 0); VIR_FREE(file); return ret; } @@ -4576,6 +4579,7 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDiskDefPtr def = NULL; int idx, ret = -1; char *dst = NULL; + const char *src; if (!priv->initpid) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -4592,6 +4596,7 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm, } def = vm->def->disks[idx]; + src = virDomainDiskGetSource(def); if (virAsprintf(&dst, "/dev/%s", def->dst) < 0) goto cleanup; @@ -4603,14 +4608,14 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm, } if (lxcDomainAttachDeviceUnlink(vm, dst) < 0) { - virDomainAuditDisk(vm, def->src, NULL, "detach", false); + virDomainAuditDisk(vm, src, NULL, "detach", false); goto cleanup; } - virDomainAuditDisk(vm, def->src, NULL, "detach", true); + virDomainAuditDisk(vm, src, NULL, "detach", true); - if (virCgroupDenyDevicePath(priv->cgroup, def->src, VIR_CGROUP_DEVICE_RWM) != 0) + if (virCgroupDenyDevicePath(priv->cgroup, src, VIR_CGROUP_DEVICE_RWM) != 0) VIR_WARN("cannot deny device %s for domain %s", - def->src, vm->def->name); + src, vm->def->name); virDomainDiskRemove(vm->def, idx); virDomainDiskDefFree(def); -- 1.8.5.3

On 03/19/2014 11:20 AM, Eric Blake wrote:
Part of a series of cleanups to use new accessor methods.
* src/lxc/lxc_cgroup.c (virLXCCgroupSetupDeviceACL): Use accessors. * src/lxc/lxc_controller.c (virLXCControllerSetupLoopDeviceDisk) (virLXCControllerSetupNBDDeviceDisk) (virLXCControllerSetupLoopDevices, virLXCControllerSetupDisk): Likewise. * src/lxc/lxc_driver.c (lxcDomainAttachDeviceDiskLive) (lxcDomainDetachDeviceDiskLive): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/lxc/lxc_cgroup.c | 6 ++--- src/lxc/lxc_controller.c | 69 +++++++++++++++++++++++++++--------------------- src/lxc/lxc_driver.c | 27 +++++++++++-------- 3 files changed, 58 insertions(+), 44 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 5a1718d..da5ccf5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c
@@ -1662,12 +1671,12 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, mode_t mode; char *tmpsrc = def->src;
Why didn't you use the accessor here?

On 03/21/2014 01:10 PM, Laine Stump wrote:
On 03/19/2014 11:20 AM, Eric Blake wrote:
Part of a series of cleanups to use new accessor methods.
* src/lxc/lxc_cgroup.c (virLXCCgroupSetupDeviceACL): Use accessors. * src/lxc/lxc_controller.c (virLXCControllerSetupLoopDeviceDisk) (virLXCControllerSetupNBDDeviceDisk) (virLXCControllerSetupLoopDevices, virLXCControllerSetupDisk): Likewise. * src/lxc/lxc_driver.c (lxcDomainAttachDeviceDiskLive) (lxcDomainDetachDeviceDiskLive): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/lxc/lxc_cgroup.c | 6 ++--- src/lxc/lxc_controller.c | 69 +++++++++++++++++++++++++++--------------------- src/lxc/lxc_driver.c | 27 +++++++++++-------- 3 files changed, 58 insertions(+), 44 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 5a1718d..da5ccf5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c
@@ -1662,12 +1671,12 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, mode_t mode; char *tmpsrc = def->src;
Why didn't you use the accessor here?
This is one of the files that will be further impacted by later patches to security labeling. The direct use of def->src here was involved in an in-place swap, then a call to relabel the new file, then another in-place swap back to the original name, all as a hack to work around the fact that the security manager requires an entire disk object before doing a label: /* Labelling normally operates on src, but we need * to actually label the dst here, so hack the config */ def->src.path = dst; if (virSecurityManagerSetImageLabel(securityDriver, ctrl->def, def) < 0) goto cleanup; ret = 0; cleanup: def->src.path = tmpsrc; My plans are to improve the security code to add an interface to label just a virDomainDiskSourceDef, where the code here and in qemu_driver that currently does the hack of an in-place swap can instead just create a new DiskSource object that gets labeled directly. And since that cleanup will get rid of the need to do the swap, converting this usage to the accessor would just be churn. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Part of a series of cleanups to use new accessor methods. * src/parallels/parallels_driver.c (parallelsGetHddInfo) (parallelsAddHdd, parallelsApplyDisksParams, parallelsCreateVm): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/parallels/parallels_driver.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 626a1e2..0c985dd 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -2,6 +2,7 @@ * parallels_driver.c: core driver functions for managing * Parallels Cloud Server hosts * + * Copyright (C) 2014 Red Hat, Inc. * Copyright (C) 2012 Parallels, Inc. * * This library is free software; you can redistribute it and/or @@ -301,24 +302,24 @@ parallelsGetHddInfo(virDomainDefPtr def, disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; if (virJSONValueObjectHasKey(value, "real") == 1) { - disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); if (!(tmp = virJSONValueObjectGetString(value, "real"))) { parallelsParseError(); return -1; } - if (VIR_STRDUP(disk->src, tmp) < 0) + if (virDomainDiskSetSource(disk, tmp) < 0) return -1; } else { - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); if (!(tmp = virJSONValueObjectGetString(value, "image"))) { parallelsParseError(); return -1; } - if (VIR_STRDUP(disk->src, tmp) < 0) + if (virDomainDiskSetSource(disk, tmp) < 0) return -1; } @@ -1647,10 +1648,11 @@ static int parallelsAddHdd(virConnectPtr conn, virStoragePoolObjPtr pool = NULL; virStorageVolPtr vol = NULL; int ret = -1; + const char *src = virDomainDiskGetSource(disk); - if (!(vol = parallelsStorageVolLookupByPathLocked(conn, disk->src))) { + if (!(vol = parallelsStorageVolLookupByPathLocked(conn, src))) { virReportError(VIR_ERR_INVALID_ARG, - _("Can't find volume with path '%s'"), disk->src); + _("Can't find volume with path '%s'"), src); return -1; } @@ -1662,11 +1664,11 @@ static int parallelsAddHdd(virConnectPtr conn, goto cleanup; } - voldef = virStorageVolDefFindByPath(pool, disk->src); + voldef = virStorageVolDefFindByPath(pool, src); if (!voldef) { virReportError(VIR_ERR_INVALID_ARG, _("Can't find storage volume definition for path '%s'"), - disk->src); + src); goto cleanup; } @@ -1725,7 +1727,8 @@ parallelsApplyDisksParams(virConnectPtr conn, parallelsDomObjPtr pdom, if (olddisk->bus != newdisk->bus || olddisk->info.addr.drive.target != newdisk->info.addr.drive.target || - !STREQ_NULLABLE(olddisk->src, newdisk->src)) { + !STREQ_NULLABLE(virDomainDiskGetSource(olddisk), + virDomainDiskGetSource(newdisk))) { char prlname[16]; char strpos[16]; @@ -2201,16 +2204,18 @@ parallelsCreateVm(virConnectPtr conn, virDomainDefPtr def) virStoragePoolObjPtr pool = NULL; virStorageVolPtr vol = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; + const char *src; for (i = 0; i < def->ndisks; i++) { if (def->disks[i]->device != VIR_DOMAIN_DISK_DEVICE_DISK) continue; - vol = parallelsStorageVolLookupByPathLocked(conn, def->disks[i]->src); + src = virDomainDiskGetSource(def->disks[i]); + vol = parallelsStorageVolLookupByPathLocked(conn, src); if (!vol) { virReportError(VIR_ERR_INVALID_ARG, _("Can't find volume with path '%s'"), - def->disks[i]->src); + src); return -1; } break; @@ -2234,11 +2239,11 @@ parallelsCreateVm(virConnectPtr conn, virDomainDefPtr def) goto error; } - privvol = virStorageVolDefFindByPath(pool, def->disks[i]->src); + privvol = virStorageVolDefFindByPath(pool, src); if (!privvol) { virReportError(VIR_ERR_INVALID_ARG, _("Can't find storage volume definition for path '%s'"), - def->disks[i]->src); + src); goto error2; } -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/phyp/phyp_driver.c (phypDomainAttachDevice, phypBuildLpar): Use accessors. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/phyp/phyp_driver.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index d97b39f..47a317e 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright IBM Corp. 2009 * * phyp_driver.c: ssh layer to access Power Hypervisors @@ -1709,7 +1709,7 @@ phypDomainAttachDevice(virDomainPtr domain, const char *xml) managed_system, vios_id); virBufferAsprintf(&buf, "mkvdev -vdev %s -vadapter %s", - dev->data.disk->src, scsi_adapter); + virDomainDiskGetSource(dev->data.disk), scsi_adapter); if (system_type == HMC) virBufferAddChar(&buf, '\''); @@ -1730,7 +1730,7 @@ phypDomainAttachDevice(virDomainPtr domain, const char *xml) virBufferAsprintf(&buf, " -m %s", managed_system); virBufferAsprintf(&buf, " slot_num,backing_device|grep %s|cut -d, -f1", - dev->data.disk->src); + virDomainDiskGetSource(dev->data.disk)); if (phypExecInt(session, &buf, conn, &slot) < 0) goto cleanup; @@ -3488,7 +3488,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) goto cleanup; } - if (!def->disks[0]->src) { + if (!virDomainDiskGetSource(def->disks[0])) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Field <src> under <disk> on the domain XML file is " "missing.")); @@ -3502,7 +3502,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) "max_mem=%lld,desired_procs=%d,virtual_scsi_adapters=%s", def->name, def->mem.cur_balloon, def->mem.cur_balloon, def->mem.max_balloon, - (int) def->vcpus, def->disks[0]->src); + (int) def->vcpus, virDomainDiskGetSource(def->disks[0])); ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/qemu/qemu_conf.c (qemuCheckSharedDevice) (qemuAddSharedDevice, qemuRemoveSharedDevice, qemuSetUnprivSGIO): Use accessors. * src/qemu/qemu_domain.c (qemuDomainDeviceDefPostParse) (qemuDomainObjCheckDiskTaint, qemuDomainSnapshotForEachQcow2Raw) (qemuDomainCheckRemoveOptionalDisk, qemuDomainCheckDiskPresence) (qemuDiskChainCheckBroken, qemuDomainDetermineDiskChain): Likewise. * src/qemu/qemu_hotplug.c (qemuDomainChangeEjectableMedia) (qemuDomainCheckEjectableMedia) (qemuDomainAttachVirtioDiskDevice, qemuDomainAttachSCSIDisk) (qemuDomainAttachUSBMassstorageDevice) (qemuDomainAttachDeviceDiskLive, qemuDomainRemoveDiskDevice) (qemuDomainDetachVirtioDiskDevice, qemuDomainDetachDiskDevice): Likewise. * src/qemu/qemu_migration.c (qemuMigrationStartNBDServer) (qemuMigrationDriveMirror, qemuMigrationCancelDriveMirror) (qemuMigrationIsSafe): Likewise. * src/qemu/qemu_process.c (qemuProcessGetVolumeQcowPassphrase) (qemuProcessHandleIOError, qemuProcessHandleBlockJob) (qemuProcessInitPasswords): Likewise. * src/qemu/qemu_driver.c (qemuDomainChangeDiskMediaLive) (qemuDomainGetBlockInfo, qemuDiskPathToAlias): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_conf.c | 22 +++++---- src/qemu/qemu_domain.c | 60 +++++++++++++----------- src/qemu/qemu_driver.c | 18 +++---- src/qemu/qemu_hotplug.c | 116 ++++++++++++++++++++++++++-------------------- src/qemu/qemu_migration.c | 17 +++---- src/qemu/qemu_process.c | 11 +++-- 6 files changed, 135 insertions(+), 109 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 91c5e3a..d280c2c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1,7 +1,7 @@ /* * qemu_conf.c: QEMU configuration management * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -744,6 +744,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, char *key = NULL; int val; int ret = 0; + const char *src; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; @@ -757,7 +758,8 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, return 0; } - if (!(sysfs_path = virGetUnprivSGIOSysfsPath(disk->src, NULL))) { + src = virDomainDiskGetSource(disk); + if (!(sysfs_path = virGetUnprivSGIOSysfsPath(src, NULL))) { ret = -1; goto cleanup; } @@ -768,7 +770,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, if (!virFileExists(sysfs_path)) goto cleanup; - if (!(key = qemuGetSharedDeviceKey(disk->src))) { + if (!(key = qemuGetSharedDeviceKey(src))) { ret = -1; goto cleanup; } @@ -779,7 +781,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, if (!(virHashLookup(sharedDevices, key))) goto cleanup; - if (virGetDeviceUnprivSGIO(disk->src, NULL, &val) < 0) { + if (virGetDeviceUnprivSGIO(src, NULL, &val) < 0) { ret = -1; goto cleanup; } @@ -791,7 +793,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED)) goto cleanup; - if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { + if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_VOLUME) { virReportError(VIR_ERR_OPERATION_INVALID, _("sgio of shared disk 'pool=%s' 'volume=%s' conflicts " "with other active domains"), @@ -800,7 +802,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, } else { virReportError(VIR_ERR_OPERATION_INVALID, _("sgio of shared disk '%s' conflicts with other " - "active domains"), disk->src); + "active domains"), src); } ret = -1; @@ -917,7 +919,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, goto cleanup; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - if (!(key = qemuGetSharedDeviceKey(disk->src))) + if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) goto cleanup; } else { if (!(dev_name = virSCSIDeviceGetDevName(NULL, @@ -1022,7 +1024,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, qemuDriverLock(driver); if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - if (!(key = qemuGetSharedDeviceKey(disk->src))) + if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) goto cleanup; } else { if (!(dev_name = virSCSIDeviceGetDevName(NULL, @@ -1079,7 +1081,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) virDomainDiskDefPtr disk = NULL; virDomainHostdevDefPtr hostdev = NULL; char *sysfs_path = NULL; - char *path = NULL; + const char *path = NULL; int val = -1; int ret = 0; @@ -1093,7 +1095,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) !virDomainDiskSourceIsBlockType(disk)) return 0; - path = disk->src; + path = virDomainDiskGetSource(disk); } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7d375e5..d417387 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -872,10 +872,10 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, /* assign default storage format and driver according to config */ if (cfg->allowDiskFormatProbing) { /* default disk format for drives */ - if (disk->format == VIR_STORAGE_FILE_NONE && - (disk->type == VIR_DOMAIN_DISK_TYPE_FILE || - disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK)) - disk->format = VIR_STORAGE_FILE_AUTO; + if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE && + (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_FILE || + virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_BLOCK)) + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_AUTO); /* default disk format for mirrored drive */ if (disk->mirror && @@ -883,15 +883,15 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, disk->mirrorFormat = VIR_STORAGE_FILE_AUTO; } else { /* default driver if probing is forbidden */ - if (!disk->driverName && - VIR_STRDUP(disk->driverName, "qemu") < 0) + if (!virDomainDiskGetDriver(disk) && + virDomainDiskSetDriver(disk, "qemu") < 0) goto cleanup; /* default disk format for drives */ - if (disk->format == VIR_STORAGE_FILE_NONE && - (disk->type == VIR_DOMAIN_DISK_TYPE_FILE || - disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK)) - disk->format = VIR_STORAGE_FILE_RAW; + if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE && + (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_FILE || + virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_BLOCK)) + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); /* default disk format for mirrored drive */ if (disk->mirror && @@ -1697,8 +1697,9 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, int logFD) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int format = virDomainDiskGetFormat(disk); - if ((!disk->format || disk->format == VIR_STORAGE_FILE_AUTO) && + if ((!format || format == VIR_STORAGE_FILE_AUTO) && cfg->allowDiskFormatProbing) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_DISK_PROBING, logFD); @@ -1939,8 +1940,9 @@ qemuDomainSnapshotForEachQcow2Raw(virQEMUDriverPtr driver, for (i = 0; i < ndisks; i++) { /* FIXME: we also need to handle LVM here */ if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (def->disks[i]->format > 0 && - def->disks[i]->format != VIR_STORAGE_FILE_QCOW2) { + int format = virDomainDiskGetFormat(def->disks[i]); + + if (format > 0 && format != VIR_STORAGE_FILE_QCOW2) { if (try_all) { /* Continue on even in the face of error, since other * disks in this VM may have the same snapshot name. @@ -1962,7 +1964,7 @@ qemuDomainSnapshotForEachQcow2Raw(virQEMUDriverPtr driver, return -1; } - qemuimgarg[4] = def->disks[i]->src; + qemuimgarg[4] = virDomainDiskGetSource(def->disks[i]); if (virRun(qemuimgarg, NULL) < 0) { if (try_all) { @@ -2160,28 +2162,29 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, char uuid[VIR_UUID_STRING_BUFLEN]; virObjectEventPtr event = NULL; virDomainDiskDefPtr del_disk = NULL; + const char *src = virDomainDiskGetSource(disk); virUUIDFormat(vm->def->uuid, uuid); VIR_DEBUG("Dropping disk '%s' on domain '%s' (UUID '%s') " "due to inaccessible source '%s'", - disk->dst, vm->def->name, uuid, disk->src); + disk->dst, vm->def->name, uuid, src); if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM || disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL, + event = virDomainEventDiskChangeNewFromObj(vm, src, NULL, disk->info.alias, VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); - VIR_FREE(disk->src); + ignore_value(virDomainDiskSetSource(disk, NULL)); } else { - event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL, + event = virDomainEventDiskChangeNewFromObj(vm, src, NULL, disk->info.alias, VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START); - if (!(del_disk = virDomainDiskRemoveByName(vm->def, disk->src))) { + if (!(del_disk = virDomainDiskRemoveByName(vm->def, src))) { virReportError(VIR_ERR_INVALID_ARG, - _("no source device %s"), disk->src); + _("no source device %s"), src); return -1; } virDomainDiskDefFree(del_disk); @@ -2244,7 +2247,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, for (i = vm->def->ndisks; i > 0; i--) { disk = vm->def->disks[i - 1]; - if (!disk->src) + if (!virDomainDiskGetSource(disk)) continue; if (qemuDomainDetermineDiskChain(driver, vm, disk, false) >= 0 && @@ -2339,7 +2342,7 @@ qemuDiskChainCheckBroken(virDomainDiskDefPtr disk) { char *brokenFile = NULL; - if (!disk->src || !disk->backingChain) + if (!virDomainDiskGetSource(disk) || !disk->backingChain) return 0; if (virStorageFileChainGetBroken(disk->backingChain, &brokenFile) < 0) @@ -2348,7 +2351,7 @@ qemuDiskChainCheckBroken(virDomainDiskDefPtr disk) if (brokenFile) { virReportError(VIR_ERR_INVALID_ARG, _("Backing file '%s' of image '%s' is missing."), - brokenFile, disk->src); + brokenFile, virDomainDiskGetSource(disk)); VIR_FREE(brokenFile); return -1; } @@ -2396,10 +2399,12 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, int ret = 0; uid_t uid; gid_t gid; + const char *src = virDomainDiskGetSource(disk); + int type = virDomainDiskGetType(disk); - if (!disk->src || - disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK || - disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) + if (!src || + type == VIR_DOMAIN_DISK_TYPE_NETWORK || + type == VIR_DOMAIN_DISK_TYPE_VOLUME) goto cleanup; if (disk->backingChain) { @@ -2413,7 +2418,8 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, qemuDomainGetImageIds(cfg, vm, disk, &uid, &gid); - disk->backingChain = virStorageFileGetMetadata(disk->src, disk->format, + disk->backingChain = virStorageFileGetMetadata(src, + virDomainDiskGetFormat(disk), uid, gid, cfg->allowDiskFormatProbing); if (!disk->backingChain) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2707bec..3b82c5a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6520,7 +6520,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, if (ret != 0 && qemuTeardownDiskCgroup(vm, disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(disk->src)); + NULLSTR(virDomainDiskGetSource(disk))); end: virObjectUnref(caps); @@ -10265,13 +10265,13 @@ qemuDomainGetBlockInfo(virDomainPtr dom, goto cleanup; } disk = vm->def->disks[idx]; - if (!disk->src) { + path = virDomainDiskGetSource(disk); + if (!path) { virReportError(VIR_ERR_INVALID_ARG, _("disk %s does not currently have a source assigned"), path); goto cleanup; } - path = disk->src; /* The path is correct, now try to open it and get its size. */ fd = qemuOpenFile(driver, vm, path, O_RDONLY, NULL, NULL); @@ -10279,18 +10279,18 @@ qemuDomainGetBlockInfo(virDomainPtr dom, goto cleanup; /* Probe for magic formats */ - if (disk->format) { - format = disk->format; + if (virDomainDiskGetFormat(disk)) { + format = virDomainDiskGetFormat(disk); } else { if (cfg->allowDiskFormatProbing) { - if ((format = virStorageFileProbeFormat(disk->src, + if ((format = virStorageFileProbeFormat(path, cfg->user, cfg->group)) < 0) goto cleanup; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("no disk format for %s and probing is disabled"), - disk->src); + path); goto cleanup; } } @@ -10341,7 +10341,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, /* ..but if guest is not using raw disk format and on a block device, * then query highest allocated extent from QEMU */ - if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK && + if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_BLOCK && format != VIR_STORAGE_FILE_RAW && S_ISBLK(sb.st_mode)) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -14620,7 +14620,7 @@ qemuDiskPathToAlias(virDomainObjPtr vm, const char *path, int *idxret) if (idxret) *idxret = idx; - if (disk->src) { + if (virDomainDiskGetSource(disk)) { if (virAsprintf(&ret, "drive-%s", disk->info.alias) < 0) return NULL; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index bf4f160..b948f2c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -71,6 +71,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; int retries = CHANGE_MEDIA_RETRIES; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *src = NULL; if (!origdisk->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -93,7 +94,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def, disk) < 0) { if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", + virDomainDiskGetSource(disk)); goto cleanup; } @@ -128,41 +130,49 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } ret = 0; - if (disk->src) { + src = virDomainDiskGetSource(disk); + if (src) { /* deliberately don't depend on 'ret' as 'eject' may have failed the * first time and we are going to check the drive state anyway */ const char *format = NULL; + int type = virDomainDiskGetType(disk); + int diskFormat = virDomainDiskGetFormat(disk); - if (disk->type != VIR_DOMAIN_DISK_TYPE_DIR) { - if (disk->format > 0) - format = virStorageFileFormatTypeToString(disk->format); - else if (origdisk->format > 0) - format = virStorageFileFormatTypeToString(origdisk->format); + if (type != VIR_DOMAIN_DISK_TYPE_DIR) { + if (diskFormat > 0) { + format = virStorageFileFormatTypeToString(diskFormat); + } else { + diskFormat = virDomainDiskGetFormat(origdisk); + if (diskFormat > 0) + format = virStorageFileFormatTypeToString(diskFormat); + } } qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorChangeMedia(priv->mon, driveAlias, - disk->src, format); + src, format); qemuDomainObjExitMonitor(driver, vm); } audit: - virDomainAuditDisk(vm, origdisk->src, disk->src, "update", ret >= 0); + if (src) + virDomainAuditDisk(vm, virDomainDiskGetSource(origdisk), + src, "update", ret >= 0); if (ret < 0) goto error; if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, origdisk) < 0) - VIR_WARN("Unable to restore security label on ejected image %s", origdisk->src); + VIR_WARN("Unable to restore security label on ejected image %s", + virDomainDiskGetSource(origdisk)); if (virDomainLockDiskDetach(driver->lockManager, vm, origdisk) < 0) - VIR_WARN("Unable to release lock on disk %s", origdisk->src); - - VIR_FREE(origdisk->src); - origdisk->src = disk->src; - disk->src = NULL; - origdisk->type = disk->type; + VIR_WARN("Unable to release lock on disk %s", + virDomainDiskGetSource(origdisk)); + if (virDomainDiskSetSource(origdisk, src) < 0) + goto error; + virDomainDiskSetType(origdisk, virDomainDiskGetType(disk)); virDomainDiskDefFree(disk); @@ -174,10 +184,10 @@ cleanup: error: if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) - VIR_WARN("Unable to restore security label on new media %s", disk->src); + VIR_WARN("Unable to restore security label on new media %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", src); goto cleanup; } @@ -213,8 +223,8 @@ qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver, if (!info) goto cleanup; - if (info->tray_open && disk->src) - VIR_FREE(disk->src); + if (info->tray_open && virDomainDiskGetSource(disk)) + ignore_value(virDomainDiskSetSource(disk, NULL)); } ret = 0; @@ -238,6 +248,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, char *drivestr = NULL; bool releaseaddr = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *src = virDomainDiskGetSource(disk); if (!disk->info.type) { if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && @@ -262,7 +273,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def, disk) < 0) { if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", src); goto cleanup; } @@ -311,10 +322,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } else if (!disk->info.type || disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { virDevicePCIAddress guestAddr = disk->info.addr.pci; - ret = qemuMonitorAddPCIDisk(priv->mon, - disk->src, - type, - &guestAddr); + ret = qemuMonitorAddPCIDisk(priv->mon, src, type, &guestAddr); if (ret == 0) { disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr)); @@ -322,7 +330,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } qemuDomainObjExitMonitor(driver, vm); - virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); + virDomainAuditDisk(vm, NULL, src, "attach", ret >= 0); if (ret < 0) goto error; @@ -337,14 +345,14 @@ cleanup: error: if (releaseaddr) - qemuDomainReleaseDeviceAddress(vm, &disk->info, disk->src); + qemuDomainReleaseDeviceAddress(vm, &disk->info, src); if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) - VIR_WARN("Unable to restore security label on %s", disk->src); + VIR_WARN("Unable to restore security label on %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", src); goto cleanup; } @@ -487,6 +495,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, char *devstr = NULL; int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *src = virDomainDiskGetSource(disk); for (i = 0; i < vm->def->ndisks; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -503,7 +512,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def, disk) < 0) { if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", src); goto cleanup; } @@ -574,7 +583,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, } qemuDomainObjExitMonitor(driver, vm); - virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); + virDomainAuditDisk(vm, NULL, src, "attach", ret >= 0); if (ret < 0) goto error; @@ -590,10 +599,10 @@ cleanup: error: if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) - VIR_WARN("Unable to restore security label on %s", disk->src); + VIR_WARN("Unable to restore security label on %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", src); goto cleanup; } @@ -611,6 +620,7 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn, char *drivestr = NULL; char *devstr = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *src = virDomainDiskGetSource(disk); for (i = 0; i < vm->def->ndisks; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -627,12 +637,12 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn, if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def, disk) < 0) { if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", src); goto cleanup; } /* XXX not correct once we allow attaching a USB CDROM */ - if (!disk->src) { + if (!src) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("disk source path is missing")); goto error; @@ -663,11 +673,11 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn, } } } else { - ret = qemuMonitorAddUSBDisk(priv->mon, disk->src); + ret = qemuMonitorAddUSBDisk(priv->mon, src); } qemuDomainObjExitMonitor(driver, vm); - virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0); + virDomainAuditDisk(vm, NULL, src, "attach", ret >= 0); if (ret < 0) goto error; @@ -683,10 +693,10 @@ cleanup: error: if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) - VIR_WARN("Unable to restore security label on %s", disk->src); + VIR_WARN("Unable to restore security label on %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", src); goto cleanup; } @@ -704,11 +714,13 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, virDomainDiskDefPtr tmp = NULL; virCapsPtr caps = NULL; int ret = -1; + const char *driverName = virDomainDiskGetDriver(disk); + const char *src = virDomainDiskGetSource(disk); - if (disk->driverName != NULL && !STREQ(disk->driverName, "qemu")) { + if (driverName && !STREQ(driverName, "qemu")) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported driver name '%s' for disk '%s'"), - disk->driverName, disk->src); + driverName, src); goto end; } @@ -794,7 +806,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (ret != 0 && qemuTeardownDiskCgroup(vm, disk) < 0) { VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(disk->src)); + NULLSTR(src)); } end: @@ -2460,11 +2472,12 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainDeviceDef dev; virObjectEventPtr event; size_t i; + const char *src = virDomainDiskGetSource(disk); VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name); - virDomainAuditDisk(vm, disk->src, NULL, "detach", true); + virDomainAuditDisk(vm, src, NULL, "detach", true); event = virDomainEventDeviceRemovedNewFromObj(vm, disk->info.alias); if (event) @@ -2477,17 +2490,17 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } } - qemuDomainReleaseDeviceAddress(vm, &disk->info, disk->src); + qemuDomainReleaseDeviceAddress(vm, &disk->info, src); if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) - VIR_WARN("Unable to restore security label on %s", disk->src); + VIR_WARN("Unable to restore security label on %s", src); if (qemuTeardownDiskCgroup(vm, disk) < 0) - VIR_WARN("Failed to tear down cgroup for disk path %s", disk->src); + VIR_WARN("Failed to tear down cgroup for disk path %s", src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", src); dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = disk; @@ -2858,14 +2871,16 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(driver, vm); - virDomainAuditDisk(vm, detach->src, NULL, "detach", false); + virDomainAuditDisk(vm, virDomainDiskGetSource(detach), + NULL, "detach", false); goto cleanup; } } else { if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { qemuDomainObjExitMonitor(driver, vm); - virDomainAuditDisk(vm, detach->src, NULL, "detach", false); + virDomainAuditDisk(vm, virDomainDiskGetSource(detach), + NULL, "detach", false); goto cleanup; } } @@ -2919,7 +2934,8 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(driver, vm); - virDomainAuditDisk(vm, detach->src, NULL, "detach", false); + virDomainAuditDisk(vm, virDomainDiskGetSource(detach), + NULL, "detach", false); goto cleanup; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 26ac99b..0a243c8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1159,7 +1159,7 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, virDomainDiskDefPtr disk = vm->def->disks[i]; /* skip shared, RO and source-less disks */ - if (disk->shared || disk->readonly || !disk->src) + if (disk->shared || disk->readonly || !virDomainDiskGetSource(disk)) continue; VIR_FREE(diskAlias); @@ -1265,7 +1265,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, virDomainBlockJobInfo info; /* skip shared, RO and source-less disks */ - if (disk->shared || disk->readonly || !disk->src) + if (disk->shared || disk->readonly || !virDomainDiskGetSource(disk)) continue; VIR_FREE(diskAlias); @@ -1351,7 +1351,7 @@ error: virDomainDiskDefPtr disk = vm->def->disks[--lastGood]; /* skip shared, RO disks */ - if (disk->shared || disk->readonly || !disk->src) + if (disk->shared || disk->readonly || !virDomainDiskGetSource(disk)) continue; VIR_FREE(diskAlias); @@ -1414,7 +1414,7 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, virDomainDiskDefPtr disk = vm->def->disks[i]; /* skip shared, RO and source-less disks */ - if (disk->shared || disk->readonly || !disk->src) + if (disk->shared || disk->readonly || !virDomainDiskGetSource(disk)) continue; VIR_FREE(diskAlias); @@ -1523,21 +1523,22 @@ qemuMigrationIsSafe(virDomainDefPtr def) for (i = 0; i < def->ndisks; i++) { virDomainDiskDefPtr disk = def->disks[i]; + const char *src = virDomainDiskGetSource(disk); /* Our code elsewhere guarantees shared disks are either readonly (in * which case cache mode doesn't matter) or used with cache=none */ - if (disk->src && + if (src && !disk->shared && !disk->readonly && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { int rc; - if (disk->type == VIR_DOMAIN_DISK_TYPE_FILE) { - if ((rc = virStorageFileIsSharedFS(disk->src)) < 0) + if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_FILE) { + if ((rc = virStorageFileIsSharedFS(src)) < 0) return false; else if (rc == 0) continue; - if ((rc = virStorageFileIsClusterFS(disk->src)) < 0) + if ((rc = virStorageFileIsClusterFS(src)) < 0) return false; else if (rc == 1) continue; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6c4543f..978e7b4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -448,7 +448,8 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn, enc->secrets[0]->type != VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) { virReportError(VIR_ERR_XML_ERROR, - _("invalid <encryption> for volume %s"), disk->src); + _("invalid <encryption> for volume %s"), + virDomainDiskGetSource(disk)); goto cleanup; } @@ -467,7 +468,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn, VIR_FREE(data); virReportError(VIR_ERR_XML_ERROR, _("format='qcow' passphrase for %s must not contain a " - "'\\0'"), disk->src); + "'\\0'"), virDomainDiskGetSource(disk)); goto cleanup; } @@ -930,7 +931,7 @@ qemuProcessHandleIOError(qemuMonitorPtr mon ATTRIBUTE_UNUSED, disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); if (disk) { - srcPath = disk->src; + srcPath = virDomainDiskGetSource(disk); devAlias = disk->info.alias; } else { srcPath = ""; @@ -987,7 +988,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); if (disk) { - path = disk->src; + path = virDomainDiskGetSource(disk); event = virDomainEventBlockJobNewFromObj(vm, path, type, status); /* XXX If we completed a block pull or commit, then recompute * the cached backing chain to match. Better would be storing @@ -2193,7 +2194,7 @@ qemuProcessInitPasswords(virConnectPtr conn, const char *alias; if (!vm->def->disks[i]->encryption || - !vm->def->disks[i]->src) + !virDomainDiskGetSource(vm->def->disks[i])) continue; if (qemuProcessGetVolumeQcowPassphrase(conn, -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/security/security_dac.c (virSecurityDACSetSecurityImageLabel) (virSecurityDACRestoreSecurityImageLabelInt) (virSecurityDACSetSecurityAllLabel): Use accessors. * src/security/security_selinux.c (virSecuritySELinuxRestoreSecurityImageLabelInt) (virSecuritySELinuxSetSecurityImageLabel) (virSecuritySELinuxSetSecurityAllLabel): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/security/security_dac.c | 17 +++++++++-------- src/security/security_selinux.c | 18 ++++++++++-------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9f45063..0bd36b7 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -355,7 +355,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, if (!priv->dynamicOwnership) return 0; - if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) + if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; params[0] = mgr; @@ -374,11 +374,12 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, int migrated) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + const char *src = virDomainDiskGetSource(disk); if (!priv->dynamicOwnership) return 0; - if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) + if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; /* Don't restore labels on readoly/shared disks, because @@ -392,7 +393,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, if (disk->readonly || disk->shared) return 0; - if (!disk->src) + if (!src) return 0; /* If we have a shared FS & doing migrated, we must not @@ -401,17 +402,17 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, * VM's I/O attempts :-) */ if (migrated) { - int rc = virStorageFileIsSharedFS(disk->src); + int rc = virStorageFileIsSharedFS(src); if (rc < 0) return -1; if (rc == 1) { VIR_DEBUG("Skipping image label restore on %s because FS is shared", - disk->src); + src); return 0; } } - return virSecurityDACRestoreSecurityFileLabel(disk->src); + return virSecurityDACRestoreSecurityFileLabel(src); } @@ -904,7 +905,7 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, for (i = 0; i < def->ndisks; i++) { /* XXX fixme - we need to recursively label the entire tree :-( */ - if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR) + if (virDomainDiskGetType(def->disks[i]) == VIR_DOMAIN_DISK_TYPE_DIR) continue; if (virSecurityDACSetSecurityImageLabel(mgr, def, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 02c7496..32f6c7d 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008-2013 Red Hat, Inc. + * Copyright (C) 2008-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -1133,6 +1133,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, { virSecurityLabelDefPtr seclabel; virSecurityDeviceLabelDefPtr disk_seclabel; + const char *src = virDomainDiskGetSource(disk); seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (seclabel == NULL) @@ -1162,7 +1163,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, if (disk->readonly || disk->shared) return 0; - if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) + if (!src || virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; /* If we have a shared FS & doing migrated, we must not @@ -1171,17 +1172,17 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, * VM's I/O attempts :-) */ if (migrated) { - int rc = virStorageFileIsSharedFS(disk->src); + int rc = virStorageFileIsSharedFS(src); if (rc < 0) return -1; if (rc == 1) { VIR_DEBUG("Skipping image label restore on %s because FS is shared", - disk->src); + src); return 0; } } - return virSecuritySELinuxRestoreSecurityFileLabel(mgr, disk->src); + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, src); } @@ -1262,7 +1263,7 @@ virSecuritySELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, if (cbdata.secdef->norelabel) return 0; - if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) + if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; return virDomainDiskDefForeachPath(disk, @@ -2271,9 +2272,10 @@ virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, for (i = 0; i < def->ndisks; i++) { /* XXX fixme - we need to recursively label the entire tree :-( */ - if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR) { + if (virDomainDiskGetType(def->disks[i]) == VIR_DOMAIN_DISK_TYPE_DIR) { VIR_WARN("Unable to relabel directory tree %s for disk %s", - def->disks[i]->src, def->disks[i]->dst); + virDomainDiskGetSource(def->disks[i]), + def->disks[i]->dst); continue; } if (virSecuritySELinuxSetSecurityImageLabel(mgr, -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/uml/uml_conf.c (umlBuildCommandLine): Use accessors. * src/uml/uml_driver.c (umlDomainAttachUmlDisk): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/uml/uml_conf.c | 4 ++-- src/uml/uml_driver.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 3567b03..27c53f9 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -1,7 +1,7 @@ /* * uml_conf.c: UML driver configuration * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -410,7 +410,7 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArgPair(cmd, disk->dst, disk->src); + virCommandAddArgPair(cmd, disk->dst, virDomainDiskGetSource(disk)); } for (i = 0; i < vm->def->nnets; i++) { diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 28e65f4..a49b56b 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2144,13 +2144,14 @@ static int umlDomainAttachUmlDisk(struct uml_driver *driver, } } - if (!disk->src) { + if (!virDomainDiskGetSource(disk)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("disk source path is missing")); goto error; } - if (virAsprintf(&cmd, "config %s=%s", disk->dst, disk->src) < 0) + if (virAsprintf(&cmd, "config %s=%s", disk->dst, + virDomainDiskGetSource(disk)) < 0) return -1; if (umlMonitorCommand(driver, vm, cmd, &reply) < 0) -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. * src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc, vboxAttachDrives) (vboxDomainAttachDeviceImpl, vboxDomainDetachDevice): Use accessors. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/vbox/vbox_tmpl.c | 137 +++++++++++++++++++++++++++++---------------------- 1 file changed, 77 insertions(+), 60 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 2aeddd0..9df25c5 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2738,7 +2738,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (VIR_ALLOC(def->disks[i]) >= 0) { def->disks[i]->device = VIR_DOMAIN_DISK_DEVICE_DISK; def->disks[i]->bus = VIR_DOMAIN_DISK_BUS_IDE; - def->disks[i]->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(def->disks[i], + VIR_DOMAIN_DISK_TYPE_FILE); } } } @@ -2755,7 +2756,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (hddType == HardDiskType_Immutable) def->disks[hddNum]->readonly = true; - ignore_value(VIR_STRDUP(def->disks[hddNum]->src, hddlocation)); + ignore_value(virDomainDiskSetSource(def->disks[hddNum], + hddlocation)); ignore_value(VIR_STRDUP(def->disks[hddNum]->dst, "hda")); hddNum++; @@ -2776,7 +2778,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (hddType == HardDiskType_Immutable) def->disks[hddNum]->readonly = true; - ignore_value(VIR_STRDUP(def->disks[hddNum]->src, hddlocation)); + ignore_value(virDomainDiskSetSource(def->disks[hddNum], + hddlocation)); ignore_value(VIR_STRDUP(def->disks[hddNum]->dst, "hdb")); hddNum++; @@ -2797,7 +2800,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (hddType == HardDiskType_Immutable) def->disks[hddNum]->readonly = true; - ignore_value(VIR_STRDUP(def->disks[hddNum]->src, hddlocation)); + ignore_value(virDomainDiskSetSource(def->disks[hddNum], + hddlocation)); ignore_value(VIR_STRDUP(def->disks[hddNum]->dst, "hdd")); hddNum++; @@ -2884,10 +2888,11 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { medium->vtbl->GetLocation(medium, &mediumLocUtf16); VBOX_UTF16_TO_UTF8(mediumLocUtf16, &mediumLocUtf8); VBOX_UTF16_FREE(mediumLocUtf16); - ignore_value(VIR_STRDUP(def->disks[diskCount]->src, mediumLocUtf8)); + ignore_value(virDomainDiskSetSource(def->disks[diskCount], + mediumLocUtf8)); VBOX_UTF8_FREE(mediumLocUtf8); - if (!(def->disks[diskCount]->src)) { + if (!virDomainDiskGetSource(def->disks[diskCount])) { VBOX_RELEASE(medium); VBOX_RELEASE(storageController); error = true; @@ -2936,7 +2941,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (readOnly == PR_TRUE) def->disks[diskCount]->readonly = true; - def->disks[diskCount]->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(def->disks[diskCount], + VIR_DOMAIN_DISK_TYPE_FILE); VBOX_RELEASE(medium); VBOX_RELEASE(storageController); @@ -3211,9 +3217,10 @@ sharedFoldersCleanup: if (VIR_ALLOC(def->disks[def->ndisks - 1]) >= 0) { def->disks[def->ndisks - 1]->device = VIR_DOMAIN_DISK_DEVICE_CDROM; def->disks[def->ndisks - 1]->bus = VIR_DOMAIN_DISK_BUS_IDE; - def->disks[def->ndisks - 1]->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(def->disks[def->ndisks - 1], + VIR_DOMAIN_DISK_TYPE_FILE); def->disks[def->ndisks - 1]->readonly = true; - ignore_value(VIR_STRDUP(def->disks[def->ndisks - 1]->src, location)); + ignore_value(virDomainDiskSetSource(def->disks[def->ndisks - 1], location)); ignore_value(VIR_STRDUP(def->disks[def->ndisks - 1]->dst, "hdc")); def->ndisks--; } else { @@ -3257,9 +3264,10 @@ sharedFoldersCleanup: if (VIR_ALLOC(def->disks[def->ndisks - 1]) >= 0) { def->disks[def->ndisks - 1]->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; def->disks[def->ndisks - 1]->bus = VIR_DOMAIN_DISK_BUS_FDC; - def->disks[def->ndisks - 1]->type = VIR_DOMAIN_DISK_TYPE_FILE; + virDomainDiskSetType(def->disks[def->ndisks - 1], + VIR_DOMAIN_DISK_TYPE_FILE); def->disks[def->ndisks - 1]->readonly = false; - ignore_value(VIR_STRDUP(def->disks[def->ndisks - 1]->src, location)); + ignore_value(virDomainDiskSetSource(def->disks[def->ndisks - 1], location)); ignore_value(VIR_STRDUP(def->disks[def->ndisks - 1]->dst, "fda")); def->ndisks--; } else { @@ -3847,14 +3855,19 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) return; for (i = 0; i < def->ndisks; i++) { - VIR_DEBUG("disk(%zu) type: %d", i, def->disks[i]->type); + const char *src = virDomainDiskGetSource(def->disks[i]); + int type = virDomainDiskGetType(def->disks[i]); + int format = virDomainDiskGetFormat(def->disks[i]); + + VIR_DEBUG("disk(%zu) type: %d", i, type); VIR_DEBUG("disk(%zu) device: %d", i, def->disks[i]->device); VIR_DEBUG("disk(%zu) bus: %d", i, def->disks[i]->bus); - VIR_DEBUG("disk(%zu) src: %s", i, def->disks[i]->src); + VIR_DEBUG("disk(%zu) src: %s", i, src); VIR_DEBUG("disk(%zu) dst: %s", i, def->disks[i]->dst); - VIR_DEBUG("disk(%zu) driverName: %s", i, def->disks[i]->driverName); + VIR_DEBUG("disk(%zu) driverName: %s", i, + virDomainDiskGetDriver(def->disks[i])); VIR_DEBUG("disk(%zu) driverType: %s", i, - virStorageFileFormatTypeToString(def->disks[i]->format)); + virStorageFileFormatTypeToString(format)); VIR_DEBUG("disk(%zu) cachemode: %d", i, def->disks[i]->cachemode); VIR_DEBUG("disk(%zu) readonly: %s", i, (def->disks[i]->readonly ? "True" : "False")); @@ -3862,8 +3875,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) ? "True" : "False")); if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE && - def->disks[i]->src != NULL) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE && src) { IDVDDrive *dvdDrive = NULL; /* Currently CDROM/DVD Drive is always IDE * Secondary Master so neglecting the following @@ -3879,7 +3891,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) vboxIID dvduuid = VBOX_IID_INITIALIZER; vboxIID dvdemptyuuid = VBOX_IID_INITIALIZER; - VBOX_UTF8_TO_UTF16(def->disks[i]->src, &dvdfileUtf16); + VBOX_UTF8_TO_UTF16(src, &dvdfileUtf16); data->vboxObj->vtbl->FindDVDImage(data->vboxObj, dvdfileUtf16, &dvdImage); @@ -3896,13 +3908,13 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) virReportError(VIR_ERR_INTERNAL_ERROR, _("can't get the uuid of the file to " "be attached to cdrom: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); } else { rc = dvdDrive->vtbl->MountImage(dvdDrive, dvduuid.value); if (NS_FAILED(rc)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("could not attach the file to cdrom: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); } else { DEBUGIID("CD/DVDImage UUID:", dvduuid.value); } @@ -3914,11 +3926,10 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VBOX_UTF16_FREE(dvdfileUtf16); VBOX_RELEASE(dvdDrive); } - } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { } } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE && - def->disks[i]->src != NULL) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE && src) { IHardDisk *hardDisk = NULL; PRUnichar *hddfileUtf16 = NULL; vboxIID hdduuid = VBOX_IID_INITIALIZER; @@ -3929,7 +3940,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) * is requested to be connected to Secondary master */ - VBOX_UTF8_TO_UTF16(def->disks[i]->src, &hddfileUtf16); + VBOX_UTF8_TO_UTF16(src, &hddfileUtf16); VBOX_UTF8_TO_UTF16("", &hddEmpty); data->vboxObj->vtbl->FindHardDisk(data->vboxObj, hddfileUtf16, @@ -3960,7 +3971,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) virReportError(VIR_ERR_INTERNAL_ERROR, _("can't get the uuid of the file to be " "attached as harddisk: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); } else { if (def->disks[i]->readonly) { hardDisk->vtbl->SetType(hardDisk, @@ -4007,7 +4018,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) virReportError(VIR_ERR_INTERNAL_ERROR, _("could not attach the file as " "harddisk: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); } else { DEBUGIID("Attached HDD with UUID", hdduuid.value); } @@ -4019,11 +4030,10 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) vboxIIDUnalloc(&hdduuid); VBOX_UTF16_FREE(hddEmpty); VBOX_UTF16_FREE(hddfileUtf16); - } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { } } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE && - def->disks[i]->src != NULL) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE && src) { IFloppyDrive *floppyDrive; machine->vtbl->GetFloppyDrive(machine, &floppyDrive); if (floppyDrive) { @@ -4034,7 +4044,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) vboxIID fduuid = VBOX_IID_INITIALIZER; vboxIID fdemptyuuid = VBOX_IID_INITIALIZER; - VBOX_UTF8_TO_UTF16(def->disks[i]->src, &fdfileUtf16); + VBOX_UTF8_TO_UTF16(src, &fdfileUtf16); rc = data->vboxObj->vtbl->FindFloppyImage(data->vboxObj, fdfileUtf16, &floppyImage); @@ -4053,7 +4063,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) virReportError(VIR_ERR_INTERNAL_ERROR, _("can't get the uuid of the file to " "be attached to floppy drive: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); } else { rc = floppyDrive->vtbl->MountImage(floppyDrive, fduuid.value); @@ -4061,7 +4071,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) virReportError(VIR_ERR_INTERNAL_ERROR, _("could not attach the file to " "floppy drive: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); } else { DEBUGIID("floppyImage UUID", fduuid.value); } @@ -4073,7 +4083,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } VBOX_RELEASE(floppyDrive); } - } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { } } } @@ -4128,22 +4138,26 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } for (i = 0; i < def->ndisks && !error; i++) { - VIR_DEBUG("disk(%zu) type: %d", i, def->disks[i]->type); + const char *src = virDomainDiskGetSource(def->disks[i]); + int type = virDomainDiskGetType(def->disks[i]); + int format = virDomainDiskGetFormat(def->disks[i]); + + VIR_DEBUG("disk(%zu) type: %d", i, type); VIR_DEBUG("disk(%zu) device: %d", i, def->disks[i]->device); VIR_DEBUG("disk(%zu) bus: %d", i, def->disks[i]->bus); - VIR_DEBUG("disk(%zu) src: %s", i, def->disks[i]->src); + VIR_DEBUG("disk(%zu) src: %s", i, src); VIR_DEBUG("disk(%zu) dst: %s", i, def->disks[i]->dst); - VIR_DEBUG("disk(%zu) driverName: %s", i, def->disks[i]->driverName); + VIR_DEBUG("disk(%zu) driverName: %s", i, + virDomainDiskGetDriver(def->disks[i])); VIR_DEBUG("disk(%zu) driverType: %s", i, - virStorageFileFormatTypeToString(def->disks[i]->format)); + virStorageFileFormatTypeToString(format)); VIR_DEBUG("disk(%zu) cachemode: %d", i, def->disks[i]->cachemode); VIR_DEBUG("disk(%zu) readonly: %s", i, (def->disks[i]->readonly ? "True" : "False")); VIR_DEBUG("disk(%zu) shared: %s", i, (def->disks[i]->shared ? "True" : "False")); - if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE && - def->disks[i]->src != NULL) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE && src) { IMedium *medium = NULL; PRUnichar *mediumUUID = NULL; PRUnichar *mediumFileUtf16 = NULL; @@ -4156,7 +4170,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) PRInt32 devicePort = 0; PRInt32 deviceSlot = 0; - VBOX_UTF8_TO_UTF16(def->disks[i]->src, &mediumFileUtf16); + VBOX_UTF8_TO_UTF16(src, &mediumFileUtf16); if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { deviceType = DeviceType_HardDisk; @@ -4245,7 +4259,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to attach the following disk/dvd/floppy " "to the machine: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); VBOX_UTF16_FREE(mediumFileUtf16); continue; } @@ -4255,7 +4269,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) virReportError(VIR_ERR_INTERNAL_ERROR, _("can't get the uuid of the file to be attached " "as harddisk/dvd/floppy: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); VBOX_RELEASE(medium); VBOX_UTF16_FREE(mediumFileUtf16); continue; @@ -4297,7 +4311,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) _("can't get the port/slot number of " "harddisk/dvd/floppy to be attached: " "%s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); VBOX_RELEASE(medium); VBOX_UTF16_FREE(mediumUUID); VBOX_UTF16_FREE(mediumFileUtf16); @@ -4320,7 +4334,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) virReportError(VIR_ERR_INTERNAL_ERROR, _("could not attach the file as " "harddisk/dvd/floppy: %s, rc=%08x"), - def->disks[i]->src, (unsigned)rc); + src, (unsigned)rc); } else { DEBUGIID("Attached HDD/DVD/Floppy with UUID", mediumUUID); } @@ -5480,9 +5494,11 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, if (NS_SUCCEEDED(rc) && machine) { if (dev->type == VIR_DOMAIN_DEVICE_DISK) { #if VBOX_API_VERSION < 3001000 + const char *src = virDomainDiskGetSource(dev->data.disk); + int type = virDomainDiskGetType(dev->data.disk); + if (dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_FILE && - dev->data.disk->src != NULL) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE && src) { IDVDDrive *dvdDrive = NULL; /* Currently CDROM/DVD Drive is always IDE * Secondary Master so neglecting the following @@ -5495,7 +5511,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, vboxIID dvduuid = VBOX_IID_INITIALIZER; vboxIID dvdemptyuuid = VBOX_IID_INITIALIZER; - VBOX_UTF8_TO_UTF16(dev->data.disk->src, &dvdfileUtf16); + VBOX_UTF8_TO_UTF16(src, &dvdfileUtf16); data->vboxObj->vtbl->FindDVDImage(data->vboxObj, dvdfileUtf16, &dvdImage); if (!dvdImage) { @@ -5507,7 +5523,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, virReportError(VIR_ERR_INTERNAL_ERROR, _("can't get the uuid of the file to " "be attached to cdrom: %s, rc=%08x"), - dev->data.disk->src, (unsigned)rc); + src, (unsigned)rc); } else { /* unmount the previous mounted image */ dvdDrive->vtbl->Unmount(dvdDrive); @@ -5515,7 +5531,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, if (NS_FAILED(rc)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("could not attach the file to cdrom: %s, rc=%08x"), - dev->data.disk->src, (unsigned)rc); + src, (unsigned)rc); } else { ret = 0; DEBUGIID("CD/DVD Image UUID:", dvduuid.value); @@ -5528,11 +5544,10 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, VBOX_UTF16_FREE(dvdfileUtf16); VBOX_RELEASE(dvdDrive); } - } else if (dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { } } else if (dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - if (dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_FILE && - dev->data.disk->src != NULL) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE && src) { IFloppyDrive *floppyDrive; machine->vtbl->GetFloppyDrive(machine, &floppyDrive); if (floppyDrive) { @@ -5542,7 +5557,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, PRUnichar *fdfileUtf16 = NULL; vboxIID fduuid = VBOX_IID_INITIALIZER; vboxIID fdemptyuuid = VBOX_IID_INITIALIZER; - VBOX_UTF8_TO_UTF16(dev->data.disk->src, &fdfileUtf16); + VBOX_UTF8_TO_UTF16(src, &fdfileUtf16); rc = data->vboxObj->vtbl->FindFloppyImage(data->vboxObj, fdfileUtf16, &floppyImage); @@ -5560,13 +5575,13 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, virReportError(VIR_ERR_INTERNAL_ERROR, _("can't get the uuid of the file to be " "attached to floppy drive: %s, rc=%08x"), - dev->data.disk->src, (unsigned)rc); + src, (unsigned)rc); } else { rc = floppyDrive->vtbl->MountImage(floppyDrive, fduuid.value); if (NS_FAILED(rc)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("could not attach the file to floppy drive: %s, rc=%08x"), - dev->data.disk->src, (unsigned)rc); + src, (unsigned)rc); } else { ret = 0; DEBUGIID("attached floppy, UUID:", fduuid.value); @@ -5579,7 +5594,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, } VBOX_RELEASE(floppyDrive); } - } else if (dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { } } #else /* VBOX_API_VERSION >= 3001000 */ @@ -5710,8 +5725,10 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml) { if (NS_SUCCEEDED(rc) && machine) { if (dev->type == VIR_DOMAIN_DEVICE_DISK) { #if VBOX_API_VERSION < 3001000 + int type = virDomainDiskGetType(dev->data.disk); + if (dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_FILE) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE) { IDVDDrive *dvdDrive = NULL; /* Currently CDROM/DVD Drive is always IDE * Secondary Master so neglecting the following @@ -5729,10 +5746,10 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml) { } VBOX_RELEASE(dvdDrive); } - } else if (dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { } } else if (dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - if (dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_FILE) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE) { IFloppyDrive *floppyDrive; machine->vtbl->GetFloppyDrive(machine, &floppyDrive); if (floppyDrive) { @@ -5757,7 +5774,7 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml) { } VBOX_RELEASE(floppyDrive); } - } else if (dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { } } #else /* VBOX_API_VERSION >= 3001000 */ -- 1.8.5.3

Part of a series of cleanups to use new accessor methods. While writing this, I also discovered that conversion from XML to vmware modified the disk source in place; if the same code is reached twice, the second call behaves differently because the first call didn't clean up its mess. * src/vmware/vmware_conf.c (vmwareVmxPath): Use accessors. (vmwareParsePath): Avoid munging input string. * src/vmware/vmware_conf.h (vmwareParsePath): Make static. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/vmware/vmware_conf.c | 18 ++++++++++-------- src/vmware/vmware_conf.h | 6 +++--- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index c96bd62..2de24a7 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -331,15 +331,15 @@ vmwareDomainConfigDisplay(vmwareDomainPtr pDomain, virDomainDefPtr def) } } -int -vmwareParsePath(char *path, char **directory, char **filename) +static int +vmwareParsePath(const char *path, char **directory, char **filename) { char *separator; separator = strrchr(path, '/'); if (separator != NULL) { - *separator++ = '\0'; + separator++; if (*separator == '\0') { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -347,7 +347,7 @@ vmwareParsePath(char *path, char **directory, char **filename) return -1; } - if (VIR_STRDUP(*directory, path) < 0) + if (VIR_STRNDUP(*directory, path, separator - path - 1) < 0) goto error; if (VIR_STRDUP(*filename, separator) < 0) { VIR_FREE(*directory); @@ -388,6 +388,7 @@ vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) char *fileName = NULL; int ret = -1; size_t i; + const char *src; /* * Build VMX URL. Use the source of the first file-based harddisk @@ -405,7 +406,7 @@ vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) for (i = 0; i < vmdef->ndisks; ++i) { if (vmdef->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK && - vmdef->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE) { + virDomainDiskGetType(vmdef->disks[i]) == VIR_DOMAIN_DISK_TYPE_FILE) { disk = vmdef->disks[i]; break; } @@ -418,21 +419,22 @@ vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) goto cleanup; } - if (disk->src == NULL) { + src = virDomainDiskGetSource(disk); + if (!src) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("First file-based harddisk has no source, cannot " "deduce datastore and path for VMX file")); goto cleanup; } - if (vmwareParsePath(disk->src, &directoryName, &fileName) < 0) { + if (vmwareParsePath(src, &directoryName, &fileName) < 0) { goto cleanup; } if (!virFileHasSuffix(fileName, ".vmdk")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting source '%s' of first file-based harddisk " - "to be a VMDK image"), disk->src); + "to be a VMDK image"), src); goto cleanup; } diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h index b9fca6c..b039f9e 100644 --- a/src/vmware/vmware_conf.h +++ b/src/vmware/vmware_conf.h @@ -1,5 +1,7 @@ /*---------------------------------------------------------------------------*/ -/* Copyright 2010, diateam (www.diateam.net) +/* + * Copyright (C) 2014, Red Hat, Inc. + * Copyright 2010, diateam (www.diateam.net) * Copyright (c) 2013, Doug Goldstein (cardoe@cardoe.com) * * This library is free software; you can redistribute it and/or @@ -71,8 +73,6 @@ int vmwareParseVersionStr(int type, const char *buf, unsigned long *version); int vmwareDomainConfigDisplay(vmwareDomainPtr domain, virDomainDefPtr vmdef); -int vmwareParsePath(char *path, char **directory, char **filename); - int vmwareConstructVmxPath(char *directoryName, char *name, char **vmxPath); -- 1.8.5.3

On 03/19/2014 11:20 AM, Eric Blake wrote:
Part of a series of cleanups to use new accessor methods.
While writing this, I also discovered that conversion from XML to vmware modified the disk source in place; if the same code is reached twice, the second call behaves differently because the first call didn't clean up its mess.
* src/vmware/vmware_conf.c (vmwareVmxPath): Use accessors. (vmwareParsePath): Avoid munging input string. * src/vmware/vmware_conf.h (vmwareParsePath): Make static.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/vmware/vmware_conf.c | 18 ++++++++++-------- src/vmware/vmware_conf.h | 6 +++--- 2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index c96bd62..2de24a7 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -331,15 +331,15 @@ vmwareDomainConfigDisplay(vmwareDomainPtr pDomain, virDomainDefPtr def) } }
-int -vmwareParsePath(char *path, char **directory, char **filename) +static int +vmwareParsePath(const char *path, char **directory, char **filename) { char *separator;
separator = strrchr(path, '/');
if (separator != NULL) { - *separator++ = '\0'; + separator++;
if (*separator == '\0') { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -347,7 +347,7 @@ vmwareParsePath(char *path, char **directory, char **filename) return -1; }
- if (VIR_STRDUP(*directory, path) < 0) + if (VIR_STRNDUP(*directory, path, separator - path - 1) < 0)
So this fixes the bug you mentioned in the log message. Might be better if it was a different patch. (No need to resend though) (BTW, you haven't seen any ACKs yet because I'm going to give one for the whole series to reduce clutter :-)

On 03/21/2014 03:01 PM, Laine Stump wrote:
On 03/19/2014 11:20 AM, Eric Blake wrote:
Part of a series of cleanups to use new accessor methods.
While writing this, I also discovered that conversion from XML to vmware modified the disk source in place; if the same code is reached twice, the second call behaves differently because the first call didn't clean up its mess.
So this fixes the bug you mentioned in the log message. Might be better if it was a different patch. (No need to resend though) (BTW, you haven't seen any ACKs yet because I'm going to give one for the whole series to reduce clutter :-)
Okay, I can split this patch into two parts when pushing; bug fix first, then accessors second. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

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) + 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) + goto cleanup; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid or not yet handled value '%s' " @@ -2288,7 +2299,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con if (virDomainDiskDefAssignAddress(xmlopt, *def) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not assign address to disk '%s'"), (*def)->src); + _("Could not assign address to disk '%s'"), + virDomainDiskGetSource(*def)); goto cleanup; } @@ -3395,11 +3407,12 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, int controllerOrBus, unit; const char *vmxDeviceType = NULL; char *fileName = NULL; + int type = virDomainDiskGetType(def); /* Convert a handful of types to their string values */ const char *busType = virDomainDiskBusTypeToString(def->bus); const char *deviceType = virDomainDeviceTypeToString(def->device); - const char *diskType = virDomainDeviceTypeToString(def->type); + const char *diskType = virDomainDeviceTypeToString(type); /* If we are dealing with a disk its a .vmdk, otherwise it must be * an ISO. @@ -3417,8 +3430,8 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, } /* We only support type='file' and type='block' */ - if (def->type != VIR_DOMAIN_DISK_TYPE_FILE && - def->type != VIR_DOMAIN_DISK_TYPE_BLOCK) { + if (type != VIR_DOMAIN_DISK_TYPE_FILE && + type != VIR_DOMAIN_DISK_TYPE_BLOCK) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("%s %s '%s' has unsupported type '%s', expecting " "'%s' or '%s'"), busType, deviceType, def->dst, @@ -3446,11 +3459,11 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, } if (def->device == VIR_DOMAIN_DISK_DEVICE_DISK && - def->type == VIR_DOMAIN_DISK_TYPE_FILE) { + type == VIR_DOMAIN_DISK_TYPE_FILE) { vmxDeviceType = (def->bus == VIR_DOMAIN_DISK_BUS_SCSI) ? "scsi-hardDisk" : "ata-hardDisk"; } else if (def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) + if (type == VIR_DOMAIN_DISK_TYPE_FILE) vmxDeviceType = "cdrom-image"; else vmxDeviceType = "atapi-cdrom"; @@ -3468,8 +3481,10 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, virBufferAsprintf(buffer, "%s%d:%d.deviceType = \"%s\"\n", busType, controllerOrBus, unit, vmxDeviceType); - if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) { - if (def->src != NULL && ! virFileHasSuffix(def->src, fileExt)) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE) { + const char *src = virDomainDiskGetSource(def); + + if (src && ! virFileHasSuffix(src, fileExt)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Image file for %s %s '%s' has " "unsupported suffix, expecting '%s'"), @@ -3477,7 +3492,7 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, return -1; } - fileName = ctx->formatFileName(def->src, ctx->opaque); + fileName = ctx->formatFileName(src, ctx->opaque); if (fileName == NULL) { return -1; @@ -3487,8 +3502,10 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, busType, controllerOrBus, unit, fileName); VIR_FREE(fileName); - } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { - if (!def->src && + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + const char *src = virDomainDiskGetSource(def); + + if (!src && def->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { virBufferAsprintf(buffer, "%s%d:%d.autodetect = \"true\"\n", busType, controllerOrBus, unit); @@ -3496,7 +3513,7 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, busType, controllerOrBus, unit); } else { virBufferAsprintf(buffer, "%s%d:%d.fileName = \"%s\"\n", - busType, controllerOrBus, unit, def->src); + busType, controllerOrBus, unit, src); } } @@ -3526,6 +3543,8 @@ virVMXFormatFloppy(virVMXContext *ctx, virDomainDiskDefPtr def, { int unit; char *fileName = NULL; + int type = virDomainDiskGetType(def); + const char *src = virDomainDiskGetSource(def); if (def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -3540,11 +3559,11 @@ virVMXFormatFloppy(virVMXContext *ctx, virDomainDiskDefPtr def, virBufferAsprintf(buffer, "floppy%d.present = \"true\"\n", unit); - if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) { + if (type == VIR_DOMAIN_DISK_TYPE_FILE) { virBufferAsprintf(buffer, "floppy%d.fileType = \"file\"\n", unit); - if (def->src != NULL) { - fileName = ctx->formatFileName(def->src, ctx->opaque); + if (src) { + fileName = ctx->formatFileName(src, ctx->opaque); if (fileName == NULL) { return -1; @@ -3555,18 +3574,18 @@ virVMXFormatFloppy(virVMXContext *ctx, virDomainDiskDefPtr def, VIR_FREE(fileName); } - } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { + } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) { virBufferAsprintf(buffer, "floppy%d.fileType = \"device\"\n", unit); - if (def->src != NULL) { + if (src) { virBufferAsprintf(buffer, "floppy%d.fileName = \"%s\"\n", - unit, def->src); + unit, src); } } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Floppy '%s' has unsupported type '%s', expecting '%s' " "or '%s'"), def->dst, - virDomainDiskTypeToString(def->type), + virDomainDiskTypeToString(type), virDomainDiskTypeToString(VIR_DOMAIN_DISK_TYPE_FILE), virDomainDiskTypeToString(VIR_DOMAIN_DISK_TYPE_BLOCK)); return -1; -- 1.8.5.3

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.

On 03/21/2014 03:10 PM, Laine Stump wrote:
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.
- (*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.
Good catch. Here's what I'm squashing to fix it. diff --git i/src/vmx/vmx.c w/src/vmx/vmx.c index 341d1af..9b5162b 100644 --- i/src/vmx/vmx.c +++ w/src/vmx/vmx.c @@ -2164,6 +2164,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con /* Setup virDomainDiskDef */ if (device == VIR_DOMAIN_DISK_DEVICE_DISK) { if (virFileHasSuffix(fileName, ".vmdk")) { + char *tmp; + if (deviceType != NULL) { if (busType == VIR_DOMAIN_DISK_BUS_SCSI && STRCASENEQ(deviceType, "scsi-hardDisk") && @@ -2185,19 +2187,18 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con } virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE); - if (virDomainDiskSetSource(*def, - ctx->parseFileName(fileName, - ctx->opaque)) < 0) + if (!(tmp = ctx->parseFileName(fileName, ctx->opaque))) + goto cleanup; + if (virDomainDiskSetSource(*def, tmp) < 0) { + VIR_FREE(tmp); goto cleanup; + } + VIR_FREE(tmp); (*def)->cachemode = writeThrough ? VIR_DOMAIN_DISK_CACHE_WRITETHRU : VIR_DOMAIN_DISK_CACHE_DEFAULT; if (mode) (*def)->transient = STRCASEEQ(mode, "independent-nonpersistent"); - - if (!virDomainDiskGetSource(*def)) { - goto cleanup; - } } else if (virFileHasSuffix(fileName, ".iso") || STRCASEEQ(deviceType, "atapi-cdrom") || STRCASEEQ(deviceType, "cdrom-raw")) { @@ -2218,6 +2219,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con } } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) { if (virFileHasSuffix(fileName, ".iso")) { + char *tmp; + if (deviceType != NULL) { if (STRCASENEQ(deviceType, "cdrom-image")) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2228,14 +2231,13 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con } virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE); - if (virDomainDiskSetSource(*def, - ctx->parseFileName(fileName, - ctx->opaque)) < 0) + if (!(tmp = ctx->parseFileName(fileName, ctx->opaque))) goto cleanup; - - if (!virDomainDiskGetSource(*def)) { + if (virDomainDiskSetSource(*def, tmp) < 0) { + VIR_FREE(tmp); goto cleanup; } + VIR_FREE(tmp); } else if (virFileHasSuffix(fileName, ".vmdk")) { /* * This function was called in order to parse a CDROM device, but @@ -2278,11 +2280,16 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con if (virDomainDiskSetSource(*def, fileName) < 0) goto cleanup; } else if (fileType != NULL && STRCASEEQ(fileType, "file")) { + char *tmp; + virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE); - if (virDomainDiskSetSource(*def, - ctx->parseFileName(fileName, - ctx->opaque)) < 0) + if (!(tmp = ctx->parseFileName(fileName, ctx->opaque))) goto cleanup; + if (virDomainDiskSetSource(*def, tmp) < 0) { + VIR_FREE(tmp); + goto cleanup; + } + VIR_FREE(tmp); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid or not yet handled value '%s' " -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Part of a series of cleanups to use new accessor methods. * src/xen/xend_internal.c (virDomainXMLDevID): Use accessors. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/xen/xend_internal.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 4b10f42..55604bc 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1,7 +1,7 @@ /* * xend_internal.c: access to Xen though the Xen Daemon interface * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori <aliguori@us.ibm.com> * * This library is free software; you can redistribute it and/or @@ -3338,14 +3338,11 @@ virDomainXMLDevID(virConnectPtr conn, xenUnifiedPrivatePtr priv = conn->privateData; char *xref; char *tmp; + const char *driver = virDomainDiskGetDriver(dev->data.disk); if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - if (dev->data.disk->driverName && - STREQ(dev->data.disk->driverName, "tap")) - strcpy(class, "tap"); - else if (dev->data.disk->driverName && - STREQ(dev->data.disk->driverName, "tap2")) - strcpy(class, "tap2"); + if (STREQ_NULLABLE(driver, "tap") || STREQ_NULLABLE(driver, "tap2")) + strcpy(class, driver); else strcpy(class, "vbd"); -- 1.8.5.3

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@redhat.com> --- 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@us.ibm.com> * @@ -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

On 03/19/2014 11:20 AM, Eric Blake wrote:
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@redhat.com> ---
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.
I looked through those sections and they all *appear* to be correct (in particular, you didn't forget to update your "src" pointer after the disk->src had been changed). But if you want to wait for someone who can actual test xenxs be my guest.

It's finally time to start tracking disk backing chains in <domain> XML. The first step is to start refactoring code so that we have an object more convenient for representing each host source resource in the context of a single guest <disk>. Ultimately, I plan to move the new type into src/util where it can be reused by virStorageFile, but to make the transition easier to review, this patch just creates the new type then fixes everything until it compiles again. * src/conf/domain_conf.h (_virDomainDiskDef): Split... (_virDomainDiskSourceDef): ...to new struct. (virDomainDiskAuthClear): Use new type. * src/conf/domain_conf.c (virDomainDiskDefFree): Split... (virDomainDiskSourceDefClear): ...to new function. (virDomainDiskGetType, virDomainDiskSetType) (virDomainDiskGetSource, virDomainDiskSetSource) (virDomainDiskGetDriver, virDomainDiskSetDriver) (virDomainDiskGetFormat, virDomainDiskSetFormat) (virDomainDiskAuthClear, virDomainDiskGetActualType) (virDomainDiskDefParseXML, virDomainDiskSourceDefFormat) (virDomainDiskDefFormat, virDomainDiskDefForeachPath) (virDomainDiskDefGetSecurityLabelDef) (virDomainDiskSourceIsBlockType): Adjust all users. * src/lxc/lxc_controller.c (virLXCControllerSetupDisk): Likewise. * src/lxc/lxc_driver.c (lxcDomainAttachDeviceMknodHelper): Likewise. * src/qemu/qemu_command.c (qemuAddRBDHost, qemuParseRBDString) (qemuParseDriveURIString, qemuParseGlusterString) (qemuParseISCSIString, qemuParseNBDString) (qemuDomainDiskGetSourceString, qemuBuildDriveStr) (qemuBuildCommandLine, qemuParseCommandLineDisk) (qemuParseCommandLine): Likewise. * src/qemu/qemu_conf.c (qemuCheckSharedDevice) (qemuAddISCSIPoolSourceHost, qemuTranslateDiskSourcePool): Likewise. * src/qemu/qemu_driver.c (qemuDomainUpdateDeviceConfig) (qemuDomainPrepareDiskChainElement) (qemuDomainSnapshotCreateInactiveExternal) (qemuDomainSnapshotPrepareDiskExternalBackingInactive) (qemuDomainSnapshotPrepareDiskInternal) (qemuDomainSnapshotPrepare) (qemuDomainSnapshotCreateSingleDiskActive) (qemuDomainSnapshotUndoSingleDiskActive) (qemuDomainBlockPivot, qemuDomainBlockJobImpl) (qemuDomainBlockCopy, qemuDomainBlockCommit): Likewise. * src/qemu/qemu_migration.c (qemuMigrationIsSafe): Likewise. * src/qemu/qemu_process.c (qemuProcessGetVolumeQcowPassphrase) (qemuProcessInitPasswords): Likewise. * src/security/security_selinux.c (virSecuritySELinuxSetSecurityFileLabel): Likewise. * src/storage/storage_driver.c (virStorageFileInitFromDiskDef): Likewise. * tests/securityselinuxlabeltest.c (testSELinuxLoadDef): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- The accessor functions made a huge difference. I wrote this patch first, and it was over 200k in size; then after redoing the code to inject accessors, this shrunk under 100k and touches fewer files. Best of all, most of the hypervisor drivers that aren't quite as actively developed are NOT part of this patch, which means my refactoring into accessors properly isolated the most common usage of fields that got relocated into the new struct. src/conf/domain_conf.c | 187 +++++++++++++----------- src/conf/domain_conf.h | 37 +++-- src/lxc/lxc_controller.c | 8 +- src/lxc/lxc_driver.c | 8 +- src/qemu/qemu_command.c | 308 ++++++++++++++++++++------------------- src/qemu/qemu_conf.c | 86 +++++------ src/qemu/qemu_driver.c | 185 +++++++++++------------ src/qemu/qemu_migration.c | 4 +- src/qemu/qemu_process.c | 8 +- src/security/security_selinux.c | 2 +- src/storage/storage_driver.c | 8 +- tests/securityselinuxlabeltest.c | 8 +- 12 files changed, 441 insertions(+), 408 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a0e07c7..c664624 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1248,25 +1248,19 @@ virDomainDiskSourcePoolDefFree(virDomainDiskSourcePoolDefPtr def) VIR_FREE(def); } -void virDomainDiskDefFree(virDomainDiskDefPtr def) + +static void +virDomainDiskSourceDefClear(virDomainDiskSourceDefPtr def) { size_t i; if (!def) return; - VIR_FREE(def->serial); - VIR_FREE(def->src); + VIR_FREE(def->path); virDomainDiskSourcePoolDefFree(def->srcpool); - VIR_FREE(def->dst); VIR_FREE(def->driverName); - virStorageFileFreeMetadata(def->backingChain); - VIR_FREE(def->mirror); - VIR_FREE(def->wwn); - VIR_FREE(def->vendor); - VIR_FREE(def->product); virStorageEncryptionFree(def->encryption); - virDomainDeviceInfoClear(&def->info); if (def->seclabels) { for (i = 0; i < def->nseclabels; i++) @@ -1276,13 +1270,31 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) virDomainDiskHostDefFree(def->nhosts, def->hosts); virDomainDiskAuthClear(def); +} + + +void +virDomainDiskDefFree(virDomainDiskDefPtr def) +{ + if (!def) + return; + + virDomainDiskSourceDefClear(&def->src); + VIR_FREE(def->serial); + VIR_FREE(def->dst); + virStorageFileFreeMetadata(def->backingChain); + VIR_FREE(def->mirror); + VIR_FREE(def->wwn); + VIR_FREE(def->vendor); + VIR_FREE(def->product); + virDomainDeviceInfoClear(&def->info); VIR_FREE(def); } void -virDomainDiskAuthClear(virDomainDiskDefPtr def) +virDomainDiskAuthClear(virDomainDiskSourceDefPtr def) { VIR_FREE(def->auth.username); @@ -1357,31 +1369,31 @@ error: int virDomainDiskGetType(virDomainDiskDefPtr def) { - return def->type; + return def->src.type; } void virDomainDiskSetType(virDomainDiskDefPtr def, int type) { - def->type = type; + def->src.type = type; } int virDomainDiskGetActualType(virDomainDiskDefPtr def) { - if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME && def->srcpool) - return def->srcpool->actualtype; + if (def->src.type == VIR_DOMAIN_DISK_TYPE_VOLUME && def->src.srcpool) + return def->src.srcpool->actualtype; - return def->type; + return def->src.type; } const char * virDomainDiskGetSource(virDomainDiskDefPtr def) { - return def->src; + return def->src.path; } @@ -1389,11 +1401,11 @@ int virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src) { int ret; - char *tmp = def->src; + char *tmp = def->src.path; - ret = VIR_STRDUP(def->src, src); + ret = VIR_STRDUP(def->src.path, src); if (ret < 0) - def->src = tmp; + def->src.path = tmp; else VIR_FREE(tmp); return ret; @@ -1403,7 +1415,7 @@ virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src) const char * virDomainDiskGetDriver(virDomainDiskDefPtr def) { - return def->driverName; + return def->src.driverName; } @@ -1411,11 +1423,11 @@ int virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) { int ret; - char *tmp = def->driverName; + char *tmp = def->src.driverName; - ret = VIR_STRDUP(def->driverName, name); + ret = VIR_STRDUP(def->src.driverName, name); if (ret < 0) - def->driverName = tmp; + def->src.driverName = tmp; else VIR_FREE(tmp); return ret; @@ -1425,14 +1437,14 @@ virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) int virDomainDiskGetFormat(virDomainDiskDefPtr def) { - return def->format; + return def->src.format; } void virDomainDiskSetFormat(virDomainDiskDefPtr def, int format) { - def->format = format; + def->src.format = format; } @@ -5262,13 +5274,13 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, type = virXMLPropString(node, "type"); if (type) { - if ((def->type = virDomainDiskTypeFromString(type)) < 0) { + if ((def->src.type = virDomainDiskTypeFromString(type)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown disk type '%s'"), type); goto error; } } else { - def->type = VIR_DOMAIN_DISK_TYPE_FILE; + def->src.type = VIR_DOMAIN_DISK_TYPE_FILE; } snapshot = virXMLPropString(node, "snapshot"); @@ -5279,22 +5291,22 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { - if (!source && !def->hosts && !def->srcpool && + if (!source && !def->src.hosts && !def->src.srcpool && xmlStrEqual(cur->name, BAD_CAST "source")) { sourceNode = cur; - if (virDomainDiskSourceDefParse(cur, def->type, + if (virDomainDiskSourceDefParse(cur, def->src.type, &source, - &def->protocol, - &def->nhosts, - &def->hosts, - &def->srcpool) < 0) + &def->src.protocol, + &def->src.nhosts, + &def->src.hosts, + &def->src.srcpool) < 0) goto error; - if (def->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { - if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) + if (def->src.type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + if (def->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI; - else if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) + else if (def->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH; } @@ -5403,7 +5415,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - def->auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_NONE; + def->src.auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_NONE; child = cur->children; while (child != NULL) { if (child->type == XML_ELEMENT_NODE && @@ -5440,17 +5452,17 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } if (authUUID != NULL) { - def->auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_UUID; + def->src.auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_UUID; if (virUUIDParse(authUUID, - def->auth.secret.uuid) < 0) { + def->src.auth.secret.uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, _("malformed uuid %s"), authUUID); goto error; } } else if (authUsage != NULL) { - def->auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_USAGE; - def->auth.secret.usage = authUsage; + def->src.auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_USAGE; + def->src.auth.secret.usage = authUsage; authUsage = NULL; } } @@ -5595,7 +5607,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present. LUN is for raw access CD-ROMs * that are not attached to a physical device presently */ - if (source == NULL && def->hosts == NULL && !def->srcpool && + if (source == NULL && def->src.hosts == NULL && !def->src.srcpool && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && def->device != VIR_DOMAIN_DISK_DEVICE_LUN && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { @@ -5608,8 +5620,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, if (sourceNode) { xmlNodePtr saved_node = ctxt->node; ctxt->node = sourceNode; - if (virSecurityDeviceLabelDefParseXML(&def->seclabels, - &def->nseclabels, + if (virSecurityDeviceLabelDefParseXML(&def->src.seclabels, + &def->src.nseclabels, vmSeclabels, nvmSeclabels, ctxt, @@ -5619,10 +5631,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } if (target == NULL) { - if (def->srcpool) { + if (def->src.srcpool) { char *tmp; if (virAsprintf(&tmp, "pool = '%s', volume = '%s'", - def->srcpool->pool, def->srcpool->volume) < 0) + def->src.srcpool->pool, def->src.srcpool->volume) < 0) goto error; virReportError(VIR_ERR_NO_TARGET, "%s", tmp); @@ -5887,7 +5899,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - if (def->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + if (def->src.type == VIR_DOMAIN_DISK_TYPE_NETWORK) { virReportError(VIR_ERR_XML_ERROR, _("Setting disk %s is not allowed for " "disk of network type"), @@ -5906,18 +5918,18 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, def->startupPolicy = val; } - def->src = source; + def->src.path = source; source = NULL; def->dst = target; target = NULL; - def->auth.username = authUsername; + def->src.auth.username = authUsername; authUsername = NULL; - def->driverName = driverName; + def->src.driverName = driverName; driverName = NULL; def->mirror = mirror; mirror = NULL; def->mirroring = mirroring; - def->encryption = encryption; + def->src.encryption = encryption; encryption = NULL; def->serial = serial; serial = NULL; @@ -5929,8 +5941,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, product = NULL; if (driverType) { - def->format = virStorageFileFormatTypeFromString(driverType); - if (def->format <= 0) { + def->src.format = virStorageFileFormatTypeFromString(driverType); + if (def->src.format <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown driver format value '%s'"), driverType); @@ -14900,15 +14912,15 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, unsigned int flags) { return virDomainDiskSourceDefFormatInternal(buf, - def->type, - def->src, + def->src.type, + def->src.path, def->startupPolicy, - def->protocol, - def->nhosts, - def->hosts, - def->nseclabels, - def->seclabels, - def->srcpool, + def->src.protocol, + def->src.nhosts, + def->src.hosts, + def->src.nseclabels, + def->src.seclabels, + def->src.srcpool, flags); } @@ -14918,7 +14930,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virDomainDiskDefPtr def, unsigned int flags) { - const char *type = virDomainDiskTypeToString(def->type); + const char *type = virDomainDiskTypeToString(def->src.type); const char *device = virDomainDiskDeviceTypeToString(def->device); const char *bus = virDomainDiskBusTypeToString(def->bus); const char *cachemode = virDomainDiskCacheTypeToString(def->cachemode); @@ -14935,7 +14947,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected disk type %d"), def->type); + _("unexpected disk type %d"), def->src.type); return -1; } if (!device) { @@ -14985,14 +14997,14 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); - if (def->driverName || def->format > 0 || def->cachemode || + if (def->src.driverName || def->src.format > 0 || def->cachemode || def->ioeventfd || def->event_idx || def->copy_on_read) { virBufferAddLit(buf, "<driver"); - if (def->driverName) - virBufferAsprintf(buf, " name='%s'", def->driverName); - if (def->format > 0) + if (def->src.driverName) + virBufferAsprintf(buf, " name='%s'", def->src.driverName); + if (def->src.format > 0) virBufferAsprintf(buf, " type='%s'", - virStorageFileFormatTypeToString(def->format)); + virStorageFileFormatTypeToString(def->src.format)); if (def->cachemode) virBufferAsprintf(buf, " cache='%s'", cachemode); if (def->error_policy) @@ -15012,23 +15024,23 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - if (def->auth.username) { + if (def->src.auth.username) { virBufferEscapeString(buf, "<auth username='%s'>\n", - def->auth.username); + def->src.auth.username); virBufferAdjustIndent(buf, 2); - if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { + if (def->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { virBufferAddLit(buf, "<secret type='iscsi'"); - } else if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + } else if (def->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { virBufferAddLit(buf, "<secret type='ceph'"); } - if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_UUID) { - virUUIDFormat(def->auth.secret.uuid, uuidstr); + if (def->src.auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_UUID) { + virUUIDFormat(def->src.auth.secret.uuid, uuidstr); virBufferAsprintf(buf, " uuid='%s'/>\n", uuidstr); } - if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) { + if (def->src.auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) { virBufferEscapeString(buf, " usage='%s'/>\n", - def->auth.secret.usage); + def->src.auth.secret.usage); } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</auth>\n"); @@ -15119,7 +15131,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, "<wwn>%s</wwn>\n", def->wwn); virBufferEscapeString(buf, "<vendor>%s</vendor>\n", def->vendor); virBufferEscapeString(buf, "<product>%s</product>\n", def->product); - if (def->encryption && virStorageEncryptionFormat(buf, def->encryption) < 0) + if (def->src.encryption && + virStorageEncryptionFormat(buf, def->src.encryption) < 0) return -1; if (virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) < 0) @@ -18536,8 +18549,8 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, if (!path || type == VIR_DOMAIN_DISK_TYPE_NETWORK || (type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT)) + disk->src.srcpool && + disk->src.srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT)) return 0; if (iter(disk, path, 0, opaque) < 0) @@ -19299,9 +19312,9 @@ virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model) if (def == NULL) return NULL; - for (i = 0; i < def->nseclabels; i++) { - if (STREQ_NULLABLE(def->seclabels[i]->model, model)) - return def->seclabels[i]; + for (i = 0; i < def->src.nseclabels; i++) { + if (STREQ_NULLABLE(def->src.seclabels[i]->model, model)) + return def->src.seclabels[i]; } return NULL; } @@ -19420,14 +19433,14 @@ virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) * If it's a block type source pool, then it's possible */ if (virDomainDiskGetType(def) == VIR_DOMAIN_DISK_TYPE_VOLUME && - def->srcpool && - def->srcpool->voltype == VIR_STORAGE_VOL_BLOCK) { + def->src.srcpool && + def->src.srcpool->voltype == VIR_STORAGE_VOL_BLOCK) { /* We don't think the volume accessed by remote URI is * block type source, since we can't/shouldn't manage it * (e.g. set sgio=filtered|unfiltered for it) in libvirt. */ - if (def->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI && - def->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) + if (def->src.srcpool->pooltype == VIR_STORAGE_POOL_ISCSI && + def->src.srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) return false; return true; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cc447b0..ef3dd38 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -705,15 +705,15 @@ struct _virDomainDiskSourcePoolDef { }; typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr; -/* Stores the virtual disk configuration */ -struct _virDomainDiskDef { +typedef struct _virDomainDiskSourceDef virDomainDiskSourceDef; +typedef virDomainDiskSourceDef *virDomainDiskSourceDefPtr; + +/* Stores information related to a host resource. In the case of + * backing chains, multiple source disks join to form a single guest + * view. TODO Move this to util/ */ +struct _virDomainDiskSourceDef { int type; /* enum virDomainDiskType */ - int device; /* enum virDomainDiskDevice */ - int bus; /* enum virDomainDiskBus */ - char *src; - char *dst; - int tray_status; /* enum virDomainDiskTray */ - int removable; /* enum virDomainFeatureState */ + char *path; int protocol; /* enum virDomainDiskProtocol */ size_t nhosts; virDomainDiskHostDefPtr hosts; @@ -726,8 +726,23 @@ struct _virDomainDiskDef { char *usage; } secret; } auth; + virStorageEncryptionPtr encryption; char *driverName; int format; /* enum virStorageFileFormat */ + + size_t nseclabels; + virSecurityDeviceLabelDefPtr *seclabels; +}; + +/* Stores the virtual disk configuration */ +struct _virDomainDiskDef { + virDomainDiskSourceDef src; + + int device; /* enum virDomainDiskDevice */ + int bus; /* enum virDomainDiskBus */ + char *dst; + int tray_status; /* enum virDomainDiskTray */ + int removable; /* enum virDomainFeatureState */ virStorageFileMetadataPtr backingChain; char *mirror; @@ -765,14 +780,10 @@ struct _virDomainDiskDef { bool shared; bool transient; virDomainDeviceInfo info; - virStorageEncryptionPtr encryption; bool rawio_specified; int rawio; /* no = 0, yes = 1 */ int sgio; /* enum virDomainDeviceSGIO */ int discard; /* enum virDomainDiskDiscard */ - - size_t nseclabels; - virSecurityDeviceLabelDefPtr *seclabels; }; @@ -2251,7 +2262,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainLeaseDefFree(virDomainLeaseDefPtr def); -void virDomainDiskAuthClear(virDomainDiskDefPtr def); +void virDomainDiskAuthClear(virDomainDiskSourceDefPtr def); void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def); void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts); virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 6ed13fb..3425110 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1669,7 +1669,7 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, int ret = -1; struct stat sb; mode_t mode; - char *tmpsrc = def->src; + char *tmpsrc = def->src.path; if (virDomainDiskGetType(def) != VIR_DOMAIN_DISK_TYPE_BLOCK) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -1686,7 +1686,7 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, LXC_STATE_DIR, ctrl->def->name, def->dst) < 0) goto cleanup; - if (stat(def->src, &sb) < 0) { + if (stat(def->src.path, &sb) < 0) { virReportSystemError(errno, _("Unable to access %s"), tmpsrc); goto cleanup; @@ -1726,14 +1726,14 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, /* Labelling normally operates on src, but we need * to actually label the dst here, so hack the config */ - def->src = dst; + def->src.path = dst; if (virSecurityManagerSetImageLabel(securityDriver, ctrl->def, def) < 0) goto cleanup; ret = 0; cleanup: - def->src = tmpsrc; + def->src.path = tmpsrc; VIR_FREE(dst); return ret; } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 5546861..7fd95cc 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3901,14 +3901,14 @@ lxcDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, switch (data->def->type) { case VIR_DOMAIN_DEVICE_DISK: { virDomainDiskDefPtr def = data->def->data.disk; - char *tmpsrc = def->src; - def->src = data->file; + char *tmpsrc = def->src.path; + def->src.path = data->file; if (virSecurityManagerSetImageLabel(data->driver->securityManager, data->vm->def, def) < 0) { - def->src = tmpsrc; + def->src.path = tmpsrc; goto cleanup; } - def->src = tmpsrc; + def->src.path = tmpsrc; } break; case VIR_DOMAIN_DEVICE_HOSTDEV: { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5ca3f74..e4783c6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3254,7 +3254,7 @@ static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport) size_t skip; char **parts; - if (VIR_EXPAND_N(disk->hosts, disk->nhosts, 1) < 0) + if (VIR_EXPAND_N(disk->src.hosts, disk->src.nhosts, 1) < 0) return -1; if ((port = strchr(hostport, ']'))) { @@ -3269,29 +3269,29 @@ static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport) if (port) { *port = '\0'; port += skip; - if (VIR_STRDUP(disk->hosts[disk->nhosts - 1].port, port) < 0) + if (VIR_STRDUP(disk->src.hosts[disk->src.nhosts - 1].port, port) < 0) goto error; } else { - if (VIR_STRDUP(disk->hosts[disk->nhosts - 1].port, "6789") < 0) + if (VIR_STRDUP(disk->src.hosts[disk->src.nhosts - 1].port, "6789") < 0) goto error; } parts = virStringSplit(hostport, "\\:", 0); if (!parts) goto error; - disk->hosts[disk->nhosts-1].name = virStringJoin((const char **)parts, ":"); + disk->src.hosts[disk->src.nhosts-1].name = virStringJoin((const char **)parts, ":"); virStringFreeList(parts); - if (!disk->hosts[disk->nhosts-1].name) + if (!disk->src.hosts[disk->src.nhosts-1].name) goto error; - disk->hosts[disk->nhosts-1].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; - disk->hosts[disk->nhosts-1].socket = NULL; + disk->src.hosts[disk->src.nhosts-1].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + disk->src.hosts[disk->src.nhosts-1].socket = NULL; return 0; error: - VIR_FREE(disk->hosts[disk->nhosts-1].port); - VIR_FREE(disk->hosts[disk->nhosts-1].name); + VIR_FREE(disk->src.hosts[disk->src.nhosts-1].port); + VIR_FREE(disk->src.hosts[disk->src.nhosts-1].name); return -1; } @@ -3301,7 +3301,7 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk) char *options = NULL; char *p, *e, *next; - p = strchr(disk->src, ':'); + p = strchr(disk->src.path, ':'); if (p) { if (VIR_STRDUP(options, p + 1) < 0) goto error; @@ -3330,7 +3330,7 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk) } if (STRPREFIX(p, "id=") && - VIR_STRDUP(disk->auth.username, p + strlen("id=")) < 0) + VIR_STRDUP(disk->src.auth.username, p + strlen("id=")) < 0) goto error; if (STRPREFIX(p, "mon_host=")) { char *h, *sep; @@ -3373,7 +3373,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, char *volimg = NULL; char *secret = NULL; - if (VIR_ALLOC(def->hosts) < 0) + if (VIR_ALLOC(def->src.hosts) < 0) goto error; transp = strchr(uri->scheme, '+'); @@ -3387,30 +3387,30 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, } if (!transp) { - def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + def->src.hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; } else { - def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp); - if (def->hosts->transport < 0) { + def->src.hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp); + if (def->src.hosts->transport < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid %s transport type '%s'"), scheme, transp); goto error; } } - def->nhosts = 0; /* set to 1 once everything succeeds */ + def->src.nhosts = 0; /* set to 1 once everything succeeds */ - if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { - if (VIR_STRDUP(def->hosts->name, uri->server) < 0) + if (def->src.hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { + if (VIR_STRDUP(def->src.hosts->name, uri->server) < 0) goto error; - if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) + if (virAsprintf(&def->src.hosts->port, "%d", uri->port) < 0) goto error; } else { - def->hosts->name = NULL; - def->hosts->port = 0; + def->src.hosts->name = NULL; + def->src.hosts->port = 0; if (uri->query) { if (STRPREFIX(uri->query, "socket=")) { sock = strchr(uri->query, '=') + 1; - if (VIR_STRDUP(def->hosts->socket, sock) < 0) + if (VIR_STRDUP(def->src.hosts->socket, sock) < 0) goto error; } else { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3421,12 +3421,11 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, } if (uri->path) { volimg = uri->path + 1; /* skip the prefix slash */ - VIR_FREE(def->src); - if (VIR_STRDUP(def->src, volimg) < 0) + VIR_FREE(def->src.path); + if (VIR_STRDUP(def->src.path, volimg) < 0) goto error; } else { - VIR_FREE(def->src); - def->src = NULL; + VIR_FREE(def->src.path); } if (uri->user) { @@ -3434,11 +3433,11 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, if (secret) *secret = '\0'; - if (VIR_STRDUP(def->auth.username, uri->user) < 0) + if (VIR_STRDUP(def->src.auth.username, uri->user) < 0) goto error; } - def->nhosts = 1; + def->src.nhosts = 1; ret = 0; cleanup: @@ -3447,8 +3446,8 @@ cleanup: return ret; error: - virDomainDiskHostDefClear(def->hosts); - VIR_FREE(def->hosts); + virDomainDiskHostDefClear(def->src.hosts); + VIR_FREE(def->src.hosts); goto cleanup; } @@ -3457,7 +3456,7 @@ qemuParseGlusterString(virDomainDiskDefPtr def) { virURIPtr uri = NULL; - if (!(uri = virURIParse(def->src))) + if (!(uri = virURIParse(def->src.path))) return -1; return qemuParseDriveURIString(def, uri, "gluster"); @@ -3470,7 +3469,7 @@ qemuParseISCSIString(virDomainDiskDefPtr def) char *slash; unsigned lun; - if (!(uri = virURIParse(def->src))) + if (!(uri = virURIParse(def->src.path))) return -1; if (uri->path && @@ -3480,7 +3479,8 @@ qemuParseISCSIString(virDomainDiskDefPtr def) *slash = '\0'; else if (virStrToLong_ui(slash + 1, NULL, 10, &lun) == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid name '%s' for iSCSI disk"), def->src); + _("invalid name '%s' for iSCSI disk"), + def->src.path); return -1; } } @@ -3497,8 +3497,8 @@ qemuParseNBDString(virDomainDiskDefPtr disk) virURIPtr uri = NULL; - if (strstr(disk->src, "://")) { - if (!(uri = virURIParse(disk->src))) + if (strstr(disk->src.path, "://")) { + if (!(uri = virURIParse(disk->src.path))) return -1; return qemuParseDriveURIString(disk, uri, "nbd"); } @@ -3506,7 +3506,7 @@ qemuParseNBDString(virDomainDiskDefPtr disk) if (VIR_ALLOC(h) < 0) goto error; - host = disk->src + strlen("nbd:"); + host = disk->src.path + strlen("nbd:"); if (STRPREFIX(host, "unix:/")) { src = strchr(host + strlen("unix:"), ':'); if (src) @@ -3519,7 +3519,7 @@ qemuParseNBDString(virDomainDiskDefPtr disk) port = strchr(host, ':'); if (!port) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse nbd filename '%s'"), disk->src); + _("cannot parse nbd filename '%s'"), disk->src.path); goto error; } @@ -3542,10 +3542,10 @@ qemuParseNBDString(virDomainDiskDefPtr disk) src = NULL; } - VIR_FREE(disk->src); - disk->src = src; - disk->nhosts = 1; - disk->hosts = h; + VIR_FREE(disk->src.path); + disk->src.path = src; + disk->src.nhosts = 1; + disk->src.hosts = h; return 0; error: @@ -3874,35 +3874,35 @@ qemuDomainDiskGetSourceString(virConnectPtr conn, *source = NULL; if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK && - disk->auth.username && - (disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI || - disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)) { + disk->src.auth.username && + (disk->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI || + disk->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)) { bool encode = false; int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; - if (disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + if (disk->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { /* qemu requires the secret to be encoded for RBD */ encode = true; secretType = VIR_SECRET_USAGE_TYPE_CEPH; } if (!(secret = qemuGetSecretString(conn, - virDomainDiskProtocolTypeToString(disk->protocol), + virDomainDiskProtocolTypeToString(disk->src.protocol), encode, - disk->auth.secretType, - disk->auth.username, - disk->auth.secret.uuid, - disk->auth.secret.usage, + disk->src.auth.secretType, + disk->src.auth.username, + disk->src.auth.secret.uuid, + disk->src.auth.secret.usage, secretType))) goto cleanup; } ret = qemuGetDriveSourceString(virDomainDiskGetActualType(disk), - disk->src, - disk->protocol, - disk->nhosts, - disk->hosts, - disk->auth.username, + disk->src.path, + disk->src.protocol, + disk->src.nhosts, + disk->src.hosts, + disk->src.auth.username, secret, source); @@ -4020,10 +4020,11 @@ qemuBuildDriveStr(virConnectPtr conn, switch (actualType) { case VIR_DOMAIN_DISK_TYPE_DIR: /* QEMU only supports magic FAT format for now */ - if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) { + if (disk->src.format > 0 && + disk->src.format != VIR_STORAGE_FILE_FAT) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported disk driver type for '%s'"), - virStorageFileFormatTypeToString(disk->format)); + virStorageFileFormatTypeToString(disk->src.format)); goto error; } @@ -4043,7 +4044,7 @@ qemuBuildDriveStr(virConnectPtr conn, case VIR_DOMAIN_DISK_TYPE_BLOCK: if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME ? + disk->src.type == VIR_DOMAIN_DISK_TYPE_VOLUME ? _("tray status 'open' is invalid for block type volume") : _("tray status 'open' is invalid for block type disk")); goto error; @@ -4103,11 +4104,11 @@ qemuBuildDriveStr(virConnectPtr conn, _("transient disks not supported yet")); goto error; } - if (disk->format > 0 && - disk->type != VIR_DOMAIN_DISK_TYPE_DIR && + if (disk->src.format > 0 && + disk->src.type != VIR_DOMAIN_DISK_TYPE_DIR && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) virBufferAsprintf(&opt, ",format=%s", - virStorageFileFormatTypeToString(disk->format)); + virStorageFileFormatTypeToString(disk->src.format)); /* generate geometry command string */ if (disk->geometry.cylinders > 0 && @@ -4318,11 +4319,11 @@ qemuBuildDriveDevStr(virDomainDefPtr def, bus); goto error; } - if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { - if (disk->protocol != VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { + if (disk->src.type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + if (disk->src.protocol != VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk device='lun' is not supported for protocol='%s'"), - virDomainDiskProtocolTypeToString(disk->protocol)); + virDomainDiskProtocolTypeToString(disk->src.protocol)); goto error; } } else if (!virDomainDiskSourceIsBlockType(disk)) { @@ -8413,11 +8414,11 @@ qemuBuildCommandLine(virConnectPtr conn, for (i = 0; i < def->ndisks; i++) { virDomainDiskDefPtr disk = def->disks[i]; - if (disk->driverName != NULL && - !STREQ(disk->driverName, "qemu")) { + if (disk->src.driverName != NULL && + !STREQ(disk->src.driverName, "qemu")) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported driver name '%s' for disk '%s'"), - disk->driverName, disk->src); + disk->src.driverName, disk->src.path); goto error; } } @@ -8541,11 +8542,11 @@ qemuBuildCommandLine(virConnectPtr conn, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src); + virCommandAddArgFormat(cmd, "disk:%s", disk->src.path); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported usb disk type for '%s'"), - disk->src); + disk->src.path); goto error; } continue; @@ -8631,7 +8632,7 @@ qemuBuildCommandLine(virConnectPtr conn, const char *fmt; virDomainDiskDefPtr disk = def->disks[i]; - if ((disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) && + if ((disk->src.type == VIR_DOMAIN_DISK_TYPE_BLOCK) && (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("tray status 'open' is invalid for " @@ -8642,11 +8643,11 @@ qemuBuildCommandLine(virConnectPtr conn, if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src); + virCommandAddArgFormat(cmd, "disk:%s", disk->src.path); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported usb disk type for '%s'"), - disk->src); + disk->src.path); goto error; } continue; @@ -8654,7 +8655,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (STREQ(disk->dst, "hdc") && disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (disk->src) { + if (disk->src.path) { snprintf(dev, NAME_MAX, "-%s", "cdrom"); } else { continue; @@ -8670,12 +8671,13 @@ qemuBuildCommandLine(virConnectPtr conn, } } - if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { + if (disk->src.type == VIR_DOMAIN_DISK_TYPE_DIR) { /* QEMU only supports magic FAT format for now */ - if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) { + if (disk->src.format > 0 && + disk->src.format != VIR_STORAGE_FILE_FAT) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported disk driver type for '%s'"), - virStorageFileFormatTypeToString(disk->format)); + virStorageFileFormatTypeToString(disk->src.format)); goto error; } if (!disk->readonly) { @@ -8690,11 +8692,11 @@ qemuBuildCommandLine(virConnectPtr conn, if (virAsprintf(&file, fmt, disk->src) < 0) goto error; - } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + } else if (disk->src.type == VIR_DOMAIN_DISK_TYPE_NETWORK) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("network disks are only supported with -drive")); } else { - if (VIR_STRDUP(file, disk->src) < 0) { + if (VIR_STRDUP(file, disk->src.path) < 0) { goto error; } } @@ -10198,28 +10200,28 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, else def->bus = VIR_DOMAIN_DISK_BUS_IDE; def->device = VIR_DOMAIN_DISK_DEVICE_DISK; - def->type = VIR_DOMAIN_DISK_TYPE_FILE; + def->src.type = VIR_DOMAIN_DISK_TYPE_FILE; for (i = 0; i < nkeywords; i++) { if (STREQ(keywords[i], "file")) { if (values[i] && STRNEQ(values[i], "")) { - def->src = values[i]; + def->src.path = values[i]; values[i] = NULL; - if (STRPREFIX(def->src, "/dev/")) - def->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else if (STRPREFIX(def->src, "nbd:") || - STRPREFIX(def->src, "nbd+")) { - def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - def->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; + if (STRPREFIX(def->src.path, "/dev/")) + def->src.type = VIR_DOMAIN_DISK_TYPE_BLOCK; + else if (STRPREFIX(def->src.path, "nbd:") || + STRPREFIX(def->src.path, "nbd+")) { + def->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; if (qemuParseNBDString(def) < 0) goto error; - } else if (STRPREFIX(def->src, "rbd:")) { - char *p = def->src; + } else if (STRPREFIX(def->src.path, "rbd:")) { + char *p = def->src.path; - def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; - if (VIR_STRDUP(def->src, p + strlen("rbd:")) < 0) + def->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + if (VIR_STRDUP(def->src.path, p + strlen("rbd:")) < 0) goto error; /* old-style CEPH_ARGS env variable is parsed later */ if (!old_style_ceph_args && qemuParseRBDString(def) < 0) { @@ -10228,57 +10230,58 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } VIR_FREE(p); - } else if (STRPREFIX(def->src, "gluster:") || - STRPREFIX(def->src, "gluster+")) { - def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; + } else if (STRPREFIX(def->src.path, "gluster:") || + STRPREFIX(def->src.path, "gluster+")) { + def->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; if (qemuParseGlusterString(def) < 0) goto error; - } else if (STRPREFIX(def->src, "iscsi:")) { - def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - def->protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; + } else if (STRPREFIX(def->src.path, "iscsi:")) { + def->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; if (qemuParseISCSIString(def) < 0) goto error; - } else if (STRPREFIX(def->src, "sheepdog:")) { - char *p = def->src; + } else if (STRPREFIX(def->src.path, "sheepdog:")) { + char *p = def->src.path; char *port, *vdi; - def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - def->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; - if (VIR_STRDUP(def->src, p + strlen("sheepdog:")) < 0) + def->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; + if (VIR_STRDUP(def->src.path, p + strlen("sheepdog:")) < 0) goto error; VIR_FREE(p); - /* def->src must be [vdiname] or [host]:[port]:[vdiname] */ - port = strchr(def->src, ':'); + /* def->src.path must be [vdiname] or [host]:[port]:[vdiname] */ + port = strchr(def->src.path, ':'); if (port) { *port = '\0'; vdi = strchr(port + 1, ':'); if (!vdi) { *port = ':'; virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse sheepdog filename '%s'"), def->src); + _("cannot parse sheepdog filename '%s'"), + def->src.path); goto error; } port++; *vdi++ = '\0'; - if (VIR_ALLOC(def->hosts) < 0) + if (VIR_ALLOC(def->src.hosts) < 0) goto error; - def->nhosts = 1; - def->hosts->name = def->src; - if (VIR_STRDUP(def->hosts->port, port) < 0) + def->src.nhosts = 1; + def->src.hosts->name = def->src.path; + if (VIR_STRDUP(def->src.hosts->port, port) < 0) goto error; - def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; - def->hosts->socket = NULL; - if (VIR_STRDUP(def->src, vdi) < 0) + def->src.hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + def->src.hosts->socket = NULL; + if (VIR_STRDUP(def->src.path, vdi) < 0) goto error; } } else - def->type = VIR_DOMAIN_DISK_TYPE_FILE; + def->src.type = VIR_DOMAIN_DISK_TYPE_FILE; } else { - def->type = VIR_DOMAIN_DISK_TYPE_FILE; + def->src.type = VIR_DOMAIN_DISK_TYPE_FILE; } } else if (STREQ(keywords[i], "if")) { if (STREQ(values[i], "ide")) { @@ -10304,9 +10307,9 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } else if (STREQ(values[i], "floppy")) def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; } else if (STREQ(keywords[i], "format")) { - if (VIR_STRDUP(def->driverName, "qemu") < 0) + if (VIR_STRDUP(def->src.driverName, "qemu") < 0) goto error; - def->format = virStorageFileFormatTypeFromString(values[i]); + def->src.format = virStorageFileFormatTypeFromString(values[i]); } else if (STREQ(keywords[i], "cache")) { if (STREQ(values[i], "off") || STREQ(values[i], "none")) @@ -10411,9 +10414,9 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (def->rerror_policy == def->error_policy) def->rerror_policy = 0; - if (!def->src && + if (!def->src.path && def->device == VIR_DOMAIN_DISK_DEVICE_DISK && - def->type != VIR_DOMAIN_DISK_TYPE_NETWORK) { + def->src.type != VIR_DOMAIN_DISK_TYPE_NETWORK) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing file parameter in drive '%s'"), val); goto error; @@ -11497,23 +11500,23 @@ qemuParseCommandLine(virCapsPtr qemuCaps, goto error; if (STRPREFIX(val, "/dev/")) - disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; + disk->src.type = VIR_DOMAIN_DISK_TYPE_BLOCK; else if (STRPREFIX(val, "nbd:")) { - disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; + disk->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; } else if (STRPREFIX(val, "rbd:")) { - disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + disk->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; val += strlen("rbd:"); } else if (STRPREFIX(val, "gluster")) { - disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; + disk->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; } else if (STRPREFIX(val, "sheepdog:")) { - disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; - disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; + disk->src.type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; val += strlen("sheepdog:"); } else - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; + disk->src.type = VIR_DOMAIN_DISK_TYPE_FILE; if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; if (((def->os.arch == VIR_ARCH_PPC64) && @@ -11539,13 +11542,13 @@ qemuParseCommandLine(virCapsPtr qemuCaps, if (VIR_STRDUP(disk->dst, arg + 1) < 0) goto error; } - if (VIR_STRDUP(disk->src, val) < 0) + if (VIR_STRDUP(disk->src.path, val) < 0) goto error; - if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + if (disk->src.type == VIR_DOMAIN_DISK_TYPE_NETWORK) { char *port; - switch (disk->protocol) { + switch (disk->src.protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: if (qemuParseNBDString(disk) < 0) goto error; @@ -11557,7 +11560,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: /* disk->src must be [vdiname] or [host]:[port]:[vdiname] */ - port = strchr(disk->src, ':'); + port = strchr(disk->src.path, ':'); if (port) { char *vdi; @@ -11569,13 +11572,13 @@ qemuParseCommandLine(virCapsPtr qemuCaps, goto error; } *vdi++ = '\0'; - if (VIR_ALLOC(disk->hosts) < 0) + if (VIR_ALLOC(disk->src.hosts) < 0) goto error; - disk->nhosts = 1; - disk->hosts->name = disk->src; - if (VIR_STRDUP(disk->hosts->port, port) < 0) + disk->src.nhosts = 1; + disk->src.hosts->name = disk->src.path; + if (VIR_STRDUP(disk->src.hosts->port, port) < 0) goto error; - if (VIR_STRDUP(disk->src, vdi) < 0) + if (VIR_STRDUP(disk->src.path, vdi) < 0) goto error; } break; @@ -11786,12 +11789,12 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } else if (STRPREFIX(val, "disk:")) { if (VIR_ALLOC(disk) < 0) goto error; - if (VIR_STRDUP(disk->src, val + strlen("disk:")) < 0) + if (VIR_STRDUP(disk->src.path, val + strlen("disk:")) < 0) goto error; - if (STRPREFIX(disk->src, "/dev/")) - disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; + if (STRPREFIX(disk->src.path, "/dev/")) + disk->src.type = VIR_DOMAIN_DISK_TYPE_BLOCK; else - disk->type = VIR_DOMAIN_DISK_TYPE_FILE; + disk->src.type = VIR_DOMAIN_DISK_TYPE_FILE; disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; disk->bus = VIR_DOMAIN_DISK_BUS_USB; disk->removable = VIR_DOMAIN_FEATURE_STATE_DEFAULT; @@ -12022,8 +12025,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps, char *hosts, *port, *saveptr = NULL, *token; virDomainDiskDefPtr first_rbd_disk = NULL; for (i = 0; i < def->ndisks; i++) { - if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_NETWORK && - def->disks[i]->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + if (def->disks[i]->src.type == VIR_DOMAIN_DISK_TYPE_NETWORK && + def->disks[i]->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { first_rbd_disk = def->disks[i]; break; } @@ -12043,10 +12046,11 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } if (VIR_STRDUP(hosts, strchr(ceph_args, ' ') + 1) < 0) goto error; - first_rbd_disk->nhosts = 0; + first_rbd_disk->src.nhosts = 0; token = strtok_r(hosts, ",", &saveptr); while (token != NULL) { - if (VIR_REALLOC_N(first_rbd_disk->hosts, first_rbd_disk->nhosts + 1) < 0) { + if (VIR_REALLOC_N(first_rbd_disk->src.hosts, + first_rbd_disk->src.nhosts + 1) < 0) { VIR_FREE(hosts); goto error; } @@ -12058,21 +12062,21 @@ qemuParseCommandLine(virCapsPtr qemuCaps, goto error; } } - first_rbd_disk->hosts[first_rbd_disk->nhosts].port = port; - if (VIR_STRDUP(first_rbd_disk->hosts[first_rbd_disk->nhosts].name, + first_rbd_disk->src.hosts[first_rbd_disk->src.nhosts].port = port; + if (VIR_STRDUP(first_rbd_disk->src.hosts[first_rbd_disk->src.nhosts].name, token) < 0) { VIR_FREE(hosts); goto error; } - first_rbd_disk->hosts[first_rbd_disk->nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; - first_rbd_disk->hosts[first_rbd_disk->nhosts].socket = NULL; + first_rbd_disk->src.hosts[first_rbd_disk->src.nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + first_rbd_disk->src.hosts[first_rbd_disk->src.nhosts].socket = NULL; - first_rbd_disk->nhosts++; + first_rbd_disk->src.nhosts++; token = strtok_r(NULL, ",", &saveptr); } VIR_FREE(hosts); - if (first_rbd_disk->nhosts == 0) { + if (first_rbd_disk->src.nhosts == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("found no rbd hosts in CEPH_ARGS '%s'"), ceph_args); goto error; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d280c2c..0d6bcfe 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -797,8 +797,8 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, virReportError(VIR_ERR_OPERATION_INVALID, _("sgio of shared disk 'pool=%s' 'volume=%s' conflicts " "with other active domains"), - disk->srcpool->pool, - disk->srcpool->volume); + disk->src.srcpool->pool, + disk->src.srcpool->volume); } else { virReportError(VIR_ERR_OPERATION_INVALID, _("sgio of shared disk '%s' conflicts with other " @@ -1158,33 +1158,33 @@ qemuAddISCSIPoolSourceHost(virDomainDiskDefPtr def, } /* iscsi pool only supports one host */ - def->nhosts = 1; + def->src.nhosts = 1; - if (VIR_ALLOC_N(def->hosts, def->nhosts) < 0) + if (VIR_ALLOC_N(def->src.hosts, def->src.nhosts) < 0) goto cleanup; - if (VIR_STRDUP(def->hosts[0].name, pooldef->source.hosts[0].name) < 0) + if (VIR_STRDUP(def->src.hosts[0].name, pooldef->source.hosts[0].name) < 0) goto cleanup; - if (virAsprintf(&def->hosts[0].port, "%d", + if (virAsprintf(&def->src.hosts[0].port, "%d", pooldef->source.hosts[0].port ? pooldef->source.hosts[0].port : 3260) < 0) goto cleanup; /* iscsi volume has name like "unit:0:0:1" */ - if (!(tokens = virStringSplit(def->srcpool->volume, ":", 0))) + if (!(tokens = virStringSplit(def->src.srcpool->volume, ":", 0))) goto cleanup; if (virStringListLength(tokens) != 4) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected iscsi volume name '%s'"), - def->srcpool->volume); + def->src.srcpool->volume); goto cleanup; } /* iscsi pool has only one source device path */ - if (virAsprintf(&def->src, "%s/%s", + if (virAsprintf(&def->src.path, "%s/%s", pooldef->source.devices[0].path, tokens[3]) < 0) goto cleanup; @@ -1192,10 +1192,10 @@ qemuAddISCSIPoolSourceHost(virDomainDiskDefPtr def, /* Storage pool have not supported these 2 attributes yet, * use the defaults. */ - def->hosts[0].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; - def->hosts[0].socket = NULL; + def->src.hosts[0].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + def->src.hosts[0].socket = NULL; - def->protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; + def->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; ret = 0; @@ -1220,34 +1220,34 @@ qemuTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def, * into the virDomainDiskDef */ if (pooldef->source.authType == VIR_STORAGE_POOL_AUTH_CHAP) { - if (VIR_STRDUP(def->auth.username, + if (VIR_STRDUP(def->src.auth.username, pooldef->source.auth.chap.username) < 0) goto cleanup; if (pooldef->source.auth.chap.secret.uuidUsable) { - def->auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_UUID; - memcpy(def->auth.secret.uuid, + def->src.auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_UUID; + memcpy(def->src.auth.secret.uuid, pooldef->source.auth.chap.secret.uuid, VIR_UUID_BUFLEN); } else { - if (VIR_STRDUP(def->auth.secret.usage, + if (VIR_STRDUP(def->src.auth.secret.usage, pooldef->source.auth.chap.secret.usage) < 0) goto cleanup; - def->auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_USAGE; + def->src.auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_USAGE; } } else if (pooldef->source.authType == VIR_STORAGE_POOL_AUTH_CEPHX) { - if (VIR_STRDUP(def->auth.username, + if (VIR_STRDUP(def->src.auth.username, pooldef->source.auth.cephx.username) < 0) goto cleanup; if (pooldef->source.auth.cephx.secret.uuidUsable) { - def->auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_UUID; - memcpy(def->auth.secret.uuid, + def->src.auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_UUID; + memcpy(def->src.auth.secret.uuid, pooldef->source.auth.cephx.secret.uuid, VIR_UUID_BUFLEN); } else { - if (VIR_STRDUP(def->auth.secret.usage, + if (VIR_STRDUP(def->src.auth.secret.usage, pooldef->source.auth.cephx.secret.usage) < 0) goto cleanup; - def->auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_USAGE; + def->src.auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_USAGE; } } ret = 0; @@ -1269,24 +1269,24 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, int ret = -1; virErrorPtr savedError = NULL; - if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME) + if (def->src.type != VIR_DOMAIN_DISK_TYPE_VOLUME) return 0; - if (!def->srcpool) + if (!def->src.srcpool) return 0; - if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool))) + if (!(pool = virStoragePoolLookupByName(conn, def->src.srcpool->pool))) return -1; if (virStoragePoolIsActive(pool) != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("storage pool '%s' containing volume '%s' " "is not active"), - def->srcpool->pool, def->srcpool->volume); + def->src.srcpool->pool, def->src.srcpool->volume); goto cleanup; } - if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume))) + if (!(vol = virStorageVolLookupByName(pool, def->src.srcpool->volume))) goto cleanup; if (virStorageVolGetInfo(vol, &info) < 0) @@ -1298,19 +1298,19 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, if (!(pooldef = virStoragePoolDefParseString(poolxml))) goto cleanup; - def->srcpool->pooltype = pooldef->type; - def->srcpool->voltype = info.type; + def->src.srcpool->pooltype = pooldef->type; + def->src.srcpool->voltype = info.type; - if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { + if (def->src.srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { virReportError(VIR_ERR_XML_ERROR, "%s", _("disk source mode is only valid when " "storage pool is of iscsi type")); goto cleanup; } - VIR_FREE(def->src); - virDomainDiskHostDefFree(def->nhosts, def->hosts); - virDomainDiskAuthClear(def); + VIR_FREE(def->src.path); + virDomainDiskHostDefFree(def->src.nhosts, def->src.hosts); + virDomainDiskAuthClear(&def->src); switch ((enum virStoragePoolType) pooldef->type) { case VIR_STORAGE_POOL_DIR: @@ -1319,7 +1319,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, case VIR_STORAGE_POOL_LOGICAL: case VIR_STORAGE_POOL_DISK: case VIR_STORAGE_POOL_SCSI: - if (!(def->src = virStorageVolGetPath(vol))) + if (!(def->src.path = virStorageVolGetPath(vol))) goto cleanup; if (def->startupPolicy && info.type != VIR_STORAGE_VOL_FILE) { @@ -1332,15 +1332,15 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, switch (info.type) { case VIR_STORAGE_VOL_FILE: - def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_FILE; + def->src.srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_FILE; break; case VIR_STORAGE_VOL_DIR: - def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_DIR; + def->src.srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_DIR; break; case VIR_STORAGE_VOL_BLOCK: - def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK; + def->src.srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK; break; case VIR_STORAGE_VOL_NETWORK: @@ -1363,20 +1363,20 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, goto cleanup; } - switch (def->srcpool->mode) { + switch (def->src.srcpool->mode) { case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DEFAULT: case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST: - def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST; + def->src.srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST; /* fallthrough */ case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST: - def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK; - if (!(def->src = virStorageVolGetPath(vol))) + def->src.srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK; + if (!(def->src.path = virStorageVolGetPath(vol))) goto cleanup; break; case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT: - def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK; - def->protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; + def->src.srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src.protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3b82c5a..49b60fa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6813,18 +6813,18 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, * Update 'orig' * We allow updating src/type//driverType/cachemode/ */ - VIR_FREE(orig->src); - orig->src = disk->src; - orig->type = disk->type; + VIR_FREE(orig->src.path); + orig->src.path = disk->src.path; + orig->src.type = disk->src.type; orig->cachemode = disk->cachemode; - if (disk->driverName) { - VIR_FREE(orig->driverName); - orig->driverName = disk->driverName; - disk->driverName = NULL; + if (disk->src.driverName) { + VIR_FREE(orig->src.driverName); + orig->src.driverName = disk->src.driverName; + disk->src.driverName = NULL; } - if (disk->format) - orig->format = disk->format; - disk->src = NULL; + if (disk->src.format) + orig->src.format = disk->src.format; + disk->src.path = NULL; break; case VIR_DOMAIN_DEVICE_NET: @@ -11965,26 +11965,27 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, /* The easiest way to label a single file with the same * permissions it would have as if part of the disk chain is to * temporarily modify the disk in place. */ - char *origsrc = disk->src; - int origformat = disk->format; + char *origsrc = disk->src.path; + int origformat = disk->src.format; virStorageFileMetadataPtr origchain = disk->backingChain; bool origreadonly = disk->readonly; int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - disk->src = (char *) file; /* casting away const is safe here */ - disk->format = VIR_STORAGE_FILE_RAW; + disk->src.path = (char *) file; /* casting away const is safe here */ + disk->src.format = VIR_STORAGE_FILE_RAW; disk->backingChain = NULL; disk->readonly = mode == VIR_DISK_CHAIN_READ_ONLY; if (mode == VIR_DISK_CHAIN_NO_ACCESS) { if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) - VIR_WARN("Unable to restore security label on %s", disk->src); + VIR_WARN("Unable to restore security label on %s", disk->src.path); if (qemuTeardownDiskCgroup(vm, disk) < 0) - VIR_WARN("Failed to teardown cgroup for disk path %s", disk->src); + VIR_WARN("Failed to teardown cgroup for disk path %s", + disk->src.path); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", disk->src); + VIR_WARN("Unable to release lock on %s", disk->src.path); } else if (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm, disk) < 0 || qemuSetupDiskCgroup(vm, disk) < 0 || @@ -11996,8 +11997,8 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, ret = 0; cleanup: - disk->src = origsrc; - disk->format = origformat; + disk->src.path = origsrc; + disk->src.format = origformat; disk->backingChain = origchain; disk->readonly = origreadonly; virObjectUnref(cfg); @@ -12094,22 +12095,22 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, NULL))) goto cleanup; - if (defdisk->format > 0) { + if (defdisk->src.format > 0) { /* adds cmd line arg: backing_file=/path/to/backing/file,backing_fmd=format */ virCommandAddArgFormat(cmd, "backing_file=%s,backing_fmt=%s", - defdisk->src, - virStorageFileFormatTypeToString(defdisk->format)); + defdisk->src.path, + virStorageFileFormatTypeToString(defdisk->src.format)); } else { if (!cfg->allowDiskFormatProbing) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown image format of '%s' and " "format probing is disabled"), - defdisk->src); + defdisk->src.path); goto cleanup; } /* adds cmd line arg: backing_file=/path/to/backing/file */ - virCommandAddArgFormat(cmd, "backing_file=%s", defdisk->src); + virCommandAddArgFormat(cmd, "backing_file=%s", defdisk->src.path); } /* adds cmd line args: /path/to/target/file */ @@ -12132,12 +12133,12 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, defdisk = vm->def->disks[snapdisk->index]; if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { - VIR_FREE(defdisk->src); - if (VIR_STRDUP(defdisk->src, snapdisk->file) < 0) { + VIR_FREE(defdisk->src.path); + if (VIR_STRDUP(defdisk->src.path, snapdisk->file) < 0) { /* we cannot rollback here in a sane way */ goto cleanup; } - defdisk->format = snapdisk->format; + defdisk->src.format = snapdisk->format; } } @@ -12260,7 +12261,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) return 0; case VIR_DOMAIN_DISK_TYPE_NETWORK: - switch ((enum virDomainDiskProtocol) disk->protocol) { + switch ((enum virDomainDiskProtocol) disk->src.protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: case VIR_DOMAIN_DISK_PROTOCOL_RBD: case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: @@ -12275,7 +12276,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) virReportError(VIR_ERR_INTERNAL_ERROR, _("external inactive snapshots are not supported on " "'network' disks using '%s' protocol"), - virDomainDiskProtocolTypeToString(disk->protocol)); + virDomainDiskProtocolTypeToString(disk->src.protocol)); return -1; } break; @@ -12465,7 +12466,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, return 0; case VIR_DOMAIN_DISK_TYPE_NETWORK: - switch ((enum virDomainDiskProtocol) disk->protocol) { + switch ((enum virDomainDiskProtocol) disk->src.protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: case VIR_DOMAIN_DISK_PROTOCOL_RBD: case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: @@ -12480,7 +12481,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, _("internal inactive snapshots are not supported on " "'network' disks using '%s' protocol"), - virDomainDiskProtocolTypeToString(disk->protocol)); + virDomainDiskProtocolTypeToString(disk->src.protocol)); return -1; } break; @@ -12540,19 +12541,19 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, active) < 0) goto cleanup; - if (dom_disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK && - (dom_disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG || - dom_disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)) { + if (dom_disk->src.type == VIR_DOMAIN_DISK_TYPE_NETWORK && + (dom_disk->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG || + dom_disk->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)) { break; } - if (vm->def->disks[i]->format > 0 && - vm->def->disks[i]->format != VIR_STORAGE_FILE_QCOW2) { + if (vm->def->disks[i]->src.format > 0 && + vm->def->disks[i]->src.format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("internal snapshot for disk %s unsupported " "for storage type %s"), disk->name, virStorageFileFormatTypeToString( - vm->def->disks[i]->format)); + vm->def->disks[i]->src.format)); goto cleanup; } break; @@ -12766,36 +12767,37 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } } - virDomainAuditDisk(vm, disk->src, source, "snapshot", ret >= 0); + virDomainAuditDisk(vm, disk->src.path, source, "snapshot", ret >= 0); if (ret < 0) goto cleanup; /* Update vm in place to match changes. */ need_unlink = false; - VIR_FREE(disk->src); - virDomainDiskHostDefFree(disk->nhosts, disk->hosts); + VIR_FREE(disk->src.path); + virDomainDiskHostDefFree(disk->src.nhosts, disk->src.hosts); - disk->src = newsource; - disk->format = format; - disk->type = snap->type; - disk->protocol = snap->protocol; - disk->nhosts = snap->nhosts; - disk->hosts = newhosts; + disk->src.path = newsource; + disk->src.format = format; + disk->src.type = snap->type; + disk->src.protocol = snap->protocol; + disk->src.nhosts = snap->nhosts; + disk->src.hosts = newhosts; newsource = NULL; newhosts = NULL; if (persistDisk) { - VIR_FREE(persistDisk->src); - virDomainDiskHostDefFree(persistDisk->nhosts, persistDisk->hosts); + VIR_FREE(persistDisk->src.path); + virDomainDiskHostDefFree(persistDisk->src.nhosts, + persistDisk->src.hosts); - persistDisk->src = persistSource; - persistDisk->format = format; - persistDisk->type = snap->type; - persistDisk->protocol = snap->protocol; - persistDisk->nhosts = snap->nhosts; - persistDisk->hosts = persistHosts; + persistDisk->src.path = persistSource; + persistDisk->src.format = format; + persistDisk->src.type = snap->type; + persistDisk->src.protocol = snap->protocol; + persistDisk->src.nhosts = snap->nhosts; + persistDisk->src.hosts = persistHosts; persistSource = NULL; persistHosts = NULL; @@ -12832,37 +12834,40 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, diskfile = virStorageFileInitFromDiskDef(disk); - if (VIR_STRDUP(source, origdisk->src) < 0 || + if (VIR_STRDUP(source, origdisk->src.path) < 0 || (persistDisk && VIR_STRDUP(persistSource, source) < 0)) goto cleanup; - qemuDomainPrepareDiskChainElement(driver, vm, disk, disk->src, + qemuDomainPrepareDiskChainElement(driver, vm, disk, disk->src.path, VIR_DISK_CHAIN_NO_ACCESS); if (need_unlink && diskfile && virStorageFileStat(diskfile, &st) == 0 && S_ISREG(st.st_mode) && virStorageFileUnlink(diskfile) < 0) - VIR_WARN("Unable to remove just-created %s", disk->src); + VIR_WARN("Unable to remove just-created %s", disk->src.path); /* Update vm in place to match changes. */ - VIR_FREE(disk->src); - disk->src = source; + VIR_FREE(disk->src.path); + disk->src.path = source; source = NULL; - disk->format = origdisk->format; - disk->type = origdisk->type; - disk->protocol = origdisk->protocol; - virDomainDiskHostDefFree(disk->nhosts, disk->hosts); - disk->nhosts = origdisk->nhosts; - disk->hosts = virDomainDiskHostDefCopy(origdisk->nhosts, origdisk->hosts); + disk->src.format = origdisk->src.format; + disk->src.type = origdisk->src.type; + disk->src.protocol = origdisk->src.protocol; + virDomainDiskHostDefFree(disk->src.nhosts, disk->src.hosts); + disk->src.nhosts = origdisk->src.nhosts; + disk->src.hosts = virDomainDiskHostDefCopy(origdisk->src.nhosts, + origdisk->src.hosts); if (persistDisk) { - VIR_FREE(persistDisk->src); - persistDisk->src = persistSource; + VIR_FREE(persistDisk->src.path); + persistDisk->src.path = persistSource; persistSource = NULL; - persistDisk->format = origdisk->format; - persistDisk->type = origdisk->type; - persistDisk->protocol = origdisk->protocol; - virDomainDiskHostDefFree(persistDisk->nhosts, persistDisk->hosts); - persistDisk->nhosts = origdisk->nhosts; - persistDisk->hosts = virDomainDiskHostDefCopy(origdisk->nhosts, origdisk->hosts); + persistDisk->src.format = origdisk->src.format; + persistDisk->src.type = origdisk->src.type; + persistDisk->src.protocol = origdisk->src.protocol; + virDomainDiskHostDefFree(persistDisk->src.nhosts, + persistDisk->src.hosts); + persistDisk->src.nhosts = origdisk->src.nhosts; + persistDisk->src.hosts = virDomainDiskHostDefCopy(origdisk->src.nhosts, + origdisk->src.hosts); } cleanup: @@ -14705,15 +14710,15 @@ qemuDomainBlockPivot(virConnectPtr conn, * label the entire chain. This action is safe even if the * backing chain has already been labeled; but only necessary when * we know for sure that there is a backing chain. */ - oldsrc = disk->src; - oldformat = disk->format; + oldsrc = disk->src.path; + oldformat = disk->src.format; oldchain = disk->backingChain; - disk->src = disk->mirror; - disk->format = disk->mirrorFormat; + disk->src.path = disk->mirror; + disk->src.format = disk->mirrorFormat; disk->backingChain = NULL; if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) { - disk->src = oldsrc; - disk->format = oldformat; + disk->src.path = oldsrc; + disk->src.format = oldformat; disk->backingChain = oldchain; goto cleanup; } @@ -14723,8 +14728,8 @@ qemuDomainBlockPivot(virConnectPtr conn, qemuSetupDiskCgroup(vm, disk) < 0 || virSecurityManagerSetImageLabel(driver->securityManager, vm->def, disk) < 0)) { - disk->src = oldsrc; - disk->format = oldformat; + disk->src.path = oldsrc; + disk->src.format = oldformat; disk->backingChain = oldchain; goto cleanup; } @@ -14754,8 +14759,8 @@ qemuDomainBlockPivot(virConnectPtr conn, * 'query-block', to see what state we really got left in * before killing the mirroring job? And just as on the * success case, there's security labeling to worry about. */ - disk->src = oldsrc; - disk->format = oldformat; + disk->src.path = oldsrc; + disk->src.format = oldformat; virStorageFileFreeMetadata(disk->backingChain); disk->backingChain = oldchain; VIR_FREE(disk->mirror); @@ -14894,7 +14899,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, if (!async) { int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; int status = VIR_DOMAIN_BLOCK_JOB_CANCELED; - event = virDomainEventBlockJobNewFromObj(vm, disk->src, type, + event = virDomainEventBlockJobNewFromObj(vm, disk->src.path, type, status); } else if (!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) { while (1) { @@ -15103,7 +15108,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm, goto endjob; VIR_FORCE_CLOSE(fd); if (!format) - disk->mirrorFormat = disk->format; + disk->mirrorFormat = disk->src.format; } else if (format) { disk->mirrorFormat = virStorageFileFormatTypeFromString(format); if (disk->mirrorFormat <= 0) { @@ -15263,7 +15268,7 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, goto endjob; disk = vm->def->disks[idx]; - if (!disk->src) { + if (!disk->src.path) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk %s has no source file to be committed"), disk->dst); @@ -15273,10 +15278,10 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, goto endjob; if (!top) { - top_canon = disk->src; + top_canon = disk->src.path; top_meta = disk->backingChain; } else if (!(top_canon = virStorageFileChainLookup(disk->backingChain, - disk->src, + disk->src.path, top, &top_meta, &top_parent))) { virReportError(VIR_ERR_INVALID_ARG, @@ -15322,7 +15327,7 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, clean_access = true; if (qemuDomainPrepareDiskChainElement(driver, vm, disk, base_canon, VIR_DISK_CHAIN_READ_WRITE) < 0 || - (top_parent && top_parent != disk->src && + (top_parent && top_parent != disk->src.path && qemuDomainPrepareDiskChainElement(driver, vm, disk, top_parent, VIR_DISK_CHAIN_READ_WRITE) < 0)) @@ -15344,7 +15349,7 @@ endjob: /* Revert access to read-only, if possible. */ qemuDomainPrepareDiskChainElement(driver, vm, disk, base_canon, VIR_DISK_CHAIN_READ_ONLY); - if (top_parent && top_parent != disk->src) + if (top_parent && top_parent != disk->src.path) qemuDomainPrepareDiskChainElement(driver, vm, disk, top_parent, VIR_DISK_CHAIN_READ_ONLY); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0a243c8..2b06150 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1542,8 +1542,8 @@ qemuMigrationIsSafe(virDomainDefPtr def) return false; else if (rc == 1) continue; - } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK && - disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + } else if (disk->src.type == VIR_DOMAIN_DISK_TYPE_NETWORK && + disk->src.protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { continue; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 978e7b4..6e214b9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -421,13 +421,13 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn, int ret = -1; virStorageEncryptionPtr enc; - if (!disk->encryption) { + if (!disk->src.encryption) { virReportError(VIR_ERR_INTERNAL_ERROR, _("disk %s does not have any encryption information"), - disk->src); + disk->src.path); return -1; } - enc = disk->encryption; + enc = disk->src.encryption; if (!conn) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2193,7 +2193,7 @@ qemuProcessInitPasswords(virConnectPtr conn, size_t secretLen; const char *alias; - if (!vm->def->disks[i]->encryption || + if (!vm->def->disks[i]->src.encryption || !virDomainDiskGetSource(vm->def->disks[i])) continue; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 32f6c7d..434abfe 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1237,7 +1237,7 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, if (!disk_seclabel) return -1; disk_seclabel->labelskip = true; - if (VIR_APPEND_ELEMENT(disk->seclabels, disk->nseclabels, + if (VIR_APPEND_ELEMENT(disk->src.seclabels, disk->src.nseclabels, disk_seclabel) < 0) { virSecurityDeviceLabelDefFree(disk_seclabel); return -1; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6a2840c..d6edf60 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2765,10 +2765,10 @@ virStorageFilePtr virStorageFileInitFromDiskDef(virDomainDiskDefPtr disk) { return virStorageFileInitInternal(virDomainDiskGetActualType(disk), - disk->src, - disk->protocol, - disk->nhosts, - disk->hosts); + disk->src.path, + disk->src.protocol, + disk->src.nhosts, + disk->src.hosts); } diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index d52f9f9..e6a0c29 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2013 Red Hat, Inc. + * Copyright (C) 2011-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -169,11 +169,11 @@ testSELinuxLoadDef(const char *testname) goto cleanup; for (i = 0; i < def->ndisks; i++) { - if (def->disks[i]->type != VIR_DOMAIN_DISK_TYPE_FILE && - def->disks[i]->type != VIR_DOMAIN_DISK_TYPE_BLOCK) + if (def->disks[i]->src.type != VIR_DOMAIN_DISK_TYPE_FILE && + def->disks[i]->src.type != VIR_DOMAIN_DISK_TYPE_BLOCK) continue; - if (testSELinuxMungePath(&def->disks[i]->src) < 0) + if (testSELinuxMungePath(&def->disks[i]->src.path) < 0) goto cleanup; } -- 1.8.5.3

On 03/19/2014 11:20 AM, Eric Blake wrote:
It's finally time to start tracking disk backing chains in <domain> XML. The first step is to start refactoring code so that we have an object more convenient for representing each host source resource in the context of a single guest <disk>. Ultimately, I plan to move the new type into src/util where it can be reused by virStorageFile, but to make the transition easier to review, this patch just creates the new type then fixes everything until it compiles again.
Okay, ACK to this too. It appears to be completely mechanical changes, so the compiler is just as good a reviewer as I am (I examined to see if there was anything that looked like more than using a different name for something (didn't see anything), and compiled successfully. So the entire series is fine except the leaks in 15/18.

On 03/21/2014 04:45 PM, Laine Stump wrote:
On 03/19/2014 11:20 AM, Eric Blake wrote:
It's finally time to start tracking disk backing chains in <domain> XML. The first step is to start refactoring code so that we have an object more convenient for representing each host source resource in the context of a single guest <disk>. Ultimately, I plan to move the new type into src/util where it can be reused by virStorageFile, but to make the transition easier to review, this patch just creates the new type then fixes everything until it compiles again.
Okay, ACK to this too. It appears to be completely mechanical changes, so the compiler is just as good a reviewer as I am (I examined to see if there was anything that looked like more than using a different name for something (didn't see anything), and compiled successfully.
So the entire series is fine except the leaks in 15/18.
Thanks; I've pushed the series now with those fixes included. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/19/2014 11:20 AM, Eric Blake wrote:
I still have quite a bit of work to consolidate util/virStorageFile, conf/snapshot_conf.c, and the new struct in this series to all share the same common struct (basically, moving the struct created in this series over to util). But by refactoring the most common accesses to go through functions instead of direct field access, I've limited the amount of code that is actually impacted by using the new struct. In general, this series should not be changing any behavior.
In case I don't get finished with the last patch today: ACK on 01 through 17, except for the two potential memory leaks in patch 15. I'm going through 18 right now.
participants (3)
-
Eric Blake
-
Laine Stump
-
Roman Bogorodskiy