Re: [PATCH 1/2] qemuDomainUndefineFlags: Grab QEMU_JOB_MODIFY

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Mon, Aug 07, 2017 at 02:20:05PM +0200, Michal Privoznik wrote:
> This API is definitely modifying state of @vm. Therefore it
> should grab a job.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b3f65f440..574c351ae 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7325,10 +7325,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>     if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)
>         goto cleanup;
>
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +        goto cleanup;
> +
>     if (!vm->persistent) {
>         virReportError(VIR_ERR_OPERATION_INVALID,
>                        "%s", _("cannot undefine transient domain"));
> -        goto cleanup;
> +        goto endjob;
>     }
>
>     if (!virDomainObjIsActive(vm) &&
> @@ -7338,15 +7341,15 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>                            _("cannot delete inactive domain with %d "
>                              "snapshots"),
>                            nsnapshots);
> -            goto cleanup;
> +            goto endjob;
>         }
>         if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0)
> -            goto cleanup;
> +            goto endjob;
>     }
>
>     name = qemuDomainManagedSavePath(driver, vm);
>     if (name == NULL)
> -        goto cleanup;
> +        goto endjob;
>
>     if (virFileExists(name)) {
>         if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE) {
> @@ -7354,13 +7357,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                                _("Failed to remove domain managed "
>                                  "save image"));
> -                goto cleanup;
> +                goto endjob;
>             }
>         } else {
>             virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                            _("Refusing to undefine while domain managed "
>                              "save image exists"));
> -            goto cleanup;
> +            goto endjob;
>         }
>     }
>
> @@ -7372,17 +7375,17 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>                 virReportSystemError(errno,
>                                      _("failed to remove nvram: %s"),
>                                      vm->def->os.loader->nvram);
> -                goto cleanup;
> +                goto endjob;
>             }
>         } else if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) {
>             virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                            _("cannot delete inactive domain with nvram"));
> -            goto cleanup;
> +            goto endjob;
>         }
>     }
>
>     if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0)
> -        goto cleanup;
> +        goto endjob;
>
>     event = virDomainEventLifecycleNewFromObj(vm,
>                                      VIR_DOMAIN_EVENT_UNDEFINED,
> @@ -7399,6 +7402,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>         qemuDomainRemoveInactive(driver, vm);

You cannot RemoveInactive with a job.  Not that it wouldn't make sense,
but the code is written that way (at least for now).  It takes another
MODIFY job itself.

>     ret = 0;
> + endjob:
> +    qemuDomainObjEndJob(driver, vm);
>
>  cleanup:
>     VIR_FREE(name);
> --
> 2.13.0
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list