Rename qemuDomainObjPrivateXMLFormatBlockjobFormatChain to qemuDomainObjPrivateXMLFormatBlockjobFormatSource and add a 'chain' parameter which allows controlling whether the backing chain is formatted. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0555caa6ab..e1da0661e6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2315,10 +2315,11 @@ typedef struct qemuDomainPrivateBlockJobFormatData { static int -qemuDomainObjPrivateXMLFormatBlockjobFormatChain(virBufferPtr buf, - const char *chainname, - virStorageSourcePtr src, - virDomainXMLOptionPtr xmlopt) +qemuDomainObjPrivateXMLFormatBlockjobFormatSource(virBufferPtr buf, + const char *element, + virStorageSourcePtr src, + virDomainXMLOptionPtr xmlopt, + bool chain) { VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; @@ -2333,10 +2334,11 @@ qemuDomainObjPrivateXMLFormatBlockjobFormatChain(virBufferPtr buf, if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, true, xmlflags, xmlopt) < 0) return -1; - if (virDomainDiskBackingStoreFormat(&childBuf, src, xmlopt, xmlflags) < 0) + if (chain && + virDomainDiskBackingStoreFormat(&childBuf, src, xmlopt, xmlflags) < 0) return -1; - if (virXMLFormatElement(buf, chainname, &attrBuf, &childBuf) < 0) + if (virXMLFormatElement(buf, element, &attrBuf, &childBuf) < 0) return -1; return 0; @@ -2375,17 +2377,19 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, virBufferAddLit(&childBuf, "/>\n"); } else { if (job->chain && - qemuDomainObjPrivateXMLFormatBlockjobFormatChain(&chainsBuf, - "disk", - job->chain, - data->xmlopt) < 0) + qemuDomainObjPrivateXMLFormatBlockjobFormatSource(&chainsBuf, + "disk", + job->chain, + data->xmlopt, + true) < 0) return -1; if (job->mirrorChain && - qemuDomainObjPrivateXMLFormatBlockjobFormatChain(&chainsBuf, - "mirror", - job->mirrorChain, - data->xmlopt) < 0) + qemuDomainObjPrivateXMLFormatBlockjobFormatSource(&chainsBuf, + "mirror", + job->mirrorChain, + data->xmlopt, + true) < 0) return -1; if (virXMLFormatElement(&childBuf, "chains", NULL, &chainsBuf) < 0) -- 2.21.0

Allow reusing original backing chain when doing a shallow copy without reuse of external image. The existing logic didn't allow it but it will be possible. Also add a note to explain that logic. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ff83d1c024..3e0aa1b90f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18263,19 +18263,23 @@ qemuDomainBlockCopyCommonValidateUserMirrorBackingStore(virStorageSourcePtr mirr { /* note that if original disk does not have backing chain, shallow is cleared */ bool shallow = flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW; - bool reuse = flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT; - if (!mirror->backingStore) { - /* deep copy won't need backing store so we can terminate it */ - if (!shallow && + if (!virStorageSourceHasBacking(mirror)) { + /* for deep copy there won't be backing chain so we can terminate it */ + if (!mirror->backingStore && + !shallow && !(mirror->backingStore = virStorageSourceNew())) return -1; - return 0; - } - - /* validate user provided backing store */ - if (virStorageSourceHasBacking(mirror)) { + /* When reusing an external image we document that the user must ensure + * that the <mirror> image must expose data as the original image did + * either by providing correct chain or prepopulating the image. This + * means we can't validate this any more regardless of whether shallow + * copy is requested. + * + * For a copy when we are not reusing external image requesting shallow + * is okay and will inherit the original backing chain */ + } else { if (!blockdev) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("backingStore of mirror target is not supported by this qemu")); @@ -18287,13 +18291,6 @@ qemuDomainBlockCopyCommonValidateUserMirrorBackingStore(virStorageSourcePtr mirr _("backingStore of mirror without VIR_DOMAIN_BLOCK_COPY_SHALLOW doesn't make sense")); return -1; } - } else { - /* shallow copy without reuse requires some kind of backing data */ - if (!reuse && shallow) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("VIR_DOMAIN_BLOCK_COPY_SHALLOW implies backing chain for mirror")); - return -1; - } } return 0; -- 2.21.0

Commit 16ca234b56fac82 refactored how the 'shallow' and 'reuse' flags are accessed but neglected to fix the clearing of 'shallow' in case when the disk has no backing chain. This means that we'd request a shallow copy even without backing chain and also a few checks would work wrong. Fix it by using the extracted variable everywhere. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3e0aa1b90f..2c03cc5dff 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18258,12 +18258,9 @@ qemuDomainBlockCopyValidateMirror(virStorageSourcePtr mirror, */ static int qemuDomainBlockCopyCommonValidateUserMirrorBackingStore(virStorageSourcePtr mirror, - unsigned int flags, + bool shallow, bool blockdev) { - /* note that if original disk does not have backing chain, shallow is cleared */ - bool shallow = flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW; - if (!virStorageSourceHasBacking(mirror)) { /* for deep copy there won't be backing chain so we can terminate it */ if (!mirror->backingStore && @@ -18376,9 +18373,10 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, /* clear the _SHALLOW flag if there is only one layer */ if (!virStorageSourceHasBacking(disk->src)) - flags &= ~VIR_DOMAIN_BLOCK_COPY_SHALLOW; + mirror_shallow = false; - if (qemuDomainBlockCopyCommonValidateUserMirrorBackingStore(mirror, flags, + if (qemuDomainBlockCopyCommonValidateUserMirrorBackingStore(mirror, + mirror_shallow, blockdev) < 0) goto endjob; -- 2.21.0

Modify the return value so that callers don't have to repeat logic. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a6de6a1e45..5882d470de 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4411,6 +4411,14 @@ virStorageFileIsInitialized(const virStorageSource *src) } +/** + * virStorageFileGetBackendForSupportCheck: + * @src: storage source to check support for + * @backend: pointer to the storage backend for @src if it's supported + * + * Returns 0 if @src is not supported by any storage backend currently linked + * 1 if it is supported and -1 on error with an error reported. + */ static int virStorageFileGetBackendForSupportCheck(const virStorageSource *src, virStorageFileBackendPtr *backend) @@ -4425,7 +4433,7 @@ virStorageFileGetBackendForSupportCheck(const virStorageSource *src, if (src->drv) { *backend = src->drv->backend; - return 0; + return 1; } actualType = virStorageSourceGetActualType(src); @@ -4433,7 +4441,10 @@ virStorageFileGetBackendForSupportCheck(const virStorageSource *src, if (virStorageFileBackendForType(actualType, src->protocol, false, backend) < 0) return -1; - return 0; + if (!*backend) + return 0; + + return 1; } @@ -4443,12 +4454,8 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) virStorageFileBackendPtr backend; int rv; - rv = virStorageFileGetBackendForSupportCheck(src, &backend); - if (rv < 0) - return -1; - - if (!backend) - return 0; + if ((rv = virStorageFileGetBackendForSupportCheck(src, &backend)) < 1) + return rv; return backend->storageFileGetUniqueIdentifier && backend->storageFileRead && @@ -4470,11 +4477,8 @@ virStorageFileSupportsSecurityDriver(const virStorageSource *src) virStorageFileBackendPtr backend; int rv; - rv = virStorageFileGetBackendForSupportCheck(src, &backend); - if (rv < 0) - return -1; - if (backend == NULL) - return 0; + if ((rv = virStorageFileGetBackendForSupportCheck(src, &backend)) < 1) + return rv; return backend->storageFileChown ? 1 : 0; } @@ -4492,13 +4496,10 @@ int virStorageFileSupportsAccess(const virStorageSource *src) { virStorageFileBackendPtr backend; - int ret; + int rv; - ret = virStorageFileGetBackendForSupportCheck(src, &backend); - if (ret < 0) - return -1; - if (backend == NULL) - return 0; + if ((rv = virStorageFileGetBackendForSupportCheck(src, &backend)) < 1) + return rv; return backend->storageFileAccess ? 1 : 0; } -- 2.21.0

Add virStorageFileSupportsCreate which allows silent check whether virStorageFileCreate is implemented. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 20 ++++++++++++++++++++ src/util/virstoragefile.h | 1 + 3 files changed, 22 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cae8febf8d..b86868e954 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2995,6 +2995,7 @@ virStorageFileReportBrokenChain; virStorageFileResize; virStorageFileStat; virStorageFileSupportsAccess; +virStorageFileSupportsCreate; virStorageFileSupportsSecurityDriver; virStorageFileUnlink; virStorageIsFile; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 5882d470de..ba56f452e9 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4505,6 +4505,26 @@ virStorageFileSupportsAccess(const virStorageSource *src) } +/** + * virStorageFileSupportsCreate: + * @src: a storage file structure + * + * Check if the storage driver supports creating storage described by @src + * via virStorageFileCreate. + */ +int +virStorageFileSupportsCreate(const virStorageSource *src) +{ + virStorageFileBackendPtr backend; + int rv; + + if ((rv = virStorageFileGetBackendForSupportCheck(src, &backend)) < 1) + return rv; + + return backend->storageFileCreate ? 1 : 0; +} + + void virStorageFileDeinit(virStorageSourcePtr src) { diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 38ba901858..2882bacf3e 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -532,6 +532,7 @@ int virStorageFileChown(const virStorageSource *src, uid_t uid, gid_t gid); int virStorageFileSupportsSecurityDriver(const virStorageSource *src); int virStorageFileSupportsAccess(const virStorageSource *src); +int virStorageFileSupportsCreate(const virStorageSource *src); int virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, -- 2.21.0

The utility of the function is extremely limited as for block copy we need to register the mirror chain earlier than when it's set with the disk. This means that it would be open-coded in that case. Avoid any weird usage and just open-code the only current usage, remove the function, and reword the docs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 20 +------------------- src/qemu/qemu_blockjob.h | 4 ---- src/qemu/qemu_domain.c | 2 +- 3 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index a5b558b9ab..8303567aed 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -129,7 +129,7 @@ qemuBlockJobDataNew(qemuBlockJobType type, * xml (if @savestatus is true). * * Note that if @job also references a separate chain e.g. for disk mirroring, - * then qemuBlockJobDiskRegisterMirror should be used separately. + * then job->mirrorchain needs to be set manually. */ int qemuBlockJobRegister(qemuBlockJobDataPtr job, @@ -274,24 +274,6 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm, } -/** - * qemuBlockJobDiskRegisterMirror: - * @job: block job to register 'mirror' chain on - * - * In cases when the disk->mirror attribute references a separate storage chain - * such as for block-copy, this function registers it with the job. Note - * that this function does not save the status XML and thus must be used before - * qemuBlockJobRegister or qemuBlockJobStarted to properly track the chain - * in the status XML. - */ -void -qemuBlockJobDiskRegisterMirror(qemuBlockJobDataPtr job) -{ - if (job->disk) - job->mirrorChain = virObjectRef(job->disk->mirror); -} - - /** * qemuBlockJobDiskGetJob: * @disk: disk definition diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 8139a1a324..5b740db5a8 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -134,10 +134,6 @@ qemuBlockJobDiskNew(virDomainObjPtr vm, const char *jobname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4); -void -qemuBlockJobDiskRegisterMirror(qemuBlockJobDataPtr job) - ATTRIBUTE_NONNULL(1); - qemuBlockJobDataPtr qemuBlockJobDiskNewPull(virDomainObjPtr vm, virDomainDiskDefPtr disk, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e1da0661e6..50dc4f3764 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2976,7 +2976,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, job->disk = disk; if (mirror) - qemuBlockJobDiskRegisterMirror(job); + job->mirrorChain = virObjectRef(job->disk->mirror); qemuDomainObjPrivateXMLParseBlockjobDataSpecific(job, ctxt); -- 2.21.0

In commit 3f93884a4d0 where the job handling of commit jobs with blockdev was added I've forgot to add a 'break' in the switch fomatting the status XML. Thankfully this would not be a problem as the cases where this fell through didn't have any code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 50dc4f3764..2e722f4ee4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2410,6 +2410,8 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, virBufferAsprintf(&childBuf, "<top node='%s'/>\n", job->data.commit.top->nodeformat); if (job->data.commit.topparent) virBufferAsprintf(&childBuf, "<topparent node='%s'/>\n", job->data.commit.topparent->nodeformat); + break; + case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: -- 2.21.0

The only code path which calls the parser with the VIR_DOMAIN_DEF_PARSE_DISK_SOURCE is from qemuDomainBlockCopy. Since that code path can properly handle backing chains for the disk and it's desired to pass the parsed chains to the block copy code remove the condition which prevents parsing the <backingStore> element. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0456369d55..52a3dd4064 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10293,10 +10293,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_STEAL_PTR(def->vendor, vendor); VIR_STEAL_PTR(def->product, product); - if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) { - if (virDomainDiskBackingStoreParse(ctxt, def->src, flags, xmlopt) < 0) - goto error; - } + if (virDomainDiskBackingStoreParse(ctxt, def->src, flags, xmlopt) < 0) + goto error; if (flags & VIR_DOMAIN_DEF_PARSE_STATUS && virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0) -- 2.21.0

Rather than copying just the top level image, let's copy the full user provided backing chain. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 8303567aed..6ac60e86d7 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -521,6 +521,7 @@ qemuBlockJobRewriteConfigDiskSource(virDomainObjPtr vm, { virDomainDiskDefPtr persistDisk = NULL; VIR_AUTOUNREF(virStorageSourcePtr) copy = NULL; + virStorageSourcePtr n; if (!vm->newDef) return; @@ -531,14 +532,24 @@ qemuBlockJobRewriteConfigDiskSource(virDomainObjPtr vm, if (!virStorageSourceIsSameLocation(disk->src, persistDisk->src)) return; - if (!(copy = virStorageSourceCopy(newsrc, false)) || + if (!(copy = virStorageSourceCopy(newsrc, true)) || virStorageSourceInitChainElement(copy, persistDisk->src, true) < 0) { VIR_WARN("Unable to update persistent definition on vm %s after block job", vm->def->name); return; } - qemuBlockJobCleanStorageSourceRuntime(copy); + for (n = copy; virStorageSourceIsBacking(n); n = n->backingStore) { + qemuBlockJobCleanStorageSourceRuntime(n); + + /* discard any detected backing store */ + if (virStorageSourceIsBacking(n->backingStore) && + n->backingStore->detected) { + virObjectUnref(n->backingStore); + n->backingStore = NULL; + break; + } + } virObjectUnref(persistDisk->src); VIR_STEAL_PTR(persistDisk->src, copy); -- 2.21.0

QEMU finally exposes an interface which allows us to instruct it to format or create arbitrary images. This is required for blockdev integration of block copy and snapshots as we need to pre-format images prior to use with blockdev-add. This path introduces job handling and also helpers for formatting and attaching a whole image described by a virStorageSource. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 250 ++++++++++++++++++ src/qemu/qemu_block.h | 14 + src/qemu/qemu_blockjob.c | 88 +++++- src/qemu/qemu_blockjob.h | 17 ++ src/qemu/qemu_domain.c | 34 ++- src/qemu/qemu_driver.c | 1 + .../blockjob-blockdev-in.xml | 45 ++++ 7 files changed, 446 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 47661fb8bd..cb9a085e5d 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2379,3 +2379,253 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src, return 0; } + + +static int +qemuBlockStorageSourceCreateGeneric(virDomainObjPtr vm, + virJSONValuePtr createProps, + virStorageSourcePtr src, + virStorageSourcePtr chain, + bool storageCreate, + qemuDomainAsyncJob asyncJob) +{ + VIR_AUTOPTR(virJSONValue) props = createProps; + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuBlockJobDataPtr job = NULL; + int ret = -1; + int rc; + + if (!(job = qemuBlockJobNewCreate(vm, src, chain, storageCreate))) + return -1; + + qemuBlockJobSyncBegin(job); + + if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) + goto cleanup; + + rc = qemuMonitorBlockdevCreate(priv->mon, job->name, props); + props = NULL; + + if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || rc < 0) + goto cleanup; + + qemuBlockJobStarted(job, vm); + + qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); + while (qemuBlockJobIsRunning(job)) { + if (virDomainObjWait(vm) < 0) + goto cleanup; + qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); + } + + if (job->state == QEMU_BLOCKJOB_STATE_FAILED || + job->state == QEMU_BLOCKJOB_STATE_CANCELLED) { + if (job->state == QEMU_BLOCKJOB_STATE_CANCELLED && !job->errmsg) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("blockdev-create job was cancelled")); + } else { + virReportError(VIR_ERR_OPERATION_FAILED, + _("failed to format image: '%s'"), NULLSTR(job->errmsg)); + } + goto cleanup; + } + + ret = 0; + + cleanup: + qemuBlockJobStartupFinalize(vm, job); + return ret; +} + + +static int +qemuBlockStorageSourceCreateStorage(virDomainObjPtr vm, + virStorageSourcePtr src, + virStorageSourcePtr chain, + qemuDomainAsyncJob asyncJob) +{ + int actualType = virStorageSourceGetActualType(src); + VIR_AUTOPTR(virJSONValue) createstorageprops = NULL; + int ret; + + /* we need to do stuff only for remote storage and local raw files */ + if (actualType != VIR_STORAGE_TYPE_NETWORK && + !(actualType == VIR_STORAGE_TYPE_FILE && src->format == VIR_STORAGE_FILE_RAW)) + return 0; + + if (qemuBlockStorageSourceCreateGetStorageProps(src, &createstorageprops) < 0) + return -1; + + if (!createstorageprops) { + /* we can always try opening it to see whether it was existing */ + return 0; + } + + ret = qemuBlockStorageSourceCreateGeneric(vm, createstorageprops, src, chain, + true, asyncJob); + createstorageprops = NULL; + + return ret; +} + + +static int +qemuBlockStorageSourceCreateFormat(virDomainObjPtr vm, + virStorageSourcePtr src, + virStorageSourcePtr backingStore, + virStorageSourcePtr chain, + qemuDomainAsyncJob asyncJob) +{ + VIR_AUTOPTR(virJSONValue) createformatprops = NULL; + int ret; + + if (src->format == VIR_STORAGE_FILE_RAW) + return 0; + + if (qemuBlockStorageSourceCreateGetFormatProps(src, backingStore, + &createformatprops) < 0) + return -1; + + if (!createformatprops) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("can't create storage format '%s'"), + virStorageFileFormatTypeToString(src->format)); + return -1; + } + + ret = qemuBlockStorageSourceCreateGeneric(vm, createformatprops, src, chain, + false, asyncJob); + createformatprops = NULL; + + return ret; +} + + +/** + * qemuBlockStorageSourceCreate: + * @vm: domain object + * @src: storage source definition to create + * @backingStore: backingStore of the new image (used only in image metadata) + * @chain: backing chain to unplug in case of a long-running job failure + * @data: qemuBlockStorageSourceAttachData for @src so that it can be attached + * @asyncJob: qemu asynchronous job type + * + * Creates and formats a storage volume according to @src and attaches it to @vm. + * @data must provide attachment data as if @src was existing. @src is attached + * after successful return of this function. If libvirtd is restarted during + * the create job @chain is unplugged, otherwise it's left for the caller. + * If @backingStore is provided, the new image will refer to it as it's backing + * store. + */ +int +qemuBlockStorageSourceCreate(virDomainObjPtr vm, + virStorageSourcePtr src, + virStorageSourcePtr backingStore, + virStorageSourcePtr chain, + qemuBlockStorageSourceAttachDataPtr data, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + int rc; + + if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) + goto cleanup; + + rc = qemuBlockStorageSourceAttachApplyStorageDeps(priv->mon, data); + + if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || rc < 0) + goto cleanup; + + if (qemuBlockStorageSourceCreateStorage(vm, src, chain, asyncJob) < 0) + goto cleanup; + + if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) + goto cleanup; + + rc = qemuBlockStorageSourceAttachApplyStorage(priv->mon, data); + + if (rc == 0) + rc = qemuBlockStorageSourceAttachApplyFormatDeps(priv->mon, data); + + if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || rc < 0) + goto cleanup; + + if (qemuBlockStorageSourceCreateFormat(vm, src, backingStore, chain, + asyncJob) < 0) + goto cleanup; + + if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) + goto cleanup; + + rc = qemuBlockStorageSourceAttachApplyFormat(priv->mon, data); + + if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || rc < 0) + goto cleanup; + + ret = 0; + + cleanup: + if (ret < 0 && + virDomainObjIsActive(vm) && + qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) == 0) { + + qemuBlockStorageSourceAttachRollback(priv->mon, data); + ignore_value(qemuDomainObjExitMonitor(priv->driver, vm)); + } + + return ret; +} + + +/** + * qemuBlockStorageSourceCreateDetectSize: + * @vm: domain object + * @src: storage source to update size/capacity on + * @templ: storage source template + * @asyncJob: qemu asynchronous job type + * + * When creating a storage source via blockdev-create we need to know the size + * and capacity of the original volume (e.g. when creating a snapshot or copy). + * This function updates @src's 'capacity' and 'physical' attributes according + * to the detected sizes from @templ. + */ +int +qemuBlockStorageSourceCreateDetectSize(virDomainObjPtr vm, + virStorageSourcePtr src, + virStorageSourcePtr templ, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + VIR_AUTOPTR(virHashTable) stats = NULL; + qemuBlockStatsPtr entry; + int rc; + + if (!(stats = virHashCreate(10, virHashValueFree))) + return -1; + + if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) + return -1; + + rc = qemuMonitorBlockStatsUpdateCapacityBlockdev(priv->mon, stats); + + if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || rc < 0) + return -1; + + if (!(entry = virHashLookup(stats, templ->nodeformat))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to update capacity data for block node '%s'"), + templ->nodeformat); + return -1; + } + + if (src->format == VIR_STORAGE_FILE_RAW) { + src->physical = entry->capacity; + } else { + src->physical = entry->physical; + } + + src->capacity = entry->capacity; + + return 0; +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index ab6b9dc791..a5e970fa1e 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -182,3 +182,17 @@ int qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src, virJSONValuePtr *props) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int +qemuBlockStorageSourceCreate(virDomainObjPtr vm, + virStorageSourcePtr src, + virStorageSourcePtr backingStore, + virStorageSourcePtr chain, + qemuBlockStorageSourceAttachDataPtr data, + qemuDomainAsyncJob asyncJob); + +int +qemuBlockStorageSourceCreateDetectSize(virDomainObjPtr vm, + virStorageSourcePtr src, + virStorageSourcePtr templ, + qemuDomainAsyncJob asyncJob); diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 6ac60e86d7..70550d17e7 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -65,7 +65,8 @@ VIR_ENUM_IMPL(qemuBlockjob, "copy", "commit", "active-commit", - ""); + "", + "create"); static virClassPtr qemuBlockJobDataClass; @@ -78,6 +79,9 @@ qemuBlockJobDataDispose(void *obj) virObjectUnref(job->chain); virObjectUnref(job->mirrorChain); + if (job->type == QEMU_BLOCKJOB_TYPE_CREATE) + virObjectUnref(job->data.create.src); + VIR_FREE(job->name); VIR_FREE(job->errmsg); } @@ -274,6 +278,37 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm, } +qemuBlockJobDataPtr +qemuBlockJobNewCreate(virDomainObjPtr vm, + virStorageSourcePtr src, + virStorageSourcePtr chain, + bool storage) +{ + VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL; + VIR_AUTOFREE(char *) jobname = NULL; + const char *nodename = src->nodeformat; + + if (storage) + nodename = src->nodestorage; + + if (virAsprintf(&jobname, "create-%s", nodename) < 0) + return NULL; + + if (!(job = qemuBlockJobDataNew(QEMU_BLOCKJOB_TYPE_CREATE, jobname))) + return NULL; + + if (virStorageSourceIsBacking(chain)) + job->chain = virObjectRef(chain); + + job->data.create.src = virObjectRef(src); + + if (qemuBlockJobRegister(job, vm, NULL, true) < 0) + return NULL; + + VIR_RETURN_PTR(job); +} + + /** * qemuBlockJobDiskGetJob: * @disk: disk definition @@ -1007,6 +1042,49 @@ qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver, job->disk->mirror = NULL; } + +static void +qemuBlockJobProcessEventConcludedCreate(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuBlockJobDataPtr job, + qemuDomainAsyncJob asyncJob) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; + + /* if there is a synchronous client waiting for this job that means that + * it will handle further hotplug of the created volume and also that + * the 'chain' which was registered is under their control */ + if (job->synchronous) { + virObjectUnref(job->chain); + job->chain = NULL; + return; + } + + if (!job->data.create.src) + return; + + if (!(backend = qemuBlockStorageSourceDetachPrepare(job->data.create.src, NULL))) + return; + + /* the format node part was not attached yet, so we don't need to detach it */ + backend->formatAttached = false; + if (job->data.create.storage) { + backend->storageAttached = false; + VIR_FREE(backend->encryptsecretAlias); + } + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return; + + qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm), backend); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return; + + qemuDomainStorageSourceAccessRevoke(driver, vm, job->data.create.src); +} + + static void qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, virQEMUDriverPtr driver, @@ -1028,6 +1106,10 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, qemuBlockJobProcessEventCompletedActiveCommit(driver, vm, job, asyncJob); break; + case QEMU_BLOCKJOB_TYPE_CREATE: + qemuBlockJobProcessEventConcludedCreate(driver, vm, job, asyncJob); + break; + case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: @@ -1051,6 +1133,10 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, } break; + case QEMU_BLOCKJOB_TYPE_CREATE: + qemuBlockJobProcessEventConcludedCreate(driver, vm, job, asyncJob); + break; + case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 5b740db5a8..ff3c4a9eb7 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -62,6 +62,7 @@ typedef enum { QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT, /* Additional enum values local to qemu */ QEMU_BLOCKJOB_TYPE_INTERNAL, + QEMU_BLOCKJOB_TYPE_CREATE, QEMU_BLOCKJOB_TYPE_LAST } qemuBlockJobType; verify((int)QEMU_BLOCKJOB_TYPE_INTERNAL == VIR_DOMAIN_BLOCK_JOB_TYPE_LAST); @@ -87,6 +88,15 @@ struct _qemuBlockJobCommitData { }; +typedef struct _qemuBlockJobCreateData qemuBlockJobCreateData; +typedef qemuBlockJobCreateData *qemuBlockJobDataCreatePtr; + +struct _qemuBlockJobCreateData { + bool storage; + virStorageSourcePtr src; +}; + + typedef struct _qemuBlockJobData qemuBlockJobData; typedef qemuBlockJobData *qemuBlockJobDataPtr; @@ -102,6 +112,7 @@ struct _qemuBlockJobData { union { qemuBlockJobPullData pull; qemuBlockJobCommitData commit; + qemuBlockJobCreateData create; } data; int type; /* qemuBlockJobType */ @@ -146,6 +157,12 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm, virStorageSourcePtr top, virStorageSourcePtr base); +qemuBlockJobDataPtr +qemuBlockJobNewCreate(virDomainObjPtr vm, + virStorageSourcePtr src, + virStorageSourcePtr chain, + bool storage); + qemuBlockJobDataPtr qemuBlockJobDiskGetJob(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2e722f4ee4..8f32f8a035 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2412,6 +2412,19 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, virBufferAsprintf(&childBuf, "<topparent node='%s'/>\n", job->data.commit.topparent->nodeformat); break; + case QEMU_BLOCKJOB_TYPE_CREATE: + if (job->data.create.storage) + virBufferAddLit(&childBuf, "<create mode='storage'/>\n"); + + if (job->data.create.src && + qemuDomainObjPrivateXMLFormatBlockjobFormatSource(&childBuf, + "src", + job->data.create.src, + data->xmlopt, + false) < 0) + return -1; + break; + case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: @@ -2856,8 +2869,12 @@ qemuDomainObjPrivateXMLParseBlockjobNodename(qemuBlockJobDataPtr job, static void qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + virDomainXMLOptionPtr xmlopt) { + VIR_AUTOFREE(char *) createmode = NULL; + xmlNodePtr tmp; + switch ((qemuBlockJobType) job->type) { case QEMU_BLOCKJOB_TYPE_PULL: qemuDomainObjPrivateXMLParseBlockjobNodename(job, @@ -2891,6 +2908,19 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job, goto broken; break; + case QEMU_BLOCKJOB_TYPE_CREATE: + if (!(tmp = virXPathNode("./src", ctxt)) || + !(job->data.create.src = qemuDomainObjPrivateXMLParseBlockjobChain(tmp, ctxt, xmlopt))) + goto broken; + + if ((createmode = virXPathString("string(./create/@mode)", ctxt))) { + if (STRNEQ(createmode, "storage")) + goto broken; + + job->data.create.storage = true; + } + break; + case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: @@ -2980,7 +3010,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, if (mirror) job->mirrorChain = virObjectRef(job->disk->mirror); - qemuDomainObjPrivateXMLParseBlockjobDataSpecific(job, ctxt); + qemuDomainObjPrivateXMLParseBlockjobDataSpecific(job, ctxt, xmlopt); if (qemuBlockJobRegister(job, vm, disk, false) < 0) return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2c03cc5dff..e358c6a1c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17730,6 +17730,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, case QEMU_BLOCKJOB_TYPE_PULL: case QEMU_BLOCKJOB_TYPE_COMMIT: case QEMU_BLOCKJOB_TYPE_INTERNAL: + case QEMU_BLOCKJOB_TYPE_CREATE: virReportError(VIR_ERR_OPERATION_INVALID, _("job type '%s' does not support pivot"), qemuBlockjobTypeToString(job->type)); diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml index d06bbb7059..408e9269db 100644 --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -243,6 +243,23 @@ <base node='libvirt-19-format'/> <top node='libvirt-17-format'/> </blockjob> + <blockjob name='create-libvirt-1337-storage' type='create' state='running'> + <create mode='storage'/> + <src type='network' format='qcow2'> + <source protocol='rbd' name='pool/volname.qcow2' tlsFromConfig='0' index='1337'> + <host name='example.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='libvirt-1337-storage'/> + <nodename type='format' name='libvirt-1337-format'/> + </nodenames> + <objects> + <secret type='auth' alias='libvirt-1337-storage-secret0'/> + </objects> + </privateData> + </source> + </src> + </blockjob> <blockjob name='pull-vdc-libvirt-9-format' type='commit' state='running'> <disk dst='vdc'/> <base node='libvirt-11-format'/> @@ -252,6 +269,34 @@ <blockjob name='drive-virtio-disk0' type='copy' state='ready'> <disk dst='vda' mirror='yes'/> </blockjob> + <blockjob name='create-libvirt-1338-format' type='create' state='running'> + <chains> + <disk type='file' format='qcow2'> + <source file='/create/src1.qcow2' index='1339'> + <privateData> + <nodenames> + <nodename type='storage' name='libvirt-1339-storage'/> + <nodename type='format' name='libvirt-1339-format'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </disk> + </chains> + <src type='file' format='qcow2'> + <source file='/tmp/create/overlay.qcow2' index='1338'> + <privateData> + <nodenames> + <nodename type='storage' name='libvirt-1338-storage'/> + <nodename type='format' name='libvirt-1338-format'/> + </nodenames> + <objects> + <secret type='encryption' alias='libvirt-1338-storage-secret0'/> + </objects> + </privateData> + </source> + </src> + </blockjob> <blockjob name='test-orphan-job0' type='copy' state='ready'> <chains> <disk type='file' format='qcow2'> -- 2.21.0

Implement job handling for the block copy job (drive/blockdev-mirror) when using -blockdev. In contrast to the previously implemented blockjobs the block copy job introduces new images to the running qemu instance, thus requires a bit more handling. When copying to new images the code now makes use of blockdev-create to format the images explicitly rather than depending on automagic qemu behaviour. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 87 +++++++++++++++++ src/qemu/qemu_blockjob.h | 16 +++ src/qemu/qemu_domain.c | 13 +++ src/qemu/qemu_driver.c | 97 ++++++++++++++++--- .../blockjob-blockdev-in.xml | 14 +++ 5 files changed, 216 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 70550d17e7..3003e9c518 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -309,6 +309,40 @@ qemuBlockJobNewCreate(virDomainObjPtr vm, } +qemuBlockJobDataPtr +qemuBlockJobDiskNewCopy(virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr mirror, + bool shallow, + bool reuse) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL; + VIR_AUTOFREE(char *) jobname = NULL; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if (virAsprintf(&jobname, "copy-%s-%s", disk->dst, disk->src->nodeformat) < 0) + return NULL; + } else { + if (!(jobname = qemuAliasDiskDriveFromDisk(disk))) + return NULL; + } + + if (!(job = qemuBlockJobDataNew(QEMU_BLOCKJOB_TYPE_COPY, jobname))) + return NULL; + + job->mirrorChain = virObjectRef(mirror); + + if (shallow && !reuse) + job->data.copy.shallownew = true; + + if (qemuBlockJobRegister(job, vm, disk, true) < 0) + return NULL; + + VIR_RETURN_PTR(job); +} + + /** * qemuBlockJobDiskGetJob: * @disk: disk definition @@ -1043,6 +1077,50 @@ qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver, } +static void +qemuBlockJobProcessEventConcludedCopyPivot(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuBlockJobDataPtr job, + qemuDomainAsyncJob asyncJob) +{ + VIR_DEBUG("copy job '%s' on VM '%s' pivoted", job->name, vm->def->name); + + if (!job->disk) + return; + + /* for shallow copy without reusing external image the user can either not + * specify the backing chain in which case libvirt will open and use the + * chain the user provided or not specify a chain in which case we'll + * inherit the rest of the chain */ + if (job->data.copy.shallownew && + !virStorageSourceIsBacking(job->disk->mirror->backingStore)) + VIR_STEAL_PTR(job->disk->mirror->backingStore, job->disk->src->backingStore); + + qemuBlockJobRewriteConfigDiskSource(vm, job->disk, job->disk->mirror); + + qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->disk->src); + virObjectUnref(job->disk->src); + VIR_STEAL_PTR(job->disk->src, job->disk->mirror); +} + + +static void +qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuBlockJobDataPtr job, + qemuDomainAsyncJob asyncJob) +{ + VIR_DEBUG("copy job '%s' on VM '%s' aborted", job->name, vm->def->name); + + if (!job->disk) + return; + + qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->disk->mirror); + virObjectUnref(job->disk->mirror); + job->disk->mirror = NULL; +} + + static void qemuBlockJobProcessEventConcludedCreate(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -1111,6 +1189,12 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, break; case QEMU_BLOCKJOB_TYPE_COPY: + if (job->state == QEMU_BLOCKJOB_STATE_PIVOTING) + qemuBlockJobProcessEventConcludedCopyPivot(driver, vm, job, asyncJob); + else + qemuBlockJobProcessEventConcludedCopyAbort(driver, vm, job, asyncJob); + break; + case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_LAST: @@ -1138,6 +1222,9 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, break; case QEMU_BLOCKJOB_TYPE_COPY: + qemuBlockJobProcessEventConcludedCopyAbort(driver, vm, job, asyncJob); + break; + case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_LAST: diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index ff3c4a9eb7..41a5cd91f8 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -97,6 +97,14 @@ struct _qemuBlockJobCreateData { }; +typedef struct _qemuBlockJobCopyData qemuBlockJobCopyData; +typedef qemuBlockJobCopyData *qemuBlockJobDataCopyPtr; + +struct _qemuBlockJobCopyData { + bool shallownew; +}; + + typedef struct _qemuBlockJobData qemuBlockJobData; typedef qemuBlockJobData *qemuBlockJobDataPtr; @@ -113,6 +121,7 @@ struct _qemuBlockJobData { qemuBlockJobPullData pull; qemuBlockJobCommitData commit; qemuBlockJobCreateData create; + qemuBlockJobCopyData copy; } data; int type; /* qemuBlockJobType */ @@ -163,6 +172,13 @@ qemuBlockJobNewCreate(virDomainObjPtr vm, virStorageSourcePtr chain, bool storage); +qemuBlockJobDataPtr +qemuBlockJobDiskNewCopy(virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr mirror, + bool shallow, + bool reuse); + qemuBlockJobDataPtr qemuBlockJobDiskGetJob(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8f32f8a035..364046a456 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2426,6 +2426,10 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, break; case QEMU_BLOCKJOB_TYPE_COPY: + if (job->data.copy.shallownew) + virBufferAddLit(&attrBuf, " shallownew='yes'"); + break; + case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_LAST: @@ -2873,6 +2877,7 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job, virDomainXMLOptionPtr xmlopt) { VIR_AUTOFREE(char *) createmode = NULL; + VIR_AUTOFREE(char *) shallownew = NULL; xmlNodePtr tmp; switch ((qemuBlockJobType) job->type) { @@ -2922,6 +2927,14 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job, break; case QEMU_BLOCKJOB_TYPE_COPY: + if ((shallownew = virXPathString("string(./@shallownew)", ctxt))) { + if (STRNEQ(shallownew, "yes")) + goto broken; + + job->data.copy.shallownew = true; + } + break; + case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_LAST: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e358c6a1c4..261a4167b5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18310,7 +18310,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, { virQEMUDriverPtr driver = conn->privateData; qemuDomainObjPrivatePtr priv = vm->privateData; - VIR_AUTOFREE(char *) device = NULL; virDomainDiskDefPtr disk = NULL; int ret = -1; bool need_unlink = false; @@ -18322,6 +18321,11 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, qemuBlockJobDataPtr job = NULL; VIR_AUTOUNREF(virStorageSourcePtr) mirror = mirrorsrc; bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) crdata = NULL; + virStorageSourcePtr n; + virStorageSourcePtr mirrorBacking = NULL; + int rc = 0; /* Preliminaries: find the disk we are editing, sanity checks */ virCheckFlags(VIR_DOMAIN_BLOCK_COPY_SHALLOW | @@ -18351,9 +18355,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; - if (!(device = qemuAliasDiskDriveFromDisk(disk))) - goto endjob; - if (qemuDomainDiskBlockJobIsActive(disk)) goto endjob; @@ -18428,7 +18429,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, goto endjob; } - /* pre-create the image file */ + /* pre-create the image file. In case when 'blockdev' is used this is + * required so that libvirt can properly label the image for access by qemu */ if (!existing) { if (virStorageFileCreate(mirror) < 0) { virReportSystemError(errno, "%s", _("failed to create copy target")); @@ -18445,6 +18447,15 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, keepParentLabel) < 0) goto endjob; + /* we must initialize XML-provided chain prior to detecting to keep semantics + * with VM startup */ + if (blockdev) { + for (n = mirror; virStorageSourceIsBacking(n); n = n->backingStore) { + if (qemuDomainPrepareStorageSourceBlockdev(disk, n, priv, cfg) < 0) + goto endjob; + } + } + /* If reusing an external image that includes a backing file but the user * did not enumerate the chain in the XML we need to detect the chain */ if (mirror_reuse && @@ -18456,18 +18467,72 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (qemuDomainStorageSourceChainAccessAllow(driver, vm, mirror) < 0) goto endjob; - if (!(job = qemuBlockJobDiskNew(vm, disk, QEMU_BLOCKJOB_TYPE_COPY, device))) + if (blockdev) { + if (mirror_reuse) { + if (!(data = qemuBuildStorageSourceChainAttachPrepareBlockdev(mirror, + priv->qemuCaps))) + goto endjob; + } else { + if (qemuBlockStorageSourceCreateDetectSize(vm, mirror, disk->src, QEMU_ASYNC_JOB_NONE) < 0) + goto endjob; + + if (mirror_shallow) { + /* if external backing store is populated we'll need to open it */ + if (virStorageSourceHasBacking(mirror)) { + if (!(data = qemuBuildStorageSourceChainAttachPrepareBlockdev(mirror->backingStore, + priv->qemuCaps))) + goto endjob; + + mirrorBacking = mirror->backingStore; + } else { + /* backing store of original image will be reused, but the + * new image must refer to it in the metadata */ + mirrorBacking = disk->src->backingStore; + } + } + + if (!(crdata = qemuBuildStorageSourceChainAttachPrepareBlockdevTop(mirror, + priv->qemuCaps))) + goto endjob; + } + + if (data) { + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuBlockStorageSourceChainAttach(priv->mon, data); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; + + if (rc < 0) + goto endjob; + } + + if (crdata && + qemuBlockStorageSourceCreate(vm, mirror, mirrorBacking, mirror->backingStore, + crdata->srcdata[0], QEMU_ASYNC_JOB_NONE) < 0) + goto endjob; + } + + if (!(job = qemuBlockJobDiskNewCopy(vm, disk, mirror, mirror_shallow, mirror_reuse))) goto endjob; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; /* Actually start the mirroring */ qemuDomainObjEnterMonitor(driver, vm); - /* qemuMonitorDriveMirror needs to honor the REUSE_EXT flag as specified - * by the user */ - ret = qemuMonitorDriveMirror(priv->mon, device, mirror->path, format, - bandwidth, granularity, buf_size, - mirror_shallow, mirror_reuse); + + if (blockdev) { + ret = qemuMonitorBlockdevMirror(priv->mon, job->name, true, + disk->src->nodeformat, + mirror->nodeformat, bandwidth, + granularity, buf_size, mirror_shallow); + } else { + /* qemuMonitorDriveMirror needs to honor the REUSE_EXT flag as specified + * by the user */ + ret = qemuMonitorDriveMirror(priv->mon, job->name, mirror->path, format, + bandwidth, granularity, buf_size, + mirror_shallow, mirror_reuse); + } + virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; @@ -18484,6 +18549,16 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, qemuBlockJobStarted(job, vm); endjob: + if (rc < 0 && + virDomainObjIsActive(vm) && + (data || crdata)) { + qemuDomainObjEnterMonitor(driver, vm); + if (data) + qemuBlockStorageSourceChainDetach(priv->mon, data); + if (crdata) + qemuBlockStorageSourceAttachRollback(priv->mon, crdata->srcdata[0]); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + } if (need_unlink && virStorageFileUnlink(mirror) < 0) VIR_WARN("%s", _("unable to remove just-created copy target")); virStorageFileDeinit(mirror); diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml index 408e9269db..67252a3729 100644 --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -260,6 +260,9 @@ </source> </src> </blockjob> + <blockjob name='pull-vdc-libvirt-4321-format' type='copy' state='ready' shallownew='yes'> + <disk dst='vdc'/> + </blockjob> <blockjob name='pull-vdc-libvirt-9-format' type='commit' state='running'> <disk dst='vdc'/> <base node='libvirt-11-format'/> @@ -571,6 +574,17 @@ </backingStore> </backingStore> </backingStore> + <mirror type='file' file='/tmp/copy2' format='qcow2' job='copy' ready='yes'> + <format type='qcow2'/> + <source file='/tmp/copy2' index='4321'> + <privateData> + <nodenames> + <nodename type='storage' name='libvirt-4321-storage'/> + <nodename type='format' name='libvirt-4321-format'/> + </nodenames> + </privateData> + </source> + </mirror> <target dev='vdc' bus='virtio'/> <alias name='virtio-disk3'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> -- 2.21.0

Now that we support blockdev for qemuDomainBlockCopy we can allow copying to remote destinations as well. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 47 ++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 261a4167b5..b4eecdd8be 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18195,6 +18195,9 @@ qemuDomainBlockCopyValidateMirror(virStorageSourcePtr mirror, int desttype = virStorageSourceGetActualType(mirror); struct stat st; + if (!virStorageSourceIsLocalStorage(mirror)) + return 0; + if (virStorageFileAccess(mirror, F_OK) < 0) { if (errno != ENOENT) { virReportSystemError(errno, "%s", @@ -18321,6 +18324,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, qemuBlockJobDataPtr job = NULL; VIR_AUTOUNREF(virStorageSourcePtr) mirror = mirrorsrc; bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); + bool mirror_initialized = false; VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; VIR_AUTOPTR(qemuBlockStorageSourceChainData) crdata = NULL; virStorageSourcePtr n; @@ -18393,15 +18397,19 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, } /* Prepare the destination file. */ - /* XXX Allow non-file mirror destinations */ - if (!virStorageSourceIsLocalStorage(mirror)) { + if (!blockdev && + !virStorageSourceIsLocalStorage(mirror)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("non-file destination not supported yet")); goto endjob; } - if (qemuDomainStorageFileInit(driver, vm, mirror, NULL) < 0) - goto endjob; + if (virStorageFileSupportsCreate(mirror) == 1) { + if (qemuDomainStorageFileInit(driver, vm, mirror, NULL) < 0) + goto endjob; + + mirror_initialized = true; + } if (qemuDomainBlockCopyValidateMirror(mirror, disk->dst, &existing) < 0) goto endjob; @@ -18410,12 +18418,19 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (!mirror_reuse) { mirror->format = disk->src->format; } else { - /* If the user passed the REUSE_EXT flag, then either they - * 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(mirror->path, cfg->user, - cfg->group); + if (mirror_initialized && + virStorageSourceIsLocalStorage(mirror)) { + /* If the user passed the REUSE_EXT flag, then either they + * 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(mirror->path, cfg->user, + cfg->group); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("reused mirror destination format must be specified")); + goto endjob; + } } } @@ -18432,12 +18447,14 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, /* pre-create the image file. In case when 'blockdev' is used this is * required so that libvirt can properly label the image for access by qemu */ if (!existing) { - if (virStorageFileCreate(mirror) < 0) { - virReportSystemError(errno, "%s", _("failed to create copy target")); - goto endjob; - } + if (mirror_initialized) { + if (virStorageFileCreate(mirror) < 0) { + virReportSystemError(errno, "%s", _("failed to create copy target")); + goto endjob; + } - need_unlink = true; + need_unlink = true; + } } if (mirror->format > 0) -- 2.21.0