qemuDomainBlockPullCommon() is one of the functions in block pull implementation that have to support 'top' to make block pull as a whole to support it. Signed-off-by: Pavel Mores <pmores(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e9a62684f0..81cca360e0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17311,6 +17311,7 @@ static int qemuDomainBlockPullCommon(virDomainObjPtr vm, const char *path, const char *base, + const char *top, unsigned long bandwidth, unsigned int flags) { @@ -17322,6 +17323,9 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm, virStorageSourcePtr baseSource = NULL; unsigned int baseIndex = 0; g_autofree char *basePath = NULL; + virStorageSourcePtr topSource = NULL; + unsigned int topIndex = 0; + g_autofree char *topPath = NULL; g_autofree char *backingPath = NULL; unsigned long long speed = bandwidth; qemuBlockJobDataPtr job = NULL; @@ -17355,6 +17359,12 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm, base, baseIndex, NULL)))) goto endjob; + if (top && + (virStorageFileParseChainIndex(disk->dst, top, &topIndex) < 0 || + !(topSource = virStorageFileChainLookup(disk->src, disk->src, + top, topIndex, NULL)))) + goto endjob; + if (baseSource) { if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) { @@ -17388,7 +17398,7 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm, speed <<= 20; } - if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource, flags))) + if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource, /*topSource, */flags))) goto endjob; if (blockdev) { @@ -18160,7 +18170,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, /* For normal rebase (enhanced blockpull), the common code handles * everything, including vm cleanup. */ if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY)) - return qemuDomainBlockPullCommon(vm, path, base, bandwidth, flags); + return qemuDomainBlockPullCommon(vm, path, base, NULL, bandwidth, flags); /* If we got here, we are doing a block copy rebase. */ if (!(dest = virStorageSourceNew())) @@ -18311,7 +18321,7 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, } /* qemuDomainBlockPullCommon consumes the reference on @vm */ - return qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags); + return qemuDomainBlockPullCommon(vm, path, NULL, NULL, bandwidth, flags); } -- 2.24.1

A new command-line option --top was added to virsh's blockpull command. Similar to how --base is handled, in presence of --top the operation is implemented internally as a rebase. To that end, a corresponding new 'top' parameter was added to virDomainBlockRebase(). Signed-off-by: Pavel Mores <pmores(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 4 ++-- src/libvirt-domain.c | 5 +++-- tools/virsh-domain.c | 14 +++++++++++--- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index b440818ec2..069d7f98ec 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2560,8 +2560,8 @@ typedef enum { } virDomainBlockRebaseFlags; int virDomainBlockRebase(virDomainPtr dom, const char *disk, - const char *base, unsigned long bandwidth, - unsigned int flags); + const char *base, const char *top, + unsigned long bandwidth, unsigned int flags); /** * virDomainBlockCopyFlags: diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 65813b68cc..1f9d1b5b84 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -10150,6 +10150,7 @@ virDomainBlockPull(virDomainPtr dom, const char *disk, * @disk: path to the block device, or device shorthand * @base: path to backing file to keep, or device shorthand, * or NULL for no backing file + * @top: path to top file, or device shorthand, or NULL for no top * @bandwidth: (optional) specify bandwidth limit; flags determine the unit * @flags: bitwise-OR of virDomainBlockRebaseFlags * @@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char *disk, */ int virDomainBlockRebase(virDomainPtr dom, const char *disk, - const char *base, unsigned long bandwidth, - unsigned int flags) + const char *base, const char *top, + unsigned long bandwidth, unsigned int flags) { virConnectPtr conn; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9d0f7d68d2..47ccdd1b94 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2436,7 +2436,7 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd) if (bytes) flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES; - if (virDomainBlockRebase(dom, path, dest, bandwidth, flags) < 0) + if (virDomainBlockRebase(dom, path, dest, NULL, bandwidth, flags) < 0) goto cleanup; } @@ -2765,6 +2765,10 @@ static const vshCmdOptDef opts_blockpull[] = { .type = VSH_OT_STRING, .help = N_("path of backing file in chain for a partial pull") }, + {.name = "top", + .type = VSH_OT_STRING, + .help = N_("path of top backing file in chain for a partial pull") + }, {.name = "wait", .type = VSH_OT_BOOL, .help = N_("wait for job to finish") @@ -2804,6 +2808,7 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd) int timeout = 0; const char *path = NULL; const char *base = NULL; + const char *top = NULL; unsigned long bandwidth = 0; unsigned int flags = 0; virshBlockJobWaitDataPtr bjWait = NULL; @@ -2817,6 +2822,9 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) return false; + if (vshCommandOptStringReq(ctl, cmd, "top", &top) < 0) + return false; + if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0) return false; @@ -2834,11 +2842,11 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd) verbose, timeout, async))) goto cleanup; - if (base || flags) { + if (base || top || flags) { if (bytes) flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES; - if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0) + if (virDomainBlockRebase(dom, path, base, top, bandwidth, flags) < 0) goto cleanup; } else { if (bytes) -- 2.24.1

On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote: Yes. Seeing as the function takes just a bunch of primitive data types and a virDomainPtr, I'm afraid I don't see much room for not changing the signature, apart from arguably dubious tricks like stuffing both base and top to the existing 'base' parameter and parsing the individual values out of it in the function body. Any convention or suggestion to help pick a good name for the new function? Thanks, pvl

On Wed, Mar 04, 2020 at 02:53:53PM +0100, Peter Krempa wrote: I'm not aware of any real demand for this, however as I stated in the cover letter I believe I need full blockpull to deal with the bug I'm actually working on, which is full support for external snapshots in snapshot-delete. Considering what you said, the "dubious trick" I mentioned in the other branch of this thread might be a bearable interim solution... pvl

On Wed, Mar 04, 2020 at 14:55:30 +0100, Pavel Mores wrote: That's gross. Please don't do that. No, that's on same par as API.

This commit only handles submission of block-stream command. Since block-stream is a job, we'll need to handle its completion as well in an upcoming commit. Signed-off-by: Pavel Mores <pmores(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3f97bf593..b1ffc68e6b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17410,7 +17410,15 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm, !(backingPath = qemuBlockGetBackingStoreString(baseSource))) goto endjob; } - device = disk->src->nodeformat; + if (topSource) { + device = topSource->nodeformat; + if (qemuDomainStorageSourceAccessAllow(driver, vm, topSource, false, false) < 0) { + virReportError(VIR_ERR_ACCESS_DENIED, "can't make 'top' writable"); + goto endjob; + } + } else { + device = disk->src->nodeformat; + } } else { device = job->name; } @@ -18171,7 +18179,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, /* For normal rebase (enhanced blockpull), the common code handles * everything, including vm cleanup. */ if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY)) - return qemuDomainBlockPullCommon(vm, path, base, NULL, bandwidth, flags); + return qemuDomainBlockPullCommon(vm, path, base, top, bandwidth, flags); /* If we got here, we are doing a block copy rebase. */ if (!(dest = virStorageSourceNew())) @@ -18302,8 +18310,8 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml, static int -qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, - unsigned int flags) +qemuDomainBlockPull(virDomainPtr dom, const char *path, + unsigned long bandwidth, unsigned int flags) { virDomainObjPtr vm; virCheckFlags(VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES, -1); -- 2.24.1