From 7b7bf001108b03c3b8983ca922f0e7727953910f Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 21 May 2014 14:22:21 -0600 Subject: [PATCH] conf: store mirroring information in virStorageSource The current implementation of 'virsh blockcopy' (virDomainBlockRebase) is limited to copying to a local file name. But future patches want to extend it to also copy to network disks. This patch converts over to a virStorageSourcePtr, although it should have no semantic change visible to the user, in anticipation of those future patches being able to use more fields for non-file destinations. * src/conf/domain_conf.h (_virDomainDiskDef): Change type of mirror information. * src/conf/domain_conf.c (virDomainDiskDefParseXML): Localize mirror parsing into new object. (virDomainDiskDefFormat): Adjust clients. * src/qemu/qemu_domain.c (qemuDomainDeviceDefPostParse): Likewise. * src/qemu/qemu_driver.c (qemuDomainBlockPivot) (qemuDomainBlockJobImpl, qemuDomainBlockCopy): Likewise. Signed-off-by: Eric Blake --- src/conf/domain_conf.c | 52 +++++++++++++++++------------------ src/conf/domain_conf.h | 3 +-- src/qemu/qemu_domain.c | 8 +++--- src/qemu/qemu_driver.c | 60 ++++++++++++++++++++++++++--------------- src/qemu/qemu_process.c | 15 ++++++----- 5 files changed, 78 insertions(+), 60 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4efdcc7f48..9c6c201ac0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1203,7 +1203,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def) virStorageSourceFree(def->src); VIR_FREE(def->serial); VIR_FREE(def->dst); - VIR_FREE(def->mirror); + virStorageSourceFree(def->mirror); VIR_FREE(def->wwn); VIR_FREE(def->vendor); VIR_FREE(def->product); @@ -5223,9 +5223,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, char *ioeventfd = NULL; char *event_idx = NULL; char *copy_on_read = NULL; - char *mirror = NULL; - char *mirrorFormat = NULL; - bool mirroring = false; char *devaddr = NULL; virStorageEncryptionPtr encryption = NULL; char *serial = NULL; @@ -5374,19 +5371,37 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, event_idx = virXMLPropString(cur, "event_idx"); copy_on_read = virXMLPropString(cur, "copy_on_read"); discard = virXMLPropString(cur, "discard"); - } else if (!mirror && xmlStrEqual(cur->name, BAD_CAST "mirror") && + } else if (!def->mirror && + xmlStrEqual(cur->name, BAD_CAST "mirror") && !(flags & VIR_DOMAIN_XML_INACTIVE)) { char *ready; - mirror = virXMLPropString(cur, "file"); - if (!mirror) { + char *mirrorFormat; + + if (VIR_ALLOC(def->mirror) < 0) + goto error; + def->mirror->type = VIR_STORAGE_TYPE_FILE; + def->mirror->path = virXMLPropString(cur, "file"); + if (!def->mirror->path) { virReportError(VIR_ERR_XML_ERROR, "%s", _("mirror requires file name")); goto error; } mirrorFormat = virXMLPropString(cur, "format"); + if (mirrorFormat) { + def->mirror->format = + virStorageFileFormatTypeFromString(mirrorFormat); + if (def->mirror->format <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown mirror format value '%s'"), + mirrorFormat); + VIR_FREE(mirrorFormat); + goto error; + } + VIR_FREE(mirrorFormat); + } ready = virXMLPropString(cur, "ready"); if (ready) { - mirroring = true; + def->mirroring = true; VIR_FREE(ready); } } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) { @@ -5906,9 +5921,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, authUsername = NULL; def->src->driverName = driverName; driverName = NULL; - def->mirror = mirror; - mirror = NULL; - def->mirroring = mirroring; def->src->encryption = encryption; encryption = NULL; def->serial = serial; @@ -5930,16 +5942,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } - if (mirrorFormat) { - def->mirrorFormat = virStorageFileFormatTypeFromString(mirrorFormat); - if (def->mirrorFormat <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown mirror format value '%s'"), - driverType); - goto error; - } - } - if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, def) < 0) goto error; @@ -5964,8 +5966,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(authUsage); VIR_FREE(driverType); VIR_FREE(driverName); - VIR_FREE(mirror); - VIR_FREE(mirrorFormat); VIR_FREE(cachetag); VIR_FREE(error_policy); VIR_FREE(rerror_policy); @@ -15160,10 +15160,10 @@ virDomainDiskDefFormat(virBufferPtr buf, * for live domains, therefore we ignore it on input except for * the internal parse on libvirtd restart. */ if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) { - virBufferEscapeString(buf, "mirror); - if (def->mirrorFormat) + virBufferEscapeString(buf, "mirror->path); + if (def->mirror->format) virBufferAsprintf(buf, " format='%s'", - virStorageFileFormatTypeToString(def->mirrorFormat)); + virStorageFileFormatTypeToString(def->mirror->format)); if (def->mirroring) virBufferAddLit(buf, " ready='yes'"); virBufferAddLit(buf, "/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3524fb1d9d..a6ac95a2f8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -606,8 +606,7 @@ struct _virDomainDiskDef { int tray_status; /* enum virDomainDiskTray */ int removable; /* enum virDomainFeatureState */ - char *mirror; - int mirrorFormat; /* virStorageFileFormat */ + virStorageSourcePtr mirror; bool mirroring; struct { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ccdfaceba6..962698b0aa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -887,8 +887,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, /* default disk format for mirrored drive */ if (disk->mirror && - disk->mirrorFormat == VIR_STORAGE_FILE_NONE) - disk->mirrorFormat = VIR_STORAGE_FILE_AUTO; + disk->mirror->format == VIR_STORAGE_FILE_NONE) + disk->mirror->format = VIR_STORAGE_FILE_AUTO; } else { /* default driver if probing is forbidden */ if (!virDomainDiskGetDriver(disk) && @@ -903,8 +903,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, /* default disk format for mirrored drive */ if (disk->mirror && - disk->mirrorFormat == VIR_STORAGE_FILE_NONE) - disk->mirrorFormat = VIR_STORAGE_FILE_RAW; + disk->mirror->format == VIR_STORAGE_FILE_NONE) + disk->mirror->format = VIR_STORAGE_FILE_RAW; } } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0e44afbd16..5d4023933d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14864,13 +14864,22 @@ qemuDomainBlockPivot(virConnectPtr conn, int ret = -1, rc; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainBlockJobInfo info; - const char *format = virStorageFileFormatTypeToString(disk->mirrorFormat); + const char *format = NULL; bool resume = false; char *oldsrc = NULL; int oldformat; virStorageSourcePtr oldchain = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + if (!disk->mirror) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("pivot of disk '%s' requires an active copy job"), + disk->dst); + goto cleanup; + } + + format = virStorageFileFormatTypeToString(disk->mirror->format); + /* Probe the status, if needed. */ if (!disk->mirroring) { qemuDomainObjEnterMonitor(driver, vm); @@ -14928,8 +14937,8 @@ qemuDomainBlockPivot(virConnectPtr conn, oldsrc = disk->src->path; oldformat = disk->src->format; oldchain = disk->src->backingStore; - disk->src->path = disk->mirror; - disk->src->format = disk->mirrorFormat; + disk->src->path = disk->mirror->path; + disk->src->format = disk->mirror->format; disk->src->backingStore = NULL; if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) { disk->src->path = oldsrc; @@ -14937,7 +14946,7 @@ qemuDomainBlockPivot(virConnectPtr conn, disk->src->backingStore = oldchain; goto cleanup; } - if (disk->mirrorFormat && disk->mirrorFormat != VIR_STORAGE_FILE_RAW && + if (disk->mirror->format && disk->mirror->format != VIR_STORAGE_FILE_RAW && (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm, disk) < 0 || qemuSetupDiskCgroup(vm, disk) < 0 || @@ -14951,7 +14960,7 @@ qemuDomainBlockPivot(virConnectPtr conn, /* Attempt the pivot. */ qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror, format); + ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror->path, format); qemuDomainObjExitMonitor(driver, vm); if (ret == 0) { @@ -14964,7 +14973,7 @@ qemuDomainBlockPivot(virConnectPtr conn, * for now, we leak the access to the original. */ VIR_FREE(oldsrc); virStorageSourceFree(oldchain); - disk->mirror = NULL; + disk->mirror->path = NULL; } else { /* On failure, qemu abandons the mirror, and reverts back to * the source disk (RHEL 6.3 has a bug where the revert could @@ -14978,9 +14987,9 @@ qemuDomainBlockPivot(virConnectPtr conn, disk->src->format = oldformat; virStorageSourceFree(disk->src->backingStore); disk->src->backingStore = oldchain; - VIR_FREE(disk->mirror); } - disk->mirrorFormat = VIR_STORAGE_FILE_NONE; + virStorageSourceFree(disk->mirror); + disk->mirror = NULL; disk->mirroring = false; cleanup: @@ -15106,8 +15115,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, if (mode == BLOCK_JOB_ABORT && disk->mirror) { /* XXX We should also revoke security labels and disk lease on * the mirror, and audit that fact, before dropping things. */ - VIR_FREE(disk->mirror); - disk->mirrorFormat = VIR_STORAGE_FILE_NONE; + virStorageSourceFree(disk->mirror); + disk->mirror = NULL; disk->mirroring = false; } @@ -15242,7 +15251,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm, int idx; struct stat st; bool need_unlink = false; - char *mirror = NULL; + virStorageSourcePtr mirror = NULL; virQEMUDriverConfigPtr cfg = NULL; /* Preliminaries: find the disk we are editing, sanity checks */ @@ -15323,6 +15332,11 @@ qemuDomainBlockCopy(virDomainObjPtr vm, goto endjob; } + if (VIR_ALLOC(mirror) < 0) + goto endjob; + /* XXX Allow non-file mirror destinations */ + mirror->type = VIR_STORAGE_TYPE_FILE; + if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) { int fd = qemuOpenFile(driver, vm, dest, O_WRONLY | O_TRUNC | O_CREAT, &need_unlink, NULL); @@ -15330,10 +15344,10 @@ qemuDomainBlockCopy(virDomainObjPtr vm, goto endjob; VIR_FORCE_CLOSE(fd); if (!format) - disk->mirrorFormat = disk->src->format; + mirror->format = disk->src->format; } else if (format) { - disk->mirrorFormat = virStorageFileFormatTypeFromString(format); - if (disk->mirrorFormat <= 0) { + mirror->format = virStorageFileFormatTypeFromString(format); + if (mirror->format <= 0) { virReportError(VIR_ERR_INVALID_ARG, _("unrecognized format '%s'"), format); goto endjob; @@ -15343,12 +15357,12 @@ qemuDomainBlockCopy(virDomainObjPtr vm, * also passed the RAW flag (and format is non-NULL), or it is * safe for us to probe the format from the file that we will * be using. */ - disk->mirrorFormat = virStorageFileProbeFormat(dest, cfg->user, - cfg->group); + mirror->format = virStorageFileProbeFormat(dest, cfg->user, + cfg->group); } - if (!format && disk->mirrorFormat > 0) - format = virStorageFileFormatTypeToString(disk->mirrorFormat); - if (VIR_STRDUP(mirror, dest) < 0) + if (!format && mirror->format > 0) + format = virStorageFileFormatTypeToString(mirror->format); + if (VIR_STRDUP(mirror->path, dest) < 0) goto endjob; if (qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest, @@ -15378,9 +15392,11 @@ qemuDomainBlockCopy(virDomainObjPtr vm, endjob: if (need_unlink && unlink(dest)) VIR_WARN("unable to unlink just-created %s", dest); - if (ret < 0 && disk) - disk->mirrorFormat = VIR_STORAGE_FILE_NONE; - VIR_FREE(mirror); + if (ret < 0 && disk) { + virStorageSourceFree(disk->mirror); + disk->mirror = NULL; + } + virStorageSourceFree(mirror); if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d906494f55..e4845ba264 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1033,12 +1033,15 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT) && status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) qemuDomainDetermineDiskChain(driver, vm, disk, true); - if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY && - status == VIR_DOMAIN_BLOCK_JOB_READY) - disk->mirroring = true; - if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY && - status == VIR_DOMAIN_BLOCK_JOB_FAILED) - VIR_FREE(disk->mirror); + if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) { + if (status == VIR_DOMAIN_BLOCK_JOB_READY) { + disk->mirroring = true; + } else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virStorageSourceFree(disk->mirror); + disk->mirror = NULL; + disk->mirroring = false; + } + } } virObjectUnlock(vm);