[libvirt RFC 03/24] qemu_block: extract qemuBlockCommit impl to separate function

[Peter Krempa]

On Tue, Aug 23, 2022 at 18:32:06 +0200, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  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.