3:06 p.m.
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