On Tue, Aug 23, 2022 at 18:32:06 +0200, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> src/qemu/qemu_block.c | 78 ++++++++++++++++++++++++++-----------------
> src/qemu/qemu_block.h | 10 ++++++
> 2 files changed, 57 insertions(+), 31 deletions(-)
>
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 5b34461853..902ec7b2a5 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -3204,28 +3204,22 @@ qemuBlockExportAddNBD(virDomainObj *vm,
>
>
> int
> -qemuBlockCommit(virDomainObj *vm,
> - virQEMUDriver *driver,
> - const char *path,
> - const char *base,
> - const char *top,
> - unsigned long bandwidth,
> - unsigned int flags)
> +qemuBlockCommitImpl(virDomainObj *vm,
> + virQEMUDriver *driver,
> + virDomainDiskDef *disk,
> + virStorageSource *baseSource,
> + virStorageSource *topSource,
> + virStorageSource *top_parent,
> + unsigned long bandwidth,
> + unsigned int flags)
> {
> qemuDomainObjPrivate *priv = vm->privateData;
> int rc = -1;
> - virDomainDiskDef *disk = NULL;
> - virStorageSource *topSource;
> - virStorageSource *baseSource = NULL;
> - virStorageSource *top_parent = NULL;
> bool clean_access = false;
> g_autofree char *backingPath = NULL;
> qemuBlockJobData *job = NULL;
> g_autoptr(virStorageSource) mirror = NULL;
>
> - if (virDomainObjCheckActive(vm) < 0)
> - return -1;
> -
> /* Convert bandwidth MiB to bytes, if necessary */
> if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES)) {
> if (bandwidth > LLONG_MAX >> 20) {
> @@ -3237,9 +3231,6 @@ qemuBlockCommit(virDomainObj *vm,
> bandwidth <<= 20;
> }
>
> - if (!(disk = qemuDomainDiskByName(vm->def, path)))
> - return -1;
> -
> if (!qemuDomainDiskBlockJobIsSupported(disk))
> return -1;
>
> @@ -3256,12 +3247,6 @@ qemuBlockCommit(virDomainObj *vm,
> if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
> return -1;
>
> - if (!top || STREQ(top, disk->dst))
> - topSource = disk->src;
> - else if (!(topSource = virStorageSourceChainLookup(disk->src, NULL, top,
> - disk->dst, &top_parent)))
> - return -1;
> -
> if (topSource == disk->src) {
> /* XXX Should we auto-pivot when COMMIT_ACTIVE is not specified? */
> if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE)) {
> @@ -3280,22 +3265,16 @@ qemuBlockCommit(virDomainObj *vm,
> if (!virStorageSourceHasBacking(topSource)) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("top '%s' in chain for '%s' has no backing file"),
> - topSource->path, path);
> + topSource->path, disk->src->path);
> return -1;
> }
>
> - if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
> - baseSource = topSource->backingStore;
> - else if (!(baseSource = virStorageSourceChainLookup(disk->src, topSource,
> - base, disk->dst, NULL)))
> - return -1;
> -
> if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&
> baseSource != topSource->backingStore) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("base '%s' is not immediately below '%s' in chain "
> "for '%s'"),
> - base, topSource->path, path);
> + baseSource->path, topSource->path, disk->src->path);
> return -1;
> }
>
> @@ -3400,6 +3379,43 @@ qemuBlockCommit(virDomainObj *vm,
> }
>
>
> +int
> +qemuBlockCommit(virDomainObj *vm,
> + virQEMUDriver *driver,
> + const char *path,
> + const char *base,
> + const char *top,
> + unsigned long bandwidth,
> + unsigned int flags)
> +{
> + virDomainDiskDef *disk = NULL;
> + virStorageSource *topSource;
> + virStorageSource *baseSource = NULL;
> + virStorageSource *top_parent = NULL;
> +
> + if (virDomainObjCheckActive(vm) < 0)
> + return -1;
> +
> + if (!(disk = qemuDomainDiskByName(vm->def, path)))
> + return -1;
> +
> + if (!top || STREQ(top, disk->dst))
> + topSource = disk->src;
> + else if (!(topSource = virStorageSourceChainLookup(disk->src, NULL, top,
> + disk->dst, &top_parent)))
> + return -1;
> +
> + if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
> + baseSource = topSource->backingStore;
> + else if (!(baseSource = virStorageSourceChainLookup(disk->src, topSource,
> + base, disk->dst, NULL)))
> + return -1;
As mentioned in my reply to 1/N I'd prefer if this stuff is kept in
qemuDomainBlockCommit and the actual implementation is named qemuBlockCommit
instead of qemuBlockCommitImpl.
