On 09/05/2014 08:09 AM, Peter Krempa wrote:
On 08/31/14 06:02, Eric Blake wrote:
In order to implement the new virDomainBlockCopy, the existing block copy internal implementation needs to be adjusted. The new function will parse XML into a storage source, and parse typed parameters into integers, then call into the same common backend. For now, it's easier to keep the same implementation limits that only local file destinations are suported, but now the check needs to be explicit.
/* bandwidth in bytes/s */ static int -qemuDomainBlockCopy(virDomainObjPtr vm, - virConnectPtr conn, - const char *path, - const char *dest, const char *format, - unsigned long long bandwidth, unsigned int flags)
You should mention that this function consumes @dest.
+qemuDomainBlockCopyCommon(virDomainObjPtr vm,
- if (VIR_ALLOC(mirror) < 0) + if (!(mirror = virStorageSourceCopy(dest, false)))
Also you won't need to copy it then.
ACK with the docs added. The tweaks of the @dest handling are not necessary.
How about the following interdiff? diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 7026a18..26e0237 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -15349,12 +15349,13 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, /* bandwidth in bytes/s. Caller must lock vm beforehand, and not - * access it afterwards. */ + * access it afterwards; likewise, caller must not access mirror + * afterwards. */ static int qemuDomainBlockCopyCommon(virDomainObjPtr vm, virConnectPtr conn, const char *path, - virStorageSourcePtr dest, + virStorageSourcePtr mirror, unsigned long long bandwidth, unsigned int flags) { @@ -15366,10 +15367,9 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, int idx; struct stat st; bool need_unlink = false; - virStorageSourcePtr mirror = NULL; virQEMUDriverConfigPtr cfg = NULL; const char *format = NULL; - int desttype = virStorageSourceGetActualType(dest); + int desttype = virStorageSourceGetActualType(mirror); /* Preliminaries: find the disk we are editing, sanity checks */ virCheckFlags(VIR_DOMAIN_BLOCK_COPY_SHALLOW | @@ -15419,7 +15419,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, goto endjob; if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) && - dest->format == VIR_STORAGE_FILE_RAW && + mirror->format == VIR_STORAGE_FILE_RAW && disk->src->backingStore->path) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk '%s' has backing file, so raw shallow copy " @@ -15430,20 +15430,20 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, /* Prepare the destination file. */ /* XXX Allow non-file mirror destinations */ - if (!virStorageSourceIsLocalStorage(dest)) { + if (!virStorageSourceIsLocalStorage(mirror)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("non-file destination not supported yet")); } - if (stat(dest->path, &st) < 0) { + if (stat(mirror->path, &st) < 0) { if (errno != ENOENT) { virReportSystemError(errno, _("unable to stat for disk %s: %s"), - disk->dst, dest->path); + disk->dst, mirror->path); goto endjob; } else if (flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT || desttype == VIR_STORAGE_TYPE_BLOCK) { virReportSystemError(errno, _("missing destination file for disk %s: %s"), - disk->dst, dest->path); + disk->dst, mirror->path); goto endjob; } } else if (!S_ISBLK(st.st_mode)) { @@ -15451,22 +15451,19 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("external destination file for disk %s already " "exists and is not a block device: %s"), - disk->dst, dest->path); + disk->dst, mirror->path); goto endjob; } if (desttype == VIR_STORAGE_TYPE_BLOCK) { virReportError(VIR_ERR_INVALID_ARG, _("blockdev flag requested for disk %s, but file " "'%s' is not a block device"), - disk->dst, dest->path); + disk->dst, mirror->path); goto endjob; } } - if (!(mirror = virStorageSourceCopy(dest, false))) - goto endjob; - - if (!dest->format) { + if (!mirror->format) { if (!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT)) { mirror->format = disk->src->format; } else { @@ -15474,14 +15471,14 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, * can also pass the RAW flag or use XML to tell us the format. * So if we get here, we assume it is safe for us to probe the * format from the file that we will be using. */ - mirror->format = virStorageFileProbeFormat(dest->path, cfg->user, + mirror->format = virStorageFileProbeFormat(mirror->path, cfg->user, cfg->group); } } /* pre-create the image file */ if (!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT)) { - int fd = qemuOpenFile(driver, vm, dest->path, + int fd = qemuOpenFile(driver, vm, mirror->path, O_WRONLY | O_TRUNC | O_CREAT, &need_unlink, NULL); if (fd < 0) @@ -15504,7 +15501,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, /* Actually start the mirroring */ qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorDriveMirror(priv->mon, device, dest->path, format, + ret = qemuMonitorDriveMirror(priv->mon, device, mirror->path, format, bandwidth, flags); virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0); qemuDomainObjExitMonitor(driver, vm); @@ -15525,8 +15522,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, vm->def->name); endjob: - if (need_unlink && unlink(dest->path)) - VIR_WARN("unable to unlink just-created %s", dest->path); + if (need_unlink && unlink(mirror->path)) + VIR_WARN("unable to unlink just-created %s", mirror->path); virStorageSourceFree(mirror); if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; @@ -15603,6 +15600,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, ret = qemuDomainBlockCopyCommon(vm, dom->conn, path, dest, bandwidth, flags); vm = NULL; + dest = NULL; cleanup: if (vm) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org