On 03/29/2018 01:52 PM, Jim Fehlig wrote:
> On 03/09/2018 09:48 AM, John Ferlan wrote:
>> For openvzDomObjFromDomainLocked and openvzDomainLookupByID
>> let's return a locked and referenced @vm object so that callers
>> can then use the common and more consistent virDomainObjEndAPI
>> in order to handle cleanup rather than needing to know that the
>> returned object is locked and calling virObjectUnlock.
>>
>> The LookupByName already returns the ref counted and locked object,
>> so this will make things more consistent.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/openvz/openvz_driver.c | 76
>> ++++++++++++++++------------------------------
>> 1 file changed, 26 insertions(+), 50 deletions(-)
>
> Reviewed-by: Jim Fehlig <jfehlig@xxxxxxxx>
>
> Regards,
> Jim
>
Thanks - although based on some of the libxl adjustments and comments
in/for the bhyve patch #1 - the following 3 functions need adjustment as
well since there's a reffed @vm object:
openvzDomainUndefineFlags
- Since we have a reffed/locked object calling into
virDomainObjListRemove, we need to Lock it upon return and allow the
subsequent virDomainObjEndAPI handle the removal.
openvzDomainMigratePrepare3Params
- Original change too aggressive here as virDomainObjListAdd
returns 2 refs and locked and we need to leave it that way.
openvzDomainMigrateConfirm3Params
- Same as openvzDomainUndefineFlags
Changes I'd squash in are:
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 649922e59..ec9541840 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1165,7 +1165,7 @@ openvzDomainUndefineFlags(virDomainPtr dom,
vm->persistent = 0;
} else {
virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
+ virObjectLock(vm);
}
ret = 0;
@@ -2263,7 +2263,8 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn,
done:
VIR_FREE(my_hostname);
virURIFree(uri);
- virDomainObjEndAPI(&vm);
+ if (vm)
+ virObjectUnlock(vm);
return ret;
}
@@ -2418,7 +2419,7 @@ openvzDomainMigrateConfirm3Params(virDomainPtr domain,
VIR_DEBUG("Domain '%s' successfully migrated", vm->def->name);
virDomainObjListRemove(driver->domains, vm);
- vm = NULL;
+ virObjectLock(vm);
ret = 0;
John
>>
>> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
>> index b31bf0714..b0b72b171 100644
>> --- a/src/openvz/openvz_driver.c
>> +++ b/src/openvz/openvz_driver.c
>> @@ -95,7 +95,7 @@ openvzDomObjFromDomainLocked(struct openvz_driver
>> *driver,
>> virDomainObjPtr vm;
>> char uuidstr[VIR_UUID_STRING_BUFLEN];
>> - if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
>> + if (!(vm = virDomainObjListFindByUUIDRef(driver->domains, uuid))) {
>> virUUIDFormat(uuid, uuidstr);
>> virReportError(VIR_ERR_NO_DOMAIN,
>> @@ -329,8 +329,7 @@ openvzDomainGetHostname(virDomainPtr dom, unsigned
>> int flags)
>> }
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return hostname;
>> error:
>> @@ -347,7 +346,7 @@ static virDomainPtr
>> openvzDomainLookupByID(virConnectPtr conn,
>> virDomainPtr dom = NULL;
>> openvzDriverLock(driver);
>> - vm = virDomainObjListFindByID(driver->domains, id);
>> + vm = virDomainObjListFindByIDRef(driver->domains, id);
>> openvzDriverUnlock(driver);
>> if (!vm) {
>> @@ -359,8 +358,7 @@ static virDomainPtr
>> openvzDomainLookupByID(virConnectPtr conn,
>> dom = virGetDomain(conn, vm->def->name, vm->def->uuid,
>> vm->def->id);
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return dom;
>> }
>> @@ -391,8 +389,7 @@ static char *openvzDomainGetOSType(virDomainPtr
>> dom)
>> ignore_value(VIR_STRDUP(ret,
>> virDomainOSTypeToString(vm->def->os.type)));
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -409,8 +406,7 @@ static virDomainPtr
>> openvzDomainLookupByUUID(virConnectPtr conn,
>> dom = virGetDomain(conn, vm->def->name, vm->def->uuid,
>> vm->def->id);
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return dom;
>> }
>> @@ -469,8 +465,7 @@ static int openvzDomainGetInfo(virDomainPtr dom,
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -492,8 +487,7 @@ openvzDomainGetState(virDomainPtr dom,
>> ret = openvzGetVEStatus(vm, state, reason);
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -509,8 +503,7 @@ static int openvzDomainIsActive(virDomainPtr dom)
>> ret = virDomainObjIsActive(obj);
>> - if (obj)
>> - virObjectUnlock(obj);
>> + virDomainObjEndAPI(&obj);
>> return ret;
>> }
>> @@ -526,8 +519,7 @@ static int openvzDomainIsPersistent(virDomainPtr
>> dom)
>> ret = obj->persistent;
>> - if (obj)
>> - virObjectUnlock(obj);
>> + virDomainObjEndAPI(&obj);
>> return ret;
>> }
>> @@ -549,8 +541,7 @@ static char *openvzDomainGetXMLDesc(virDomainPtr
>> dom, unsigned int flags) {
>> ret = virDomainDefFormat(vm->def, driver->caps,
>> virDomainDefFormatConvertXMLFlags(flags));
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -600,8 +591,7 @@ static int openvzDomainSuspend(virDomainPtr dom)
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -631,8 +621,7 @@ static int openvzDomainResume(virDomainPtr dom)
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -670,8 +659,7 @@ openvzDomainShutdownFlags(virDomainPtr dom,
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -724,8 +712,7 @@ static int openvzDomainReboot(virDomainPtr dom,
>> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
>> VIR_DOMAIN_RUNNING_BOOTED);
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -1183,8 +1170,7 @@ openvzDomainUndefineFlags(virDomainPtr dom,
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> openvzDriverUnlock(driver);
>> return ret;
>> }
>> @@ -1213,8 +1199,7 @@ openvzDomainSetAutostart(virDomainPtr dom, int
>> autostart)
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -1243,8 +1228,7 @@ openvzDomainGetAutostart(virDomainPtr dom, int
>> *autostart)
>> cleanup:
>> VIR_FREE(value);
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -1336,8 +1320,7 @@ static int
>> openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -1934,8 +1917,7 @@ openvzDomainInterfaceStats(virDomainPtr dom,
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -2029,8 +2011,7 @@ openvzDomainUpdateDeviceFlags(virDomainPtr
>> dom, const char *xml,
>> cleanup:
>> openvzDriverUnlock(driver);
>> virDomainDeviceDefFree(dev);
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -2166,8 +2147,7 @@ openvzDomainMigrateBegin3Params(virDomainPtr
>> domain,
>> VIR_DOMAIN_DEF_FORMAT_SECURE);
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return xml;
>> }
>> @@ -2269,8 +2249,7 @@
>> openvzDomainMigratePrepare3Params(virConnectPtr dconn,
>> done:
>> VIR_FREE(my_hostname);
>> virURIFree(uri);
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -2323,8 +2302,7 @@ openvzDomainMigratePerform3Params(virDomainPtr
>> domain,
>> cleanup:
>> virCommandFree(cmd);
>> virURIFree(uri);
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -2431,8 +2409,7 @@ openvzDomainMigrateConfirm3Params(virDomainPtr
>> domain,
>> ret = 0;
>> cleanup:
>> - if (vm)
>> - virObjectUnlock(vm);
>> + virDomainObjEndAPI(&vm);
>> return ret;
>> }
>> @@ -2450,8 +2427,7 @@ openvzDomainHasManagedSaveImage(virDomainPtr
>> dom, unsigned int flags)
>> ret = 0;
>> - if (obj)
>> - virObjectUnlock(obj);
>> + virDomainObjEndAPI(&obj);
>> return ret;
>> }
>>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
