Rather than repeat code throughout, create and use a couple of
accessors in order to lookup by UUID.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
src/openvz/openvz_driver.c | 266 +++++++++++++--------------------------------
1 file changed, 76 insertions(+), 190 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index a211c370e..167ba2f7a 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -87,6 +87,39 @@ static void openvzDriverUnlock(struct openvz_driver *driver)
struct openvz_driver ovz_driver;
+
+static virDomainObjPtr
+openvzDomObjFromDomainLocked(struct openvz_driver *driver,
+ const unsigned char *uuid)
+{
+ virDomainObjPtr vm;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
+ virUUIDFormat(uuid, uuidstr);
+
+ virReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
+ return NULL;
+ }
+
+ return vm;
+}
+
+
+static virDomainObjPtr
+openvzDomObjFromDomain(struct openvz_driver *driver,
+ const unsigned char *uuid)
+{
+ virDomainObjPtr vm;
+
+ openvzDriverLock(driver);
+ vm = openvzDomObjFromDomainLocked(driver, uuid);
+ openvzDriverUnlock(driver);
+ return vm;
+}
+
+
static int
openvzDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps ATTRIBUTE_UNUSED,
@@ -281,15 +314,8 @@ openvzDomainGetHostname(virDomainPtr dom, unsigned int flags)
virDomainObjPtr vm;
virCheckFlags(0, NULL);
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return NULL;
hostname = openvzVEGetStringParam(dom, "hostname");
if (hostname == NULL)
@@ -359,18 +385,11 @@ static char *openvzDomainGetOSType(virDomainPtr dom)
virDomainObjPtr vm;
char *ret = NULL;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return NULL;
ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(vm->def->os.type)));
- cleanup:
if (vm)
virObjectUnlock(vm);
return ret;
@@ -384,18 +403,11 @@ static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn,
virDomainObjPtr vm;
virDomainPtr dom = NULL;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, uuid)))
+ return NULL;
dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
- cleanup:
if (vm)
virObjectUnlock(vm);
return dom;
@@ -432,15 +444,8 @@ static int openvzDomainGetInfo(virDomainPtr dom,
int state;
int ret = -1;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
if (openvzGetVEStatus(vm, &state, NULL) == -1)
goto cleanup;
@@ -480,19 +485,11 @@ openvzDomainGetState(virDomainPtr dom,
virCheckFlags(0, -1);
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
ret = openvzGetVEStatus(vm, state, reason);
- cleanup:
if (vm)
virObjectUnlock(vm);
return ret;
@@ -505,16 +502,11 @@ static int openvzDomainIsActive(virDomainPtr dom)
virDomainObjPtr obj;
int ret = -1;
- openvzDriverLock(driver);
- obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
- if (!obj) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
- goto cleanup;
- }
+ if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
+
ret = virDomainObjIsActive(obj);
- cleanup:
if (obj)
virObjectUnlock(obj);
return ret;
@@ -527,16 +519,11 @@ static int openvzDomainIsPersistent(virDomainPtr dom)
virDomainObjPtr obj;
int ret = -1;
- openvzDriverLock(driver);
- obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
- if (!obj) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
- goto cleanup;
- }
+ if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
+
ret = obj->persistent;
- cleanup:
if (obj)
virObjectUnlock(obj);
return ret;
@@ -554,20 +541,12 @@ static char *openvzDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
/* Flags checked by virDomainDefFormat */
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return NULL;
ret = virDomainDefFormat(vm->def, driver->caps,
virDomainDefFormatConvertXMLFlags(flags));
- cleanup:
if (vm)
virObjectUnlock(vm);
return ret;
@@ -600,15 +579,8 @@ static int openvzDomainSuspend(virDomainPtr dom)
const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--suspend", NULL};
int ret = -1;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -638,15 +610,8 @@ static int openvzDomainResume(virDomainPtr dom)
const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--resume", NULL};
int ret = -1;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -681,15 +646,8 @@ openvzDomainShutdownFlags(virDomainPtr dom,
virCheckFlags(0, -1);
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
if (openvzGetVEStatus(vm, &status, NULL) == -1)
goto cleanup;
@@ -744,15 +702,8 @@ static int openvzDomainReboot(virDomainPtr dom,
virCheckFlags(0, -1);
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
if (openvzGetVEStatus(vm, &status, NULL) == -1)
goto cleanup;
@@ -1210,12 +1161,8 @@ openvzDomainUndefineFlags(virDomainPtr dom,
virCheckFlags(0, -1);
openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
goto cleanup;
- }
if (openvzGetVEStatus(vm, &status, NULL) == -1)
goto cleanup;
@@ -1255,15 +1202,8 @@ openvzDomainSetAutostart(virDomainPtr dom, int autostart)
"--save", NULL };
int ret = -1;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
openvzSetProgramSentinal(prog, vm->def->name);
if (virRun(prog, NULL) < 0)
@@ -1284,15 +1224,8 @@ openvzDomainGetAutostart(virDomainPtr dom, int *autostart)
char *value = NULL;
int ret = -1;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
if (openvzReadVPSConfigParam(strtoI(vm->def->name), "ONBOOT", &value) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1383,15 +1316,8 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
return -1;
}
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
if (nvcpus <= 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1987,17 +1913,8 @@ openvzDomainInterfaceStats(virDomainPtr dom,
virDomainNetDefPtr net = NULL;
int ret = -1;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"), uuidstr);
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2082,13 +1999,8 @@ openvzDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
goto cleanup;
- }
if (virStrToLong_i(vm->def->name, NULL, 10, &veid) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -2230,15 +2142,8 @@ openvzDomainMigrateBegin3Params(virDomainPtr domain,
if (virTypedParamsValidate(params, nparams, OPENVZ_MIGRATION_PARAMETERS) < 0)
return NULL;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
- goto cleanup;
- }
+ if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
+ return NULL;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2394,15 +2299,8 @@ openvzDomainMigratePerform3Params(virDomainPtr domain,
&uri_str) < 0)
goto cleanup;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
goto cleanup;
- }
/* parse dst host:port from uri */
uri = virURIParse(uri_str);
@@ -2504,15 +2402,8 @@ openvzDomainMigrateConfirm3Params(virDomainPtr domain,
if (virTypedParamsValidate(params, nparams, OPENVZ_MIGRATION_PARAMETERS) < 0)
goto cleanup;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
- openvzDriverUnlock(driver);
-
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
goto cleanup;
- }
if (cancelled) {
if (openvzGetVEStatus(vm, &status, NULL) == -1)
@@ -2552,16 +2443,11 @@ openvzDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
virCheckFlags(0, -1);
- openvzDriverLock(driver);
- obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- openvzDriverUnlock(driver);
- if (!obj) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
- goto cleanup;
- }
+ if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
+ return -1;
+
ret = 0;
- cleanup:
if (obj)
virObjectUnlock(obj);
return ret;
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/09/2018 09:48 AM, John Ferlan wrote:
> Rather than repeat code throughout, create and use a couple of
> accessors in order to lookup by UUID.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> src/openvz/openvz_driver.c | 266 +++++++++++++--------------------------------
> 1 file changed, 76 insertions(+), 190 deletions(-)
>
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index a211c370e..167ba2f7a 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
> @@ -87,6 +87,39 @@ static void openvzDriverUnlock(struct openvz_driver *driver)
>
> struct openvz_driver ovz_driver;
>
> +
> +static virDomainObjPtr
> +openvzDomObjFromDomainLocked(struct openvz_driver *driver,
> + const unsigned char *uuid)
> +{
> + virDomainObjPtr vm;
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> +
> + if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
> + virUUIDFormat(uuid, uuidstr);
> +
> + virReportError(VIR_ERR_NO_DOMAIN,
> + _("no domain with matching uuid '%s'"), uuidstr);
> + return NULL;
> + }
> +
> + return vm;
> +}
> +
> +
> +static virDomainObjPtr
> +openvzDomObjFromDomain(struct openvz_driver *driver,
> + const unsigned char *uuid)
> +{
> + virDomainObjPtr vm;
> +
> + openvzDriverLock(driver);
> + vm = openvzDomObjFromDomainLocked(driver, uuid);
> + openvzDriverUnlock(driver);
> + return vm;
> +}
> +
> +
> static int
> openvzDomainDefPostParse(virDomainDefPtr def,
> virCapsPtr caps ATTRIBUTE_UNUSED,
> @@ -281,15 +314,8 @@ openvzDomainGetHostname(virDomainPtr dom, unsigned int flags)
> virDomainObjPtr vm;
>
> virCheckFlags(0, NULL);
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return NULL;
It looks like the cleanup label can be removed from this function too.
>
> hostname = openvzVEGetStringParam(dom, "hostname");
> if (hostname == NULL)
> @@ -359,18 +385,11 @@ static char *openvzDomainGetOSType(virDomainPtr dom)
> virDomainObjPtr vm;
> char *ret = NULL;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return NULL;
>
> ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(vm->def->os.type)));
>
> - cleanup:
> if (vm)
> virObjectUnlock(vm);
> return ret;
> @@ -384,18 +403,11 @@ static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn,
> virDomainObjPtr vm;
> virDomainPtr dom = NULL;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, uuid)))
> + return NULL;
>
> dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
>
> - cleanup:
> if (vm)
> virObjectUnlock(vm);
> return dom;
> @@ -432,15 +444,8 @@ static int openvzDomainGetInfo(virDomainPtr dom,
> int state;
> int ret = -1;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> if (openvzGetVEStatus(vm, &state, NULL) == -1)
> goto cleanup;
> @@ -480,19 +485,11 @@ openvzDomainGetState(virDomainPtr dom,
>
> virCheckFlags(0, -1);
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> ret = openvzGetVEStatus(vm, state, reason);
>
> - cleanup:
> if (vm)
> virObjectUnlock(vm);
> return ret;
> @@ -505,16 +502,11 @@ static int openvzDomainIsActive(virDomainPtr dom)
> virDomainObjPtr obj;
> int ret = -1;
>
> - openvzDriverLock(driver);
> - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> - if (!obj) {
> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
> - goto cleanup;
> - }
> + if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
> +
> ret = virDomainObjIsActive(obj);
>
> - cleanup:
> if (obj)
> virObjectUnlock(obj);
> return ret;
> @@ -527,16 +519,11 @@ static int openvzDomainIsPersistent(virDomainPtr dom)
> virDomainObjPtr obj;
> int ret = -1;
>
> - openvzDriverLock(driver);
> - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> - if (!obj) {
> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
> - goto cleanup;
> - }
> + if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
> +
> ret = obj->persistent;
>
> - cleanup:
> if (obj)
> virObjectUnlock(obj);
> return ret;
> @@ -554,20 +541,12 @@ static char *openvzDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
>
> /* Flags checked by virDomainDefFormat */
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return NULL;
>
> ret = virDomainDefFormat(vm->def, driver->caps,
> virDomainDefFormatConvertXMLFlags(flags));
>
> - cleanup:
> if (vm)
> virObjectUnlock(vm);
> return ret;
> @@ -600,15 +579,8 @@ static int openvzDomainSuspend(virDomainPtr dom)
> const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--suspend", NULL};
> int ret = -1;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> if (!virDomainObjIsActive(vm)) {
> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> @@ -638,15 +610,8 @@ static int openvzDomainResume(virDomainPtr dom)
> const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--resume", NULL};
> int ret = -1;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> if (!virDomainObjIsActive(vm)) {
> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> @@ -681,15 +646,8 @@ openvzDomainShutdownFlags(virDomainPtr dom,
>
> virCheckFlags(0, -1);
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> if (openvzGetVEStatus(vm, &status, NULL) == -1)
> goto cleanup;
> @@ -744,15 +702,8 @@ static int openvzDomainReboot(virDomainPtr dom,
>
> virCheckFlags(0, -1);
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> if (openvzGetVEStatus(vm, &status, NULL) == -1)
> goto cleanup;
> @@ -1210,12 +1161,8 @@ openvzDomainUndefineFlags(virDomainPtr dom,
> virCheckFlags(0, -1);
>
> openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> + if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
> goto cleanup;
> - }
Not related to your goal, but I wonder why the pattern here is different? Why
does the driver need to be locked during the entire undefine operation?
>
> if (openvzGetVEStatus(vm, &status, NULL) == -1)
> goto cleanup;
> @@ -1255,15 +1202,8 @@ openvzDomainSetAutostart(virDomainPtr dom, int autostart)
> "--save", NULL };
> int ret = -1;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> openvzSetProgramSentinal(prog, vm->def->name);
> if (virRun(prog, NULL) < 0)
> @@ -1284,15 +1224,8 @@ openvzDomainGetAutostart(virDomainPtr dom, int *autostart)
> char *value = NULL;
> int ret = -1;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> if (openvzReadVPSConfigParam(strtoI(vm->def->name), "ONBOOT", &value) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -1383,15 +1316,8 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
> return -1;
> }
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> if (nvcpus <= 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -1987,17 +1913,8 @@ openvzDomainInterfaceStats(virDomainPtr dom,
> virDomainNetDefPtr net = NULL;
> int ret = -1;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> - virUUIDFormat(dom->uuid, uuidstr);
> - virReportError(VIR_ERR_NO_DOMAIN,
> - _("no domain with matching uuid '%s'"), uuidstr);
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
>
> if (!virDomainObjIsActive(vm)) {
> virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -2082,13 +1999,8 @@ openvzDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
> VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
>
> openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> + if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
> goto cleanup;
> - }
Same here. It's odd that the driver needs to be locked for the whole operation.
But it's orthogonal to your work.
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Regards,
Jim
>
> if (virStrToLong_i(vm->def->name, NULL, 10, &veid) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -2230,15 +2142,8 @@ openvzDomainMigrateBegin3Params(virDomainPtr domain,
> if (virTypedParamsValidate(params, nparams, OPENVZ_MIGRATION_PARAMETERS) < 0)
> return NULL;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> - goto cleanup;
> - }
> + if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
> + return NULL;
>
> if (!virDomainObjIsActive(vm)) {
> virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -2394,15 +2299,8 @@ openvzDomainMigratePerform3Params(virDomainPtr domain,
> &uri_str) < 0)
> goto cleanup;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> + if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
> goto cleanup;
> - }
>
> /* parse dst host:port from uri */
> uri = virURIParse(uri_str);
> @@ -2504,15 +2402,8 @@ openvzDomainMigrateConfirm3Params(virDomainPtr domain,
> if (virTypedParamsValidate(params, nparams, OPENVZ_MIGRATION_PARAMETERS) < 0)
> goto cleanup;
>
> - openvzDriverLock(driver);
> - vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
> - openvzDriverUnlock(driver);
> -
> - if (!vm) {
> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
> - _("no domain with matching uuid"));
> + if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
> goto cleanup;
> - }
>
> if (cancelled) {
> if (openvzGetVEStatus(vm, &status, NULL) == -1)
> @@ -2552,16 +2443,11 @@ openvzDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
>
> virCheckFlags(0, -1);
>
> - openvzDriverLock(driver);
> - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> - openvzDriverUnlock(driver);
> - if (!obj) {
> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
> - goto cleanup;
> - }
> + if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
> + return -1;
> +
> ret = 0;
>
> - cleanup:
> if (obj)
> virObjectUnlock(obj);
> return ret;
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/29/2018 01:36 PM, Jim Fehlig wrote:
> On 03/09/2018 09:48 AM, John Ferlan wrote:
>> Rather than repeat code throughout, create and use a couple of
>> accessors in order to lookup by UUID.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> src/openvz/openvz_driver.c | 266
>> +++++++++++++--------------------------------
>> 1 file changed, 76 insertions(+), 190 deletions(-)
>>
>> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
>> index a211c370e..167ba2f7a 100644
>> --- a/src/openvz/openvz_driver.c
>> +++ b/src/openvz/openvz_driver.c
>> @@ -87,6 +87,39 @@ static void openvzDriverUnlock(struct openvz_driver
>> *driver)
>> struct openvz_driver ovz_driver;
>> +
>> +static virDomainObjPtr
>> +openvzDomObjFromDomainLocked(struct openvz_driver *driver,
>> + const unsigned char *uuid)
>> +{
>> + virDomainObjPtr vm;
>> + char uuidstr[VIR_UUID_STRING_BUFLEN];
>> +
>> + if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
>> + virUUIDFormat(uuid, uuidstr);
>> +
>> + virReportError(VIR_ERR_NO_DOMAIN,
>> + _("no domain with matching uuid '%s'"), uuidstr);
>> + return NULL;
>> + }
>> +
>> + return vm;
>> +}
>> +
>> +
>> +static virDomainObjPtr
>> +openvzDomObjFromDomain(struct openvz_driver *driver,
>> + const unsigned char *uuid)
>> +{
>> + virDomainObjPtr vm;
>> +
>> + openvzDriverLock(driver);
>> + vm = openvzDomObjFromDomainLocked(driver, uuid);
>> + openvzDriverUnlock(driver);
>> + return vm;
>> +}
>> +
>> +
>> static int
>> openvzDomainDefPostParse(virDomainDefPtr def,
>> virCapsPtr caps ATTRIBUTE_UNUSED,
>> @@ -281,15 +314,8 @@ openvzDomainGetHostname(virDomainPtr dom,
>> unsigned int flags)
>> virDomainObjPtr vm;
>> virCheckFlags(0, NULL);
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return NULL;
>
> It looks like the cleanup label can be removed from this function too.
>
True, but it's not "as clean" as others since error does the goto clean
thing. The others where the cleanup: label disappears is because there
was only one goto cleanup... and the "if (vm)" would be useless too, but
that's handled in a couple of patches anyway.
In any case, I'll add an adjustment with my eventual followup series
that alters virDomainObjAdd processing.
>> hostname = openvzVEGetStringParam(dom, "hostname");
>> if (hostname == NULL)
>> @@ -359,18 +385,11 @@ static char *openvzDomainGetOSType(virDomainPtr
>> dom)
>> virDomainObjPtr vm;
>> char *ret = NULL;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return NULL;
>> ignore_value(VIR_STRDUP(ret,
>> virDomainOSTypeToString(vm->def->os.type)));
>> - cleanup:
>> if (vm)
>> virObjectUnlock(vm);
>> return ret;
>> @@ -384,18 +403,11 @@ static virDomainPtr
>> openvzDomainLookupByUUID(virConnectPtr conn,
>> virDomainObjPtr vm;
>> virDomainPtr dom = NULL;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, uuid)))
>> + return NULL;
>> dom = virGetDomain(conn, vm->def->name, vm->def->uuid,
>> vm->def->id);
>> - cleanup:
>> if (vm)
>> virObjectUnlock(vm);
>> return dom;
>> @@ -432,15 +444,8 @@ static int openvzDomainGetInfo(virDomainPtr dom,
>> int state;
>> int ret = -1;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> if (openvzGetVEStatus(vm, &state, NULL) == -1)
>> goto cleanup;
>> @@ -480,19 +485,11 @@ openvzDomainGetState(virDomainPtr dom,
>> virCheckFlags(0, -1);
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> ret = openvzGetVEStatus(vm, state, reason);
>> - cleanup:
>> if (vm)
>> virObjectUnlock(vm);
>> return ret;
>> @@ -505,16 +502,11 @@ static int openvzDomainIsActive(virDomainPtr dom)
>> virDomainObjPtr obj;
>> int ret = -1;
>> - openvzDriverLock(driver);
>> - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> - if (!obj) {
>> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
>> - goto cleanup;
>> - }
>> + if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> +
>> ret = virDomainObjIsActive(obj);
>> - cleanup:
>> if (obj)
>> virObjectUnlock(obj);
>> return ret;
>> @@ -527,16 +519,11 @@ static int openvzDomainIsPersistent(virDomainPtr
>> dom)
>> virDomainObjPtr obj;
>> int ret = -1;
>> - openvzDriverLock(driver);
>> - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> - if (!obj) {
>> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
>> - goto cleanup;
>> - }
>> + if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> +
>> ret = obj->persistent;
>> - cleanup:
>> if (obj)
>> virObjectUnlock(obj);
>> return ret;
>> @@ -554,20 +541,12 @@ static char *openvzDomainGetXMLDesc(virDomainPtr
>> dom, unsigned int flags) {
>> /* Flags checked by virDomainDefFormat */
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return NULL;
>> ret = virDomainDefFormat(vm->def, driver->caps,
>> virDomainDefFormatConvertXMLFlags(flags));
>> - cleanup:
>> if (vm)
>> virObjectUnlock(vm);
>> return ret;
>> @@ -600,15 +579,8 @@ static int openvzDomainSuspend(virDomainPtr dom)
>> const char *prog[] = {VZCTL, "--quiet", "chkpnt",
>> PROGRAM_SENTINEL, "--suspend", NULL};
>> int ret = -1;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> if (!virDomainObjIsActive(vm)) {
>> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> @@ -638,15 +610,8 @@ static int openvzDomainResume(virDomainPtr dom)
>> const char *prog[] = {VZCTL, "--quiet", "chkpnt",
>> PROGRAM_SENTINEL, "--resume", NULL};
>> int ret = -1;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> if (!virDomainObjIsActive(vm)) {
>> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> @@ -681,15 +646,8 @@ openvzDomainShutdownFlags(virDomainPtr dom,
>> virCheckFlags(0, -1);
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> if (openvzGetVEStatus(vm, &status, NULL) == -1)
>> goto cleanup;
>> @@ -744,15 +702,8 @@ static int openvzDomainReboot(virDomainPtr dom,
>> virCheckFlags(0, -1);
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> if (openvzGetVEStatus(vm, &status, NULL) == -1)
>> goto cleanup;
>> @@ -1210,12 +1161,8 @@ openvzDomainUndefineFlags(virDomainPtr dom,
>> virCheckFlags(0, -1);
>> openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> + if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
>> goto cleanup;
>> - }
>
> Not related to your goal, but I wonder why the pattern here is
> different? Why does the driver need to be locked during the entire
> undefine operation?
>
My assumption, old driver, no one updated it once the domainobjlist code
became self locking. I will add it to a future adjustment when I have to
mess with the Add code
Tks -
John
>> if (openvzGetVEStatus(vm, &status, NULL) == -1)
>> goto cleanup;
>> @@ -1255,15 +1202,8 @@ openvzDomainSetAutostart(virDomainPtr dom, int
>> autostart)
>> "--save", NULL };
>> int ret = -1;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> openvzSetProgramSentinal(prog, vm->def->name);
>> if (virRun(prog, NULL) < 0)
>> @@ -1284,15 +1224,8 @@ openvzDomainGetAutostart(virDomainPtr dom, int
>> *autostart)
>> char *value = NULL;
>> int ret = -1;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> if (openvzReadVPSConfigParam(strtoI(vm->def->name), "ONBOOT",
>> &value) < 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> @@ -1383,15 +1316,8 @@ static int
>> openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
>> return -1;
>> }
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> if (nvcpus <= 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> @@ -1987,17 +1913,8 @@ openvzDomainInterfaceStats(virDomainPtr dom,
>> virDomainNetDefPtr net = NULL;
>> int ret = -1;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - char uuidstr[VIR_UUID_STRING_BUFLEN];
>> - virUUIDFormat(dom->uuid, uuidstr);
>> - virReportError(VIR_ERR_NO_DOMAIN,
>> - _("no domain with matching uuid '%s'"), uuidstr);
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> if (!virDomainObjIsActive(vm)) {
>> virReportError(VIR_ERR_OPERATION_INVALID,
>> @@ -2082,13 +1999,8 @@ openvzDomainUpdateDeviceFlags(virDomainPtr dom,
>> const char *xml,
>> VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
>> openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> + if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
>> goto cleanup;
>> - }
>
> Same here. It's odd that the driver needs to be locked for the whole
> operation. But it's orthogonal to your work.
>
> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
>
> Regards,
> Jim
>
>> if (virStrToLong_i(vm->def->name, NULL, 10, &veid) < 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> @@ -2230,15 +2142,8 @@ openvzDomainMigrateBegin3Params(virDomainPtr
>> domain,
>> if (virTypedParamsValidate(params, nparams,
>> OPENVZ_MIGRATION_PARAMETERS) < 0)
>> return NULL;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> - goto cleanup;
>> - }
>> + if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
>> + return NULL;
>> if (!virDomainObjIsActive(vm)) {
>> virReportError(VIR_ERR_OPERATION_INVALID,
>> @@ -2394,15 +2299,8 @@ openvzDomainMigratePerform3Params(virDomainPtr
>> domain,
>> &uri_str) < 0)
>> goto cleanup;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> + if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
>> goto cleanup;
>> - }
>> /* parse dst host:port from uri */
>> uri = virURIParse(uri_str);
>> @@ -2504,15 +2402,8 @@ openvzDomainMigrateConfirm3Params(virDomainPtr
>> domain,
>> if (virTypedParamsValidate(params, nparams,
>> OPENVZ_MIGRATION_PARAMETERS) < 0)
>> goto cleanup;
>> - openvzDriverLock(driver);
>> - vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
>> - openvzDriverUnlock(driver);
>> -
>> - if (!vm) {
>> - virReportError(VIR_ERR_NO_DOMAIN, "%s",
>> - _("no domain with matching uuid"));
>> + if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
>> goto cleanup;
>> - }
>> if (cancelled) {
>> if (openvzGetVEStatus(vm, &status, NULL) == -1)
>> @@ -2552,16 +2443,11 @@ openvzDomainHasManagedSaveImage(virDomainPtr
>> dom, unsigned int flags)
>> virCheckFlags(0, -1);
>> - openvzDriverLock(driver);
>> - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
>> - openvzDriverUnlock(driver);
>> - if (!obj) {
>> - virReportError(VIR_ERR_NO_DOMAIN, NULL);
>> - goto cleanup;
>> - }
>> + if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
>> + return -1;
>> +
>> ret = 0;
>> - cleanup:
>> if (obj)
>> virObjectUnlock(obj);
>> return ret;
>>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.