Re: [PATCH 1/2] qemu: Relax locking in DomainHasManagedSaveImage and DomainMonitorCommand

[Peter Krempa <pkrempa@xxxxxxxxxx>]

On 12/13/12 12:40, Michal Privoznik wrote:
> There is no need to hold qemu lock during the whole execution
> of these two APIs.
> ---
>   src/qemu/qemu_driver.c |   12 ++++++------
>   1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2fda44e..74e442b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3288,6 +3288,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
>
>       qemuDriverLock(driver);
>       vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> +    qemuDriverUnlock(driver);
>       if (!vm) {
>           char uuidstr[VIR_UUID_STRING_BUFLEN];
>           virUUIDFormat(dom->uuid, uuidstr);
> @@ -3301,7 +3302,6 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
>   cleanup:
>       if (vm)
>           virDomainObjUnlock(vm);
> -    qemuDriverUnlock(driver);
>       return ret;
>   }

This is already taken care of upstream with:
commit 2745177b346a0ebbd3629e298178c60a54af242a
Author: Peter Krempa <pkrempa@xxxxxxxxxx>
Date:   Tue Dec 11 19:28:17 2012 +0100

    qemu: Refactor managed save functions to use domain lookup helpers


> @@ -12676,6 +12676,7 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
>
>       qemuDriverLock(driver);
>       vm = virDomainFindByUUID(&driver->domains, domain->uuid);
> +    qemuDriverUnlock(driver);
>       if (!vm) {
>           char uuidstr[VIR_UUID_STRING_BUFLEN];
>           virUUIDFormat(domain->uuid, uuidstr);
> @@ -12688,9 +12689,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
>           virReportError(VIR_ERR_OPERATION_INVALID,
>                          "%s", _("domain is not running"));
>           goto cleanup;
> -   }
> +    }
>
> -    if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>           goto cleanup;
>
>       if (!virDomainObjIsActive(vm)) {
> @@ -12705,9 +12706,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
>
>       hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP);
>
> -    qemuDomainObjEnterMonitorWithDriver(driver, vm);
> +    qemuDomainObjEnterMonitor(driver, vm);
>       ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp);
> -    qemuDomainObjExitMonitorWithDriver(driver, vm);
> +    qemuDomainObjExitMonitor(driver, vm);
>
>   endjob:
>       if (qemuDomainObjEndJob(driver, vm) == 0) {
> @@ -12717,7 +12718,6 @@ endjob:
>   cleanup:
>       if (vm)
>           virDomainObjUnlock(vm);
> -    qemuDriverUnlock(driver);
>       return ret;
>   }

This cleanup makes sense, but it'd be great to take advantage of the 
lookup helpers I introduced lately to decrease the complexity even more.

>

Peter

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