[libvirt] [PATCH 0/2] Slightly rework domain ID handling

The first patch fixes an actual bug: our virConnectGetAllDomainStats API does not fill out the domain ID, therefore users just see domain name and UUID. And while fixing it I realized, that our virGetDomain function can do better. However, given that we are in the freeze, I'd like to push just the first patch and save the second for after the release as it touches a lot of places and thus has a great potential o breaking things. Although, in my testing it actually fixed some other places that we forgot to set domain ID. Michal Privoznik (2): qemuDomainGetStats: Copy domain ID too virGetDomain: Set domain ID too daemon/remote.c | 5 +---- src/bhyve/bhyve_driver.c | 20 +++++------------- src/conf/domain_event.c | 11 +++++----- src/conf/virdomainobjlist.c | 10 +++------ src/datatypes.c | 8 ++++++-- src/datatypes.h | 3 ++- src/esx/esx_driver.c | 49 ++++++++++++++------------------------------- src/hyperv/hyperv_wmi.c | 15 +++++--------- src/libxl/libxl_driver.c | 20 +++++------------- src/libxl/libxl_migration.c | 2 +- src/lxc/lxc_driver.c | 20 +++++------------- src/openvz/openvz_driver.c | 24 ++++++---------------- src/phyp/phyp_driver.c | 12 +++-------- src/qemu/qemu_driver.c | 23 ++++++++------------- src/qemu/qemu_migration.c | 4 ++-- src/remote/remote_driver.c | 5 +---- src/test/test_driver.c | 20 +++++------------- src/uml/uml_driver.c | 15 +++++--------- src/vbox/vbox_common.c | 44 +++++++++++++++------------------------- src/vmware/vmware_driver.c | 20 +++++------------- src/vz/vz_driver.c | 22 ++++++-------------- src/xen/xen_driver.c | 28 ++++++-------------------- src/xenapi/xenapi_driver.c | 30 +++++++++++---------------- 23 files changed, 129 insertions(+), 281 deletions(-) -- 2.10.2

One of the problems with our virGetDomain function is that it copies just domain name and domain UUID. Therefore it's very easy to forget aboud domain ID. This can cause some bugs, like virConnectGetAllDomainStats not reporting proper domain IDs. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d5fc00e..40c2eab 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19556,6 +19556,9 @@ qemuDomainGetStats(virConnectPtr conn, if (!(tmp->dom = virGetDomain(conn, dom->def->name, dom->def->uuid))) goto cleanup; + /* We have to copy the domain ID by hand */ + tmp->dom->id = dom->def->id; + *record = tmp; tmp = NULL; ret = 0; -- 2.10.2

On Tue, Mar 28, 2017 at 05:15:07PM +0200, Michal Privoznik wrote:
One of the problems with our virGetDomain function is that it copies just domain name and domain UUID. Therefore it's very easy to forget aboud domain ID. This can cause some bugs, like virConnectGetAllDomainStats not reporting proper domain IDs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d5fc00e..40c2eab 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19556,6 +19556,9 @@ qemuDomainGetStats(virConnectPtr conn, if (!(tmp->dom = virGetDomain(conn, dom->def->name, dom->def->uuid))) goto cleanup;
+ /* We have to copy the domain ID by hand */ + tmp->dom->id = dom->def->id; + *record = tmp; tmp = NULL; ret = 0;
ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v

So far our code is full of the following pattern: dom = virGetDomain(conn, name, uuid) if (dom) dom->id = 42; There is no reasong why it couldn't be just: dom = virGetDomain(conn, name, uuid, id); After all, client domain representation consists of tuple (name, uuid, id). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- daemon/remote.c | 5 +---- src/bhyve/bhyve_driver.c | 20 +++++------------- src/conf/domain_event.c | 11 +++++----- src/conf/virdomainobjlist.c | 10 +++------ src/datatypes.c | 8 ++++++-- src/datatypes.h | 3 ++- src/esx/esx_driver.c | 49 ++++++++++++++------------------------------- src/hyperv/hyperv_wmi.c | 15 +++++--------- src/libxl/libxl_driver.c | 20 +++++------------- src/libxl/libxl_migration.c | 2 +- src/lxc/lxc_driver.c | 20 +++++------------- src/openvz/openvz_driver.c | 24 ++++++---------------- src/phyp/phyp_driver.c | 12 +++-------- src/qemu/qemu_driver.c | 26 ++++++++---------------- src/qemu/qemu_migration.c | 4 ++-- src/remote/remote_driver.c | 5 +---- src/test/test_driver.c | 20 +++++------------- src/uml/uml_driver.c | 15 +++++--------- src/vbox/vbox_common.c | 44 +++++++++++++++------------------------- src/vmware/vmware_driver.c | 20 +++++------------- src/vz/vz_driver.c | 22 ++++++-------------- src/xen/xen_driver.c | 28 ++++++-------------------- src/xenapi/xenapi_driver.c | 30 +++++++++++---------------- 23 files changed, 129 insertions(+), 284 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 5696b43..1610fea 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6964,13 +6964,10 @@ remoteDispatchStorageVolGetInfoFlags(virNetServerPtr server ATTRIBUTE_UNUSED, static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain domain) { - virDomainPtr dom; - dom = virGetDomain(conn, domain.name, BAD_CAST domain.uuid); /* Should we believe the domain.id sent by the client? Maybe * this should be a check rather than an assignment? XXX */ - if (dom) dom->id = domain.id; - return dom; + return virGetDomain(conn, domain.name, BAD_CAST domain.uuid, domain.id); } static virNetworkPtr diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 74cc5ab..ed2221a 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -570,9 +570,7 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag VIR_DOMAIN_EVENT_DEFINED_ADDED : VIR_DOMAIN_EVENT_DEFINED_UPDATED); - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virObjectUnref(caps); @@ -817,9 +815,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn, if (virDomainLookupByUUIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -845,9 +841,7 @@ static virDomainPtr bhyveDomainLookupByName(virConnectPtr conn, if (virDomainLookupByNameEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); @@ -873,9 +867,7 @@ bhyveDomainLookupByID(virConnectPtr conn, if (virDomainLookupByIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -991,9 +983,7 @@ bhyveDomainCreateXML(virConnectPtr conn, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virObjectUnref(caps); diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 6243b42..c15f32a 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1764,10 +1764,11 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, virConnectObjectEventGenericCallback cb, void *cbopaque) { - virDomainPtr dom = virGetDomain(conn, event->meta.name, event->meta.uuid); + virDomainPtr dom = virGetDomain(conn, event->meta.name, + event->meta.uuid, event->meta.id); + if (!dom) return; - dom->id = event->meta.id; switch ((virDomainEventID) event->eventID) { case VIR_DOMAIN_EVENT_ID_LIFECYCLE: @@ -2108,13 +2109,13 @@ virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, virConnectObjectEventGenericCallback cb, void *cbopaque) { - virDomainPtr dom = virGetDomain(conn, event->meta.name, event->meta.uuid); + virDomainPtr dom; virDomainQemuMonitorEventPtr qemuMonitorEvent; virDomainQemuMonitorEventData *data = cbopaque; - if (!dom) + if (!(dom = virGetDomain(conn, event->meta.name, + event->meta.uuid, event->meta.id))) return; - dom->id = event->meta.id; qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event; ((virConnectDomainQemuMonitorEventCallback)cb)(conn, dom, diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 8d425d6..c121382 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -1027,15 +1027,11 @@ virDomainObjListExport(virDomainObjListPtr domlist, virDomainObjPtr vm = vms[i]; virObjectLock(vm); + doms[i] = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); + virObjectUnlock(vm); - if (!(doms[i] = virGetDomain(conn, vm->def->name, vm->def->uuid))) { - virObjectUnlock(vm); + if (!doms[i]) goto cleanup; - } - - doms[i]->id = vm->def->id; - - virObjectUnlock(vm); } *domains = doms; diff --git a/src/datatypes.c b/src/datatypes.c index fe8b9c2..3e3148d 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -261,6 +261,7 @@ virConnectCloseCallbackDataGetCallback(virConnectCloseCallbackDataPtr closeData) * @conn: the hypervisor connection * @name: pointer to the domain name * @uuid: pointer to the uuid + * @id: domain ID * * Allocates a new domain object. When the object is no longer needed, * virObjectUnref() must be called in order to not leak data. @@ -268,7 +269,10 @@ virConnectCloseCallbackDataGetCallback(virConnectCloseCallbackDataPtr closeData) * Returns a pointer to the domain object, or NULL on error. */ virDomainPtr -virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) +virGetDomain(virConnectPtr conn, + const char *name, + const unsigned char *uuid, + int id) { virDomainPtr ret = NULL; @@ -286,7 +290,7 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) goto error; ret->conn = virObjectRef(conn); - ret->id = -1; + ret->id = id; memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); return ret; diff --git a/src/datatypes.h b/src/datatypes.h index d525835..288e057 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -683,7 +683,8 @@ struct _virNWFilter { virConnectPtr virGetConnect(void); virDomainPtr virGetDomain(virConnectPtr conn, const char *name, - const unsigned char *uuid); + const unsigned char *uuid, + int id); virNetworkPtr virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid); diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 166d4bc..6333855 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1509,13 +1509,11 @@ esxDomainLookupByID(virConnectPtr conn, int id) if (id != id_candidate) continue; - domain = virGetDomain(conn, name_candidate, uuid_candidate); + domain = virGetDomain(conn, name_candidate, uuid_candidate, id); if (!domain) goto cleanup; - domain->id = id; - break; } @@ -1557,17 +1555,11 @@ esxDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) goto cleanup; } - domain = virGetDomain(conn, name, uuid); - - if (!domain) - goto cleanup; - /* Only running/suspended virtual machines have an ID != -1 */ - if (powerState != esxVI_VirtualMachinePowerState_PoweredOff) { - domain->id = id; - } else { - domain->id = -1; - } + if (powerState == esxVI_VirtualMachinePowerState_PoweredOff) + id = -1; + + domain = virGetDomain(conn, name, uuid, id); cleanup: esxVI_String_Free(&propertyNameList); @@ -1613,17 +1605,11 @@ esxDomainLookupByName(virConnectPtr conn, const char *name) goto cleanup; } - domain = virGetDomain(conn, name, uuid); - - if (!domain) - goto cleanup; - /* Only running/suspended virtual machines have an ID != -1 */ - if (powerState != esxVI_VirtualMachinePowerState_PoweredOff) { - domain->id = id; - } else { - domain->id = -1; - } + if (powerState == esxVI_VirtualMachinePowerState_PoweredOff) + id = -1; + + domain = virGetDomain(conn, name, uuid, id); cleanup: esxVI_String_Free(&propertyNameList); @@ -3208,10 +3194,7 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; } - domain = virGetDomain(conn, def->name, def->uuid); - - if (domain) - domain->id = -1; + domain = virGetDomain(conn, def->name, def->uuid, -1); /* FIXME: Add proper rollback in case of an error */ @@ -5105,14 +5088,12 @@ esxConnectListAllDomains(virConnectPtr conn, if (VIR_RESIZE_N(doms, ndoms, count, 2) < 0) goto cleanup; - if (!(dom = virGetDomain(conn, name, uuid))) - goto cleanup; - /* Only running/suspended virtual machines have an ID != -1 */ - if (powerState != esxVI_VirtualMachinePowerState_PoweredOff) - dom->id = id; - else - dom->id = -1; + if (powerState == esxVI_VirtualMachinePowerState_PoweredOff) + id = -1; + + if (!(dom = virGetDomain(conn, name, uuid, id))) + goto cleanup; doms[count++] = dom; } diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 13acd09..ef1cf25 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -608,6 +608,7 @@ hypervMsvmComputerSystemToDomain(virConnectPtr conn, virDomainPtr *domain) { unsigned char uuid[VIR_UUID_BUFLEN]; + int id = -1; if (domain == NULL || *domain != NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -621,18 +622,12 @@ hypervMsvmComputerSystemToDomain(virConnectPtr conn, return -1; } - *domain = virGetDomain(conn, computerSystem->data->ElementName, uuid); + if (hypervIsMsvmComputerSystemActive(computerSystem, NULL)) + id = computerSystem->data->ProcessID; - if (*domain == NULL) - return -1; + *domain = virGetDomain(conn, computerSystem->data->ElementName, uuid, id); - if (hypervIsMsvmComputerSystemActive(computerSystem, NULL)) { - (*domain)->id = computerSystem->data->ProcessID; - } else { - (*domain)->id = -1; - } - - return 0; + return *domain ? 0 : -1; } int diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index e5b8f48..0d65342 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1072,9 +1072,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, goto endjob; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); endjob: libxlDomainObjEndJob(driver, vm); @@ -1102,9 +1100,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id) if (virDomainLookupByIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -1128,9 +1124,7 @@ libxlDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (virDomainLookupByUUIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -1154,9 +1148,7 @@ libxlDomainLookupByName(virConnectPtr conn, const char *name) if (virDomainLookupByNameEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); @@ -2825,9 +2817,7 @@ libxlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, !oldDef ? diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 073cfda..228c850 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -1322,7 +1322,7 @@ libxlDomainMigrationFinish(virConnectPtr dconn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, cfg->caps) < 0) goto cleanup; - dom = virGetDomain(dconn, vm->def->name, vm->def->uuid); + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (dom == NULL) { diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 27b4b75..22c8b58 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -267,9 +267,7 @@ static virDomainPtr lxcDomainLookupByID(virConnectPtr conn, if (virDomainLookupByIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -297,9 +295,7 @@ static virDomainPtr lxcDomainLookupByUUID(virConnectPtr conn, if (virDomainLookupByUUIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); @@ -323,9 +319,7 @@ static virDomainPtr lxcDomainLookupByName(virConnectPtr conn, if (virDomainLookupByNameEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); @@ -509,9 +503,7 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) VIR_DOMAIN_EVENT_DEFINED_ADDED : VIR_DOMAIN_EVENT_DEFINED_UPDATED); - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainDefFree(def); @@ -1277,9 +1269,7 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, VIR_DOMAIN_EVENT_STARTED_BOOTED); virDomainAuditStart(vm, "booted", true); - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); virLXCDomainObjEndJob(driver, vm); diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 647c852..9c4a196 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -329,9 +329,7 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -395,9 +393,7 @@ static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn, goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -421,9 +417,7 @@ static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); @@ -1053,9 +1047,7 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla } } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = -1; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, -1); cleanup: virDomainDefFree(vmdef); @@ -1144,9 +1136,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, } } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainDefFree(vmdef); @@ -2496,9 +2486,7 @@ openvzDomainMigrateFinish3Params(virConnectPtr dconn, vm->def->id = strtoI(vm->def->name); virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_MIGRATED); - dom = virGetDomain(dconn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 1803aa5..2123784 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3214,10 +3214,7 @@ phypDomainLookupByName(virConnectPtr conn, const char *lpar_name) if (phypGetLparUUID(lpar_uuid, lpar_id, conn) == -1) return NULL; - dom = virGetDomain(conn, lpar_name, lpar_uuid); - - if (dom) - dom->id = lpar_id; + dom = virGetDomain(conn, lpar_name, lpar_uuid, lpar_id); return dom; } @@ -3237,10 +3234,7 @@ phypDomainLookupByID(virConnectPtr conn, int lpar_id) if (phypGetLparUUID(lpar_uuid, lpar_id, conn) == -1) goto cleanup; - dom = virGetDomain(conn, lpar_name, lpar_uuid); - - if (dom) - dom->id = lpar_id; + dom = virGetDomain(conn, lpar_name, lpar_uuid, lpar_id); cleanup: VIR_FREE(lpar_name); @@ -3593,7 +3587,7 @@ phypDomainCreateXML(virConnectPtr conn, } } - if ((dom = virGetDomain(conn, def->name, def->uuid)) == NULL) + if ((dom = virGetDomain(conn, def->name, def->uuid, def->id)) == NULL) goto err; if (phypBuildLpar(conn, def) == -1) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 40c2eab..dcc46e1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1534,8 +1534,7 @@ static virDomainPtr qemuDomainLookupByID(virConnectPtr conn, if (virDomainLookupByIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -1563,8 +1562,7 @@ static virDomainPtr qemuDomainLookupByUUID(virConnectPtr conn, if (virDomainLookupByUUIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); @@ -1589,8 +1587,7 @@ static virDomainPtr qemuDomainLookupByName(virConnectPtr conn, if (virDomainLookupByNameEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); @@ -1794,9 +1791,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, } virDomainAuditStart(vm, "booted", true); - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); qemuProcessEndJob(driver, vm); @@ -7101,8 +7096,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, VIR_DOMAIN_EVENT_DEFINED_UPDATED); VIR_INFO("Creating domain '%s'", vm->def->name); - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainDefFree(oldDef); @@ -15735,9 +15729,7 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn, monConfig = NULL; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); qemuDomainObjEndJob(driver, vm); @@ -19553,12 +19545,10 @@ qemuDomainGetStats(virConnectPtr conn, } } - if (!(tmp->dom = virGetDomain(conn, dom->def->name, dom->def->uuid))) + if (!(tmp->dom = virGetDomain(conn, dom->def->name, + dom->def->uuid, dom->def->id))) goto cleanup; - /* We have to copy the domain ID by hand */ - tmp->dom->id = dom->def->id; - *record = tmp; tmp = NULL; ret = 0; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index dabfe10..87d7dcd 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5160,7 +5160,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, if (flags & VIR_MIGRATE_OFFLINE) { if (retcode == 0 && qemuMigrationPersist(driver, vm, mig, false) == 0) - dom = virGetDomain(dconn, vm->def->name, vm->def->uuid); + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, -1); goto endjob; } @@ -5294,7 +5294,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, } } - dom = virGetDomain(dconn, vm->def->name, vm->def->uuid); + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, vm->def->id); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 27bcc9b..1242bd6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8080,10 +8080,7 @@ remoteStorageVolGetInfoFlags(virStorageVolPtr vol, static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain domain) { - virDomainPtr dom; - dom = virGetDomain(conn, domain.name, BAD_CAST domain.uuid); - if (dom) dom->id = domain.id; - return dom; + return virGetDomain(conn, domain.name, BAD_CAST domain.uuid, domain.id); } static virNetworkPtr diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 8669495..cce4d2d 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1673,9 +1673,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); - ret = virGetDomain(conn, dom->def->name, dom->def->uuid); - if (ret) - ret->id = dom->def->id; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: if (dom) @@ -1699,9 +1697,7 @@ static virDomainPtr testDomainLookupByID(virConnectPtr conn, goto cleanup; } - ret = virGetDomain(conn, dom->def->name, dom->def->uuid); - if (ret) - ret->id = dom->def->id; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: if (dom) @@ -1721,9 +1717,7 @@ static virDomainPtr testDomainLookupByUUID(virConnectPtr conn, goto cleanup; } - ret = virGetDomain(conn, dom->def->name, dom->def->uuid); - if (ret) - ret->id = dom->def->id; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: if (dom) @@ -1743,9 +1737,7 @@ static virDomainPtr testDomainLookupByName(virConnectPtr conn, goto cleanup; } - ret = virGetDomain(conn, dom->def->name, dom->def->uuid); - if (ret) - ret->id = dom->def->id; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: virDomainObjEndAPI(&dom); @@ -2699,9 +2691,7 @@ static virDomainPtr testDomainDefineXMLFlags(virConnectPtr conn, VIR_DOMAIN_EVENT_DEFINED_ADDED : VIR_DOMAIN_EVENT_DEFINED_UPDATED); - ret = virGetDomain(conn, dom->def->name, dom->def->uuid); - if (ret) - ret->id = dom->def->id; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: virDomainDefFree(def); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 532ce3b..03edc89 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1359,8 +1359,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr conn, if (virDomainLookupByIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -1387,8 +1386,7 @@ static virDomainPtr umlDomainLookupByUUID(virConnectPtr conn, if (virDomainLookupByUUIDEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -1415,8 +1413,7 @@ static virDomainPtr umlDomainLookupByName(virConnectPtr conn, if (virDomainLookupByNameEnsureACL(conn, vm->def) < 0) goto cleanup; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainObjEndAPI(&vm); @@ -1616,8 +1613,7 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainDefFree(def); @@ -2093,8 +2089,7 @@ umlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainDefFree(def); diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index b71506a..d2b36ac 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -806,9 +806,7 @@ static virDomainPtr vboxDomainLookupByID(virConnectPtr conn, int id) * itself, so need not worry. */ - ret = virGetDomain(conn, machineNameUtf8, uuid); - if (ret) - ret->id = id + 1; + ret = virGetDomain(conn, machineNameUtf8, uuid, id + 1); /* Cleanup all the XPCOM allocated stuff here */ VBOX_UTF8_FREE(machineNameUtf8); @@ -862,8 +860,9 @@ virDomainPtr vboxDomainLookupByUUID(virConnectPtr conn, vboxIIDUnalloc(&iid); if (memcmp(uuid, iid_as_uuid, VIR_UUID_BUFLEN) == 0) { - PRUint32 state; + int id = -1; + matched = true; @@ -872,16 +871,10 @@ virDomainPtr vboxDomainLookupByUUID(virConnectPtr conn, gVBoxAPI.UIMachine.GetState(machine, &state); - /* get a new domain pointer from virGetDomain, if it fails - * then no need to assign the id, else assign the id, cause - * it is -1 by default. rest is taken care by virGetDomain - * itself, so need not worry. - */ + if (gVBoxAPI.machineStateChecker.Online(state)) + id = i + 1; - ret = virGetDomain(conn, machineNameUtf8, iid_as_uuid); - if (ret && - gVBoxAPI.machineStateChecker.Online(state)) - ret->id = i + 1; + ret = virGetDomain(conn, machineNameUtf8, iid_as_uuid, id); } if (matched) @@ -936,8 +929,8 @@ vboxDomainLookupByName(virConnectPtr conn, const char *name) VBOX_UTF16_TO_UTF8(machineNameUtf16, &machineNameUtf8); if (STREQ(name, machineNameUtf8)) { - PRUint32 state; + int id = -1; matched = true; @@ -947,16 +940,10 @@ vboxDomainLookupByName(virConnectPtr conn, const char *name) gVBoxAPI.UIMachine.GetState(machine, &state); - /* get a new domain pointer from virGetDomain, if it fails - * then no need to assign the id, else assign the id, cause - * it is -1 by default. rest is taken care by virGetDomain - * itself, so need not worry. - */ + if (gVBoxAPI.machineStateChecker.Online(state)) + id = i + 1; - ret = virGetDomain(conn, machineNameUtf8, uuid); - if (ret && - gVBoxAPI.machineStateChecker.Online(state)) - ret->id = i + 1; + ret = virGetDomain(conn, machineNameUtf8, uuid, id); } VBOX_UTF8_FREE(machineNameUtf8); @@ -1982,7 +1969,7 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags gVBoxAPI.UISession.Close(data->vboxSession); vboxIIDUnalloc(&mchiid); - ret = virGetDomain(conn, def->name, def->uuid); + ret = virGetDomain(conn, def->name, def->uuid, -1); VBOX_RELEASE(machine); virDomainDefFree(def); @@ -7333,6 +7320,7 @@ vboxConnectListAllDomains(virConnectPtr conn, for (i = 0; i < machines.count; i++) { IMachine *machine = machines.items[i]; + int id = -1; if (!machine) continue; @@ -7397,7 +7385,10 @@ vboxConnectListAllDomains(virConnectPtr conn, vboxIIDToUUID(&iid, uuid); vboxIIDUnalloc(&iid); - dom = virGetDomain(conn, machineNameUtf8, uuid); + if (active) + id = i + 1; + + dom = virGetDomain(conn, machineNameUtf8, uuid, id); VBOX_UTF8_FREE(machineNameUtf8); VBOX_UTF16_FREE(machineNameUtf16); @@ -7405,9 +7396,6 @@ vboxConnectListAllDomains(virConnectPtr conn, if (!dom) goto cleanup; - if (active) - dom->id = i + 1; - doms[count++] = dom; } diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index d3497bd..6853658 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -427,9 +427,7 @@ vmwareDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla vmdef = NULL; vm->persistent = 1; - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = -1; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, -1); cleanup: virDomainDefFree(vmdef); @@ -728,9 +726,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml, goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: virDomainDefFree(vmdef); @@ -855,9 +851,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id) goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -906,9 +900,7 @@ vmwareDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) @@ -932,9 +924,7 @@ vmwareDomainLookupByName(virConnectPtr conn, const char *name) goto cleanup; } - dom = virGetDomain(conn, vm->def->name, vm->def->uuid); - if (dom) - dom->id = vm->def->id; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: if (vm) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 88f1960..bb1afd0 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -586,9 +586,7 @@ vzDomainLookupByID(virConnectPtr conn, int id) if (virDomainLookupByIDEnsureACL(conn, dom->def) < 0) goto cleanup; - ret = virGetDomain(conn, dom->def->name, dom->def->uuid); - if (ret) - ret->id = dom->def->id; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: virObjectUnlock(dom); @@ -615,9 +613,7 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (virDomainLookupByUUIDEnsureACL(conn, dom->def) < 0) goto cleanup; - ret = virGetDomain(conn, dom->def->name, dom->def->uuid); - if (ret) - ret->id = dom->def->id; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: virObjectUnlock(dom); @@ -642,9 +638,7 @@ vzDomainLookupByName(virConnectPtr conn, const char *name) if (virDomainLookupByNameEnsureACL(conn, dom->def) < 0) goto cleanup; - ret = virGetDomain(conn, dom->def->name, dom->def->uuid); - if (ret) - ret->id = dom->def->id; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: virDomainObjEndAPI(&dom); @@ -898,9 +892,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) } } - retdom = virGetDomain(conn, def->name, def->uuid); - if (retdom) - retdom->id = def->id; + retdom = virGetDomain(conn, def->name, def->uuid, def->id); cleanup: if (job) @@ -3351,9 +3343,7 @@ vzDomainMigrateFinish3Params(virConnectPtr dconn, if (virDomainMigrateFinish3ParamsEnsureACL(dconn, dom->def) < 0) goto cleanup; - domain = virGetDomain(dconn, dom->def->name, dom->def->uuid); - if (domain) - domain->id = dom->def->id; + domain = virGetDomain(dconn, dom->def->name, dom->def->uuid, dom->def->id); cleanup: /* In this situation we have to restore domain on source. But the migration @@ -3767,7 +3757,7 @@ vzDomainGetAllStats(virConnectPtr conn, if (vzDomainGetBalloonStats(dom, stat, &maxparams) < 0) goto error; - if (!(stat->dom = virGetDomain(conn, dom->def->name, dom->def->uuid))) + if (!(stat->dom = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id))) goto error; return stat; diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 7a2f4a1..4d14089 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -774,9 +774,7 @@ xenUnifiedDomainCreateXML(virConnectPtr conn, if (xenDaemonCreateXML(conn, def) < 0) goto cleanup; - ret = virGetDomain(conn, def->name, def->uuid); - if (ret) - ret->id = def->id; + ret = virGetDomain(conn, def->name, def->uuid, def->id); cleanup: virDomainDefFree(def); @@ -795,10 +793,7 @@ xenUnifiedDomainLookupByID(virConnectPtr conn, int id) if (virDomainLookupByIDEnsureACL(conn, def) < 0) goto cleanup; - if (!(ret = virGetDomain(conn, def->name, def->uuid))) - goto cleanup; - - ret->id = def->id; + ret = virGetDomain(conn, def->name, def->uuid, def->id); cleanup: virDomainDefFree(def); @@ -818,10 +813,7 @@ xenUnifiedDomainLookupByUUID(virConnectPtr conn, if (virDomainLookupByUUIDEnsureACL(conn, def) < 0) goto cleanup; - if (!(ret = virGetDomain(conn, def->name, def->uuid))) - goto cleanup; - - ret->id = def->id; + ret = virGetDomain(conn, def->name, def->uuid, def->id); cleanup: virDomainDefFree(def); @@ -841,10 +833,7 @@ xenUnifiedDomainLookupByName(virConnectPtr conn, if (virDomainLookupByNameEnsureACL(conn, def) < 0) goto cleanup; - if (!(ret = virGetDomain(conn, def->name, def->uuid))) - goto cleanup; - - ret->id = def->id; + ret = virGetDomain(conn, def->name, def->uuid, def->id); cleanup: virDomainDefFree(def); @@ -1720,9 +1709,7 @@ xenUnifiedDomainMigrateFinish(virConnectPtr dconn, goto cleanup; } - ret = virGetDomain(dconn, minidef->name, minidef->uuid); - if (ret) - ret->id = minidef->id; + ret = virGetDomain(dconn, minidef->name, minidef->uuid, minidef->id); cleanup: virDomainDefFree(def); @@ -1817,10 +1804,7 @@ xenUnifiedDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int if (xenDaemonDomainDefineXML(conn, def) < 0) goto cleanup; - ret = virGetDomain(conn, def->name, def->uuid); - - if (ret) - ret->id = -1; + ret = virGetDomain(conn, def->name, def->uuid, -1); cleanup: virDomainDefFree(def); diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index a9ed407..37d1a6f 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -580,14 +580,14 @@ xenapiDomainCreateXML(virConnectPtr conn, ignore_value(virUUIDParse(record->uuid, raw_uuid)); if (vm) { if (xen_vm_start(priv->session, vm, false, false)) { - domP = virGetDomain(conn, record->name_label, raw_uuid); + domP = virGetDomain(conn, record->name_label, + raw_uuid, record->domid); if (!domP) { xen_vm_record_free(record); xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, _("Domain Pointer is invalid")); return domP; } - domP->id = record->domid; xen_vm_free(vm); } else @@ -627,18 +627,16 @@ xenapiDomainLookupByID(virConnectPtr conn, int id) for (i = 0; i < result->size; i++) { xen_vm_get_domid(session, &domID, result->contents[i]); if (domID == id) { + int64_t domid = -1; + xen_vm_get_record(session, &record, result->contents[i]); xen_vm_get_uuid(session, &uuid, result->contents[i]); ignore_value(virUUIDParse(uuid, raw_uuid)); - domP = virGetDomain(conn, record->name_label, raw_uuid); - if (domP) { - int64_t domid = -1; - xen_vm_get_domid(session, &domid, result->contents[i]); - domP->id = domid; - } else { + xen_vm_get_domid(session, &domid, result->contents[i]); + domP = virGetDomain(conn, record->name_label, raw_uuid, domid); + if (!domP) { xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, _("Domain Pointer not valid")); - domP = NULL; } xen_uuid_free(uuid); xen_vm_record_free(record); @@ -676,13 +674,10 @@ xenapiDomainLookupByUUID(virConnectPtr conn, if (xen_vm_get_by_uuid(session, &vm, uuidStr)) { xen_vm_get_record(session, &record, vm); if (record != NULL) { - domP = virGetDomain(conn, record->name_label, uuid); + domP = virGetDomain(conn, record->name_label, uuid, record->domid); if (!domP) { xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, _("Domain Pointer not valid")); - domP = NULL; - } else { - domP->id = record->domid; } xen_vm_record_free(record); } else { @@ -722,12 +717,11 @@ xenapiDomainLookupByName(virConnectPtr conn, vm = vms->contents[0]; xen_vm_get_uuid(session, &uuid, vm); if (uuid != NULL) { + int64_t domid = -1; ignore_value(virUUIDParse(uuid, raw_uuid)); - domP = virGetDomain(conn, name, raw_uuid); + xen_vm_get_domid(session, &domid, vm); + domP = virGetDomain(conn, name, raw_uuid, domid); if (domP != NULL) { - int64_t domid = -1; - xen_vm_get_domid(session, &domid, vm); - domP->id = domid; xen_uuid_free(uuid); xen_vm_set_free(vms); return domP; @@ -1776,7 +1770,7 @@ xenapiDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla if (record != NULL) { unsigned char raw_uuid[VIR_UUID_BUFLEN]; ignore_value(virUUIDParse(record->uuid, raw_uuid)); - domP = virGetDomain(conn, record->name_label, raw_uuid); + domP = virGetDomain(conn, record->name_label, raw_uuid, -1); if (!domP && !priv->session->ok) xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN, NULL); xen_vm_record_free(record); -- 2.10.2

On Tue, Mar 28, 2017 at 05:15:08PM +0200, Michal Privoznik wrote:
So far our code is full of the following pattern:
dom = virGetDomain(conn, name, uuid) if (dom) dom->id = 42;
There is no reasong why it couldn't be just:
dom = virGetDomain(conn, name, uuid, id);
After all, client domain representation consists of tuple (name, uuid, id).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACK - looks like a sensible change to improve the static safety of the code. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
participants (2)
-
Michal Privoznik
-
Richard W.M. Jones