[libvirt] [PATCH 0/6] Refactor critical section in virDomainListAllDomains and fix qemuConnectGetAllDomainStats
7:44 a.m.
Peter Krempa (6):
util: Make the virDomainListFree helper more universal
conf: Extract code to filter domain list into a separate function
conf: Rename virDomainObjListFilter type to virDomainObjListACLFilter
conf: Refactor domain list collection critical section
conf: Add helper to convert list of virDomains to a list of
virDomainObjs
qemu: Convert qemuConnectGetAllDomainStats to use new helpers
daemon/remote.c | 2 +-
src/conf/domain_conf.c | 258 ++++++++++++++++++++++++++++---------------
src/conf/domain_conf.h | 29 +++--
src/libvirt_private.syms | 5 +-
src/qemu/qemu_driver.c | 86 +++++++--------
src/util/virobject.c | 41 +++++++
src/util/virobject.h | 2 +
tools/virsh-domain-monitor.c | 2 +-
8 files changed, 275 insertions(+), 150 deletions(-)
--
2.3.5
7:44 a.m.
New subject: [libvirt] [PATCH 1/6] util: Make the virDomainListFree helper more universal
Extend it to a universal helper used for clearing lists of any objects.
Note that the argument type is specifically void * to allow implicit
typecasting.
Additionally add a helper that works on non-NULL terminated arrays once
we know the length.
---
daemon/remote.c | 2 +-
src/conf/domain_conf.c | 24 +-----------------------
src/conf/domain_conf.h | 2 --
src/libvirt_private.syms | 3 ++-
src/qemu/qemu_driver.c | 2 +-
src/util/virobject.c | 41 +++++++++++++++++++++++++++++++++++++++++
src/util/virobject.h | 2 ++
tools/virsh-domain-monitor.c | 2 +-
8 files changed, 49 insertions(+), 29 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 3a3f168..e259a76 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -6382,7 +6382,7 @@ remoteDispatchConnectGetAllDomainStats(virNetServerPtr server
ATTRIBUTE_UNUSED,
virNetMessageSaveError(rerr);
virDomainStatsRecordListFree(retStats);
- virDomainListFree(doms);
+ virObjectListFree(doms);
return rv;
}
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5f74ab1..b9c4c61 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23033,28 +23033,6 @@ virDomainListPopulate(void *payload,
#undef MATCH
-/**
- * virDomainListFree:
- * @list: list of domains to free
- *
- * Frees a NULL-terminated list of domains without messing with currently
- * set libvirt errors.
- */
-void
-virDomainListFree(virDomainPtr *list)
-{
- virDomainPtr *next;
-
- if (!list)
- return;
-
- for (next = list; *next; next++)
- virObjectUnref(*next);
-
- VIR_FREE(list);
-}
-
-
int
virDomainObjListExport(virDomainObjListPtr doms,
virConnectPtr conn,
@@ -23090,7 +23068,7 @@ virDomainObjListExport(virDomainObjListPtr doms,
ret = data.ndomains;
cleanup:
- virDomainListFree(data.domains);
+ virObjectListFree(data.domains);
virObjectUnlock(doms);
return ret;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 96988ef..11e4bb9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3054,8 +3054,6 @@ int virDomainObjListExport(virDomainObjListPtr doms,
virDomainObjListFilter filter,
unsigned int flags);
-void virDomainListFree(virDomainPtr *list);
-
int
virDomainDefMaybeAddController(virDomainDefPtr def,
int type,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c8e6fb4..d7cac20 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -341,7 +341,6 @@ virDomainLifecycleCrashTypeFromString;
virDomainLifecycleCrashTypeToString;
virDomainLifecycleTypeFromString;
virDomainLifecycleTypeToString;
-virDomainListFree;
virDomainLiveConfigHelperMethod;
virDomainLoaderDefFree;
virDomainLoaderTypeFromString;
@@ -1881,6 +1880,8 @@ virClassNew;
virObjectFreeCallback;
virObjectFreeHashData;
virObjectIsClass;
+virObjectListFree;
+virObjectListFreeCount;
virObjectLock;
virObjectLockableNew;
virObjectNew;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3695b26..3032b7a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19909,7 +19909,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
virDomainObjEndAPI(&dom);
virDomainStatsRecordListFree(tmpstats);
- virDomainListFree(domlist);
+ virObjectListFree(domlist);
return ret;
}
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 9ccd310..51876b9 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -405,3 +405,44 @@ void virObjectFreeHashData(void *opaque, const void *name
ATTRIBUTE_UNUSED)
{
virObjectUnref(opaque);
}
+
+
+/**
+ * virObjectListFree:
+ * @list: A pointer to a NULL-terminated list of object pointers to free
+ *
+ * Unrefs all members of @list and frees the list itself.
+ */
+void virObjectListFree(void *list)
+{
+ void **next;
+
+ if (!list)
+ return;
+
+ for (next = (void **) list; *next; next++)
+ virObjectUnref(*next);
+
+ VIR_FREE(list);
+}
+
+
+/**
+ * virObjectListFreeCount:
+ * @list: A pointer to a list of object pointers to freea
+ * @count: Number of elements in the list.
+ *
+ * Unrefs all members of @list and frees the list itself.
+ */
+void virObjectListFreeCount(void *list, size_t count)
+{
+ size_t i;
+
+ if (!list)
+ return;
+
+ for (i = 0; i < count; i++)
+ virObjectUnref(((void **)list)[i]);
+
+ VIR_FREE(list);
+}
diff --git a/src/util/virobject.h b/src/util/virobject.h
index ad1f0c1..c3ecc1e 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -99,5 +99,7 @@ void virObjectLock(void *lockableobj)
void virObjectUnlock(void *lockableobj)
ATTRIBUTE_NONNULL(1);
+void virObjectListFree(void *list);
+void virObjectListFreeCount(void *list, size_t count);
#endif /* __VIR_OBJECT_H */
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 9686531..91c57e2 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -2193,7 +2193,7 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
virDomainStatsRecordListFree(records);
- virDomainListFree(domlist);
+ virObjectListFree(domlist);
return ret;
}
--
2.3.5
7:44 a.m.
New subject: [libvirt] [PATCH 2/6] conf: Extract code to filter domain list into a separate function
Separate the code to simplify future refactors.
---
src/conf/domain_conf.c | 82 +++++++++++++++++++++++++++++---------------------
1 file changed, 47 insertions(+), 35 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b9c4c61..057602b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22927,43 +22927,19 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
return ret;
}
-struct virDomainListData {
- virConnectPtr conn;
- virDomainPtr *domains;
- virDomainObjListFilter filter;
- unsigned int flags;
- int ndomains;
- bool error;
-};
-#define MATCH(FLAG) (data->flags & (FLAG))
-static void
-virDomainListPopulate(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
+#define MATCH(FLAG) (filter & (FLAG))
+static bool
+virDomainObjMatchFilter(virDomainObjPtr vm,
+ unsigned int filter)
{
- struct virDomainListData *data = opaque;
- virDomainObjPtr vm = payload;
- virDomainPtr dom;
-
- if (data->error)
- return;
-
- virObjectLock(vm);
- /* check if the domain matches the filter */
-
- /* filter by the callback function (access control checks) */
- if (data->filter != NULL &&
- !data->filter(data->conn, vm->def))
- goto cleanup;
-
/* filter by active state */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) &&
!((MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE) &&
virDomainObjIsActive(vm)) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE) &&
!virDomainObjIsActive(vm))))
- goto cleanup;
+ return false;
/* filter by persistence */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT) &&
@@ -22971,7 +22947,7 @@ virDomainListPopulate(void *payload,
vm->persistent) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) &&
!vm->persistent)))
- goto cleanup;
+ return false;
/* filter by domain state */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) {
@@ -22986,7 +22962,7 @@ virDomainListPopulate(void *payload,
(st != VIR_DOMAIN_RUNNING &&
st != VIR_DOMAIN_PAUSED &&
st != VIR_DOMAIN_SHUTOFF))))
- goto cleanup;
+ return false;
}
/* filter by existence of managed save state */
@@ -22995,22 +22971,59 @@ virDomainListPopulate(void *payload,
vm->hasManagedSave) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE) &&
!vm->hasManagedSave)))
- goto cleanup;
+ return false;
/* filter by autostart option */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_AUTOSTART) &&
!((MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART) && vm->autostart) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART) && !vm->autostart)))
- goto cleanup;
+ return false;
/* filter by snapshot existence */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT)) {
int nsnap = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0);
if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT) && nsnap > 0) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT) && nsnap <= 0)))
- goto cleanup;
+ return false;
}
+ return true;
+}
+#undef MATCH
+
+
+struct virDomainListData {
+ virConnectPtr conn;
+ virDomainPtr *domains;
+ virDomainObjListFilter filter;
+ unsigned int flags;
+ int ndomains;
+ bool error;
+};
+
+static void
+virDomainListPopulate(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+ struct virDomainListData *data = opaque;
+ virDomainObjPtr vm = payload;
+ virDomainPtr dom;
+
+ if (data->error)
+ return;
+
+ virObjectLock(vm);
+ /* check if the domain matches the filter */
+
+ /* filter by the callback function (access control checks) */
+ if (data->filter != NULL &&
+ !data->filter(data->conn, vm->def))
+ goto cleanup;
+
+ if (!virDomainObjMatchFilter(vm, data->flags))
+ goto cleanup;
+
/* just count the machines */
if (!data->domains) {
data->ndomains++;
@@ -23030,7 +23043,6 @@ virDomainListPopulate(void *payload,
virObjectUnlock(vm);
return;
}
-#undef MATCH
int
--
2.3.5
7:44 a.m.
New subject: [libvirt] [PATCH 3/6] conf: Rename virDomainObjListFilter type to virDomainObjListACLFilter
The passed function is meant to filter domains according to ACL match.
---
src/conf/domain_conf.c | 16 ++++++++--------
src/conf/domain_conf.h | 12 ++++++------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 057602b..c2b7d6a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22043,7 +22043,7 @@ virDomainGetFilesystemForTarget(virDomainDefPtr def,
struct virDomainObjListData {
- virDomainObjListFilter filter;
+ virDomainObjListACLFilter filter;
virConnectPtr conn;
bool active;
int count;
@@ -22074,7 +22074,7 @@ virDomainObjListCount(void *payload,
int
virDomainObjListNumOfDomains(virDomainObjListPtr doms,
bool active,
- virDomainObjListFilter filter,
+ virDomainObjListACLFilter filter,
virConnectPtr conn)
{
struct virDomainObjListData data = { filter, conn, active, 0 };
@@ -22085,7 +22085,7 @@ virDomainObjListNumOfDomains(virDomainObjListPtr doms,
}
struct virDomainIDData {
- virDomainObjListFilter filter;
+ virDomainObjListACLFilter filter;
virConnectPtr conn;
int numids;
int maxids;
@@ -22113,7 +22113,7 @@ int
virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
int *ids,
int maxids,
- virDomainObjListFilter filter,
+ virDomainObjListACLFilter filter,
virConnectPtr conn)
{
struct virDomainIDData data = { filter, conn,
@@ -22125,7 +22125,7 @@ virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
}
struct virDomainNameData {
- virDomainObjListFilter filter;
+ virDomainObjListACLFilter filter;
virConnectPtr conn;
int oom;
int numnames;
@@ -22163,7 +22163,7 @@ int
virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
char **const names,
int maxnames,
- virDomainObjListFilter filter,
+ virDomainObjListACLFilter filter,
virConnectPtr conn)
{
struct virDomainNameData data = { filter, conn,
@@ -22995,7 +22995,7 @@ virDomainObjMatchFilter(virDomainObjPtr vm,
struct virDomainListData {
virConnectPtr conn;
virDomainPtr *domains;
- virDomainObjListFilter filter;
+ virDomainObjListACLFilter filter;
unsigned int flags;
int ndomains;
bool error;
@@ -23049,7 +23049,7 @@ int
virDomainObjListExport(virDomainObjListPtr doms,
virConnectPtr conn,
virDomainPtr **domains,
- virDomainObjListFilter filter,
+ virDomainObjListACLFilter filter,
unsigned int flags)
{
int ret = -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 11e4bb9..bbfe96e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2318,8 +2318,8 @@ struct _virDomainObj {
typedef struct _virDomainObjList virDomainObjList;
typedef virDomainObjList *virDomainObjListPtr;
-typedef bool (*virDomainObjListFilter)(virConnectPtr conn,
- virDomainDefPtr def);
+typedef bool (*virDomainObjListACLFilter)(virConnectPtr conn,
+ virDomainDefPtr def);
/* This structure holds various callbacks and data needed
@@ -2840,18 +2840,18 @@ unsigned int virDomainVideoDefaultRAM(const virDomainDef *def,
int virDomainObjListNumOfDomains(virDomainObjListPtr doms,
bool active,
- virDomainObjListFilter filter,
+ virDomainObjListACLFilter filter,
virConnectPtr conn);
int virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
int *ids,
int maxids,
- virDomainObjListFilter filter,
+ virDomainObjListACLFilter filter,
virConnectPtr conn);
int virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
char **const names,
int maxnames,
- virDomainObjListFilter filter,
+ virDomainObjListACLFilter filter,
virConnectPtr conn);
typedef int (*virDomainObjListIterator)(virDomainObjPtr dom,
@@ -3051,7 +3051,7 @@ VIR_ENUM_DECL(virDomainStartupPolicy)
int virDomainObjListExport(virDomainObjListPtr doms,
virConnectPtr conn,
virDomainPtr **domains,
- virDomainObjListFilter filter,
+ virDomainObjListACLFilter filter,
unsigned int flags);
int
--
2.3.5
7:44 a.m.
New subject: [libvirt] [PATCH 4/6] conf: Refactor domain list collection critical section
Until now the virDomainListAllDomains API would lock the domain list and
then every single domain object to access and filter it. This would
potentially allow a unresponsive VM to block the whole daemon if a
*listAllDomains call would get stuck.
To avoid this problem this patch collects a list of referenced domain
objects first from the list and then unlocks it right away. The
expensive operation requiring locking of the domain object is executed
after the list lock is dropped. While a single blocked domain will still
lock up a listAllDomains call, the domain list won't be held locked and
thus other APIs won't be blocked.
Additionally this patch also fixes the lookup code, where we'd ignore
the vm->removing flag and thus potentially return domain objects that
would be deleted very soon so calling any API wouldn't make sense.
As other clients also could benefit from operating on a list of domain
objects rather than the public domain descriptors a new intermediate
API - virDomainObjListCollect - is introduced by this patch.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1181074
---
src/conf/domain_conf.c | 142 +++++++++++++++++++++++++++++------------------
src/conf/domain_conf.h | 6 ++
src/libvirt_private.syms | 1 +
3 files changed, 95 insertions(+), 54 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c2b7d6a..66fe470 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22993,98 +22993,132 @@ virDomainObjMatchFilter(virDomainObjPtr vm,
struct virDomainListData {
- virConnectPtr conn;
- virDomainPtr *domains;
- virDomainObjListACLFilter filter;
- unsigned int flags;
- int ndomains;
- bool error;
+ virDomainObjPtr *vms;
+ size_t nvms;
};
+
static void
-virDomainListPopulate(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
+virDomainObjListCollectIterator(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
{
struct virDomainListData *data = opaque;
- virDomainObjPtr vm = payload;
- virDomainPtr dom;
- if (data->error)
- return;
+ data->vms[data->nvms++] = virObjectRef(payload);
+}
- virObjectLock(vm);
- /* check if the domain matches the filter */
- /* filter by the callback function (access control checks) */
- if (data->filter != NULL &&
- !data->filter(data->conn, vm->def))
- goto cleanup;
+static void
+virDomainObjListFilter(virDomainObjPtr **list,
+ size_t *nvms,
+ virConnectPtr conn,
+ virDomainObjListACLFilter filter,
+ unsigned int flags)
+{
+ size_t i = 0;
- if (!virDomainObjMatchFilter(vm, data->flags))
- goto cleanup;
+ while (i < *nvms) {
+ virDomainObjPtr vm = (*list)[i];
- /* just count the machines */
- if (!data->domains) {
- data->ndomains++;
- goto cleanup;
+ virObjectLock(vm);
+
+ /* do not list the object if:
+ * 1) it's being removed.
+ * 2) connection does not have ACL to see it
+ * 3) it doesn't match the filter
+ */
+ if (vm->removing ||
+ (filter && !filter(conn, vm->def)) ||
+ !virDomainObjMatchFilter(vm, flags)) {
+ virObjectUnlock(vm);
+ virObjectUnref(vm);
+ VIR_DELETE_ELEMENT(*list, i, *nvms);
+ continue;
+ }
+
+ virObjectUnlock(vm);
+ i++;
}
+}
- if (!(dom = virGetDomain(data->conn, vm->def->name, vm->def->uuid)))
{
- data->error = true;
- goto cleanup;
+
+int
+virDomainObjListCollect(virDomainObjListPtr domlist,
+ virConnectPtr conn,
+ virDomainObjPtr **vms,
+ size_t *nvms,
+ virDomainObjListACLFilter filter,
+ unsigned int flags)
+{
+ struct virDomainListData data = { NULL, 0 };
+
+ virObjectLock(domlist);
+ if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) {
+ virObjectUnlock(domlist);
+ return -1;
}
- dom->id = vm->def->id;
+ virHashForEach(domlist->objs, virDomainObjListCollectIterator, &data);
+ virObjectUnlock(domlist);
- data->domains[data->ndomains++] = dom;
+ virDomainObjListFilter(&data.vms, &data.nvms, conn, filter, flags);
- cleanup:
- virObjectUnlock(vm);
- return;
+ *nvms = data.nvms;
+ *vms = data.vms;
+
+ return 0;
}
int
-virDomainObjListExport(virDomainObjListPtr doms,
+virDomainObjListExport(virDomainObjListPtr domlist,
virConnectPtr conn,
virDomainPtr **domains,
virDomainObjListACLFilter filter,
unsigned int flags)
{
+ virDomainObjPtr *vms = NULL;
+ virDomainPtr *doms = NULL;
+ size_t nvms = 0;
+ size_t i;
int ret = -1;
- struct virDomainListData data = {
- conn, NULL,
- filter,
- flags, 0, false
- };
+ if (virDomainObjListCollect(domlist, conn, &vms, &nvms, filter, flags) <
0)
+ return -1;
- virObjectLock(doms);
- if (domains &&
- VIR_ALLOC_N(data.domains, virHashSize(doms->objs) + 1) < 0)
- goto cleanup;
+ if (domains) {
+ if (VIR_ALLOC_N(doms, nvms + 1) < 0)
+ goto cleanup;
- virHashForEach(doms->objs, virDomainListPopulate, &data);
+ for (i = 0; i < nvms; i++) {
+ virDomainObjPtr vm = vms[i];
- if (data.error)
- goto cleanup;
+ virObjectLock(vm);
+
+ if (!(doms[i] = virGetDomain(conn, vm->def->name,
vm->def->uuid))) {
+ virObjectUnlock(vm);
+ goto cleanup;
+ }
+
+ doms[i]->id = vm->def->id;
+
+ virObjectUnlock(vm);
+ }
- if (data.domains) {
- /* trim the array to the final size */
- ignore_value(VIR_REALLOC_N(data.domains, data.ndomains + 1));
- *domains = data.domains;
- data.domains = NULL;
+ *domains = doms;
+ doms = NULL;
}
- ret = data.ndomains;
+ ret = nvms;
cleanup:
- virObjectListFree(data.domains);
- virObjectUnlock(doms);
+ virObjectListFree(doms);
+ virObjectListFreeCount(vms, nvms);
return ret;
}
+
virSecurityLabelDefPtr
virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bbfe96e..ac11a20 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3048,6 +3048,12 @@ VIR_ENUM_DECL(virDomainStartupPolicy)
VIR_CONNECT_LIST_DOMAINS_FILTERS_AUTOSTART | \
VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT)
+int virDomainObjListCollect(virDomainObjListPtr doms,
+ virConnectPtr conn,
+ virDomainObjPtr **vms,
+ size_t *nvms,
+ virDomainObjListACLFilter filter,
+ unsigned int flags);
int virDomainObjListExport(virDomainObjListPtr doms,
virConnectPtr conn,
virDomainPtr **domains,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d7cac20..9a7a944 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -385,6 +385,7 @@ virDomainObjGetMetadata;
virDomainObjGetPersistentDef;
virDomainObjGetState;
virDomainObjListAdd;
+virDomainObjListCollect;
virDomainObjListExport;
virDomainObjListFindByID;
virDomainObjListFindByName;
--
2.3.5
5:29 a.m.
New subject: [libvirt] [PATCH 4/6] conf: Refactor domain list collection critical section
On 04/30/2015 08:44 AM, Peter Krempa wrote:
...
- if (!(dom = virGetDomain(data->conn, vm->def->name, vm->def->uuid)))
{
- data->error = true;
- goto cleanup;
+
+int
+virDomainObjListCollect(virDomainObjListPtr domlist,
+ virConnectPtr conn,
+ virDomainObjPtr **vms,
+ size_t *nvms,
+ virDomainObjListACLFilter filter,
+ unsigned int flags)
+{
+ struct virDomainListData data = { NULL, 0 };
+
+ virObjectLock(domlist);
+ if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) {
Coverity checker wasn't very happy with this especially if virHashSize
returns a negative number.
John
+ virObjectUnlock(domlist);
+ return -1;
}
- dom->id = vm->def->id;
+ virHashForEach(domlist->objs, virDomainObjListCollectIterator, &data);
+ virObjectUnlock(domlist);
- data->domains[data->ndomains++] = dom;
+ virDomainObjListFilter(&data.vms, &data.nvms, conn, filter, flags);
- cleanup:
- virObjectUnlock(vm);
- return;
+ *nvms = data.nvms;
+ *vms = data.vms;
+
+ return 0;
}
5:37 a.m.
New subject: [libvirt] [PATCH 4/6] conf: Refactor domain list collection critical section
On Mon, May 11, 2015 at 06:29:35 -0400, John Ferlan wrote:
On 04/30/2015 08:44 AM, Peter Krempa wrote:
...
>
> - if (!(dom = virGetDomain(data->conn, vm->def->name,
vm->def->uuid))) {
> - data->error = true;
> - goto cleanup;
> +
> +int
> +virDomainObjListCollect(virDomainObjListPtr domlist,
> + virConnectPtr conn,
> + virDomainObjPtr **vms,
> + size_t *nvms,
> + virDomainObjListACLFilter filter,
> + unsigned int flags)
> +{
> + struct virDomainListData data = { NULL, 0 };
> +
> + virObjectLock(domlist);
> + if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) {
Coverity checker wasn't very happy with this especially if virHashSize
returns a negative number.
False positive. virHashSize returns null only if the hash is NULL which
never happens for domlist-objs.
Peter
7:44 a.m.
New subject: [libvirt] [PATCH 5/6] conf: Add helper to convert list of virDomains to a list of virDomainObjs
Add virDomainObjListConvert that will take a list of virDomains, apply
filters and return a list of virDomainObjs.
---
src/conf/domain_conf.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 9 ++++++++
src/libvirt_private.syms | 1 +
3 files changed, 64 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 66fe470..73dc33f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23072,6 +23072,60 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
int
+virDomainObjListConvert(virDomainObjListPtr domlist,
+ virConnectPtr conn,
+ virDomainPtr *doms,
+ size_t ndoms,
+ virDomainObjPtr **vms,
+ size_t *nvms,
+ virDomainObjListACLFilter filter,
+ unsigned int flags,
+ bool skip_missing)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virDomainObjPtr vm;
+ size_t i;
+
+ *nvms = 0;
+ *vms = NULL;
+
+ virObjectLock(domlist);
+ for (i = 0; i < ndoms; i++) {
+ virDomainPtr dom = doms[i];
+
+ virUUIDFormat(dom->uuid, uuidstr);
+
+ if (!(vm = virHashLookup(domlist->objs, uuidstr))) {
+ if (!skip_missing) {
+ virReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'
(%s)"),
+ uuidstr, dom->name);
+ virObjectUnlock(domlist);
+ goto error;
+ }
+ } else {
+ virObjectRef(vm);
+
+ if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0)
+ goto error;
+ }
+ }
+ virObjectUnlock(domlist);
+
+ virDomainObjListFilter(vms, nvms, conn, filter, flags);
+
+ return 0;
+
+ error:
+ virObjectListFreeCount(*vms, *nvms);
+ *vms = NULL;
+ *nvms = 0;
+
+ return -1;
+}
+
+
+int
virDomainObjListExport(virDomainObjListPtr domlist,
virConnectPtr conn,
virDomainPtr **domains,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ac11a20..06ca82f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3059,6 +3059,15 @@ int virDomainObjListExport(virDomainObjListPtr doms,
virDomainPtr **domains,
virDomainObjListACLFilter filter,
unsigned int flags);
+int virDomainObjListConvert(virDomainObjListPtr domlist,
+ virConnectPtr conn,
+ virDomainPtr *doms,
+ size_t ndoms,
+ virDomainObjPtr **vms,
+ size_t *nvms,
+ virDomainObjListACLFilter filter,
+ unsigned int flags,
+ bool skip_missing);
int
virDomainDefMaybeAddController(virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9a7a944..67a7e21 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -386,6 +386,7 @@ virDomainObjGetPersistentDef;
virDomainObjGetState;
virDomainObjListAdd;
virDomainObjListCollect;
+virDomainObjListConvert;
virDomainObjListExport;
virDomainObjListFindByID;
virDomainObjListFindByName;
--
2.3.5
6:22 a.m.
New subject: [libvirt] [PATCH 5/6] conf: Add helper to convert list of virDomains to a list of virDomainObjs
On 30.04.2015 14:44, Peter Krempa wrote:
Add virDomainObjListConvert that will take a list of virDomains,
apply
filters and return a list of virDomainObjs.
---
src/conf/domain_conf.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 9 ++++++++
src/libvirt_private.syms | 1 +
3 files changed, 64 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 66fe470..73dc33f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23072,6 +23072,60 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
int
+virDomainObjListConvert(virDomainObjListPtr domlist,
+ virConnectPtr conn,
+ virDomainPtr *doms,
+ size_t ndoms,
+ virDomainObjPtr **vms,
+ size_t *nvms,
+ virDomainObjListACLFilter filter,
+ unsigned int flags,
+ bool skip_missing)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virDomainObjPtr vm;
+ size_t i;
+
+ *nvms = 0;
+ *vms = NULL;
+
+ virObjectLock(domlist);
+ for (i = 0; i < ndoms; i++) {
+ virDomainPtr dom = doms[i];
+
+ virUUIDFormat(dom->uuid, uuidstr);
+
+ if (!(vm = virHashLookup(domlist->objs, uuidstr))) {
+ if (!skip_missing) {
+ virReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'
(%s)"),
+ uuidstr, dom->name);
+ virObjectUnlock(domlist);
Move Unlock() before the virReportError(). Not that it would matter,
but: a) it keeps list locked for shorter time b) I always felt like
ReportError() and goto error should be joined together.
+ goto error;
+ }
+ } else {
+ virObjectRef(vm);
+
+ if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0)
Please unlock the @domlist before jumping anywhere.
+ goto error;
+ }
+ }
+ virObjectUnlock(domlist);
+
+ virDomainObjListFilter(vms, nvms, conn, filter, flags);
+
+ return 0;
+
+ error:
+ virObjectListFreeCount(*vms, *nvms);
+ *vms = NULL;
+ *nvms = 0;
+
+ return -1;
+}
+
+
Michal
1:47 a.m.
New subject: [libvirt] [PATCH 5/6] conf: Add helper to convert list of virDomains to a list of virDomainObjs
On Mon, May 04, 2015 at 13:22:17 +0200, Michal Privoznik wrote:
On 30.04.2015 14:44, Peter Krempa wrote:
> Add virDomainObjListConvert that will take a list of virDomains, apply
> filters and return a list of virDomainObjs.
> ---
> src/conf/domain_conf.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_conf.h | 9 ++++++++
> src/libvirt_private.syms | 1 +
> 3 files changed, 64 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 66fe470..73dc33f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -23072,6 +23072,60 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
>
>
> int
> +virDomainObjListConvert(virDomainObjListPtr domlist,
> + virConnectPtr conn,
> + virDomainPtr *doms,
> + size_t ndoms,
> + virDomainObjPtr **vms,
> + size_t *nvms,
> + virDomainObjListACLFilter filter,
> + unsigned int flags,
> + bool skip_missing)
> +{
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virDomainObjPtr vm;
> + size_t i;
> +
> + *nvms = 0;
> + *vms = NULL;
> +
> + virObjectLock(domlist);
> + for (i = 0; i < ndoms; i++) {
> + virDomainPtr dom = doms[i];
> +
> + virUUIDFormat(dom->uuid, uuidstr);
> +
> + if (!(vm = virHashLookup(domlist->objs, uuidstr))) {
> + if (!skip_missing) {
> + virReportError(VIR_ERR_NO_DOMAIN,
> + _("no domain with matching uuid '%s'
(%s)"),
> + uuidstr, dom->name);
> + virObjectUnlock(domlist);
Move Unlock() before the virReportError(). Not that it would matter,
but: a) it keeps list locked for shorter time b) I always felt like
ReportError() and goto error should be joined together.
Okay.
> + goto error;
> + }
> + } else {
> + virObjectRef(vm);
> +
> + if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0)
Please unlock the @domlist before jumping anywhere.
Uhh, right. Also @vm needs to be unref'd before jumping to error.
> + goto error;
> + }
> + }
> + virObjectUnlock(domlist);
> +
> + virDomainObjListFilter(vms, nvms, conn, filter, flags);
> +
> + return 0;
> +
> + error:
> + virObjectListFreeCount(*vms, *nvms);
> + *vms = NULL;
> + *nvms = 0;
> +
> + return -1;
> +}
> +
> +
I've also found the function hard to read after a week of hollidays so
I've changed the control flow a bit so that the else section is not
necessary and I've inverted the (!skip_missing) check to 'continue' the
for loop.
Thanks for the review I'll push this shortly.
Peter
5:33 a.m.
New subject: [libvirt] [PATCH 5/6] conf: Add helper to convert list of virDomains to a list of virDomainObjs
On 04/30/2015 08:44 AM, Peter Krempa wrote:
Add virDomainObjListConvert that will take a list of virDomains,
apply
filters and return a list of virDomainObjs.
---
src/conf/domain_conf.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 9 ++++++++
src/libvirt_private.syms | 1 +
3 files changed, 64 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 66fe470..73dc33f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23072,6 +23072,60 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
int
+virDomainObjListConvert(virDomainObjListPtr domlist,
+ virConnectPtr conn,
+ virDomainPtr *doms,
+ size_t ndoms,
+ virDomainObjPtr **vms,
+ size_t *nvms,
+ virDomainObjListACLFilter filter,
+ unsigned int flags,
+ bool skip_missing)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virDomainObjPtr vm;
+ size_t i;
+
+ *nvms = 0;
+ *vms = NULL;
+
+ virObjectLock(domlist);
+ for (i = 0; i < ndoms; i++) {
+ virDomainPtr dom = doms[i];
+
+ virUUIDFormat(dom->uuid, uuidstr);
+
+ if (!(vm = virHashLookup(domlist->objs, uuidstr))) {
+ if (!skip_missing) {
+ virReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'
(%s)"),
+ uuidstr, dom->name);
+ virObjectUnlock(domlist);
+ goto error;
+ }
+ } else {
+ virObjectRef(vm);
+
+ if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0)
+ goto error;
+ }
+ }
+ virObjectUnlock(domlist);
+
+ virDomainObjListFilter(vms, nvms, conn, filter, flags);
Coverity complains here too - if 'vms' is NULL, then ObjListFilter
derefs. Coverity shows an example of ndoms == 2 and for each iteration
through the loop it takes the skip_missing path.
John
+
+ return 0;
+
+ error:
+ virObjectListFreeCount(*vms, *nvms);
+ *vms = NULL;
+ *nvms = 0;
+
+ return -1;
+}
+
+
+int
virDomainObjListExport(virDomainObjListPtr domlist,
virConnectPtr conn,
virDomainPtr **domains,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ac11a20..06ca82f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3059,6 +3059,15 @@ int virDomainObjListExport(virDomainObjListPtr doms,
virDomainPtr **domains,
virDomainObjListACLFilter filter,
unsigned int flags);
+int virDomainObjListConvert(virDomainObjListPtr domlist,
+ virConnectPtr conn,
+ virDomainPtr *doms,
+ size_t ndoms,
+ virDomainObjPtr **vms,
+ size_t *nvms,
+ virDomainObjListACLFilter filter,
+ unsigned int flags,
+ bool skip_missing);
int
virDomainDefMaybeAddController(virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9a7a944..67a7e21 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -386,6 +386,7 @@ virDomainObjGetPersistentDef;
virDomainObjGetState;
virDomainObjListAdd;
virDomainObjListCollect;
+virDomainObjListConvert;
virDomainObjListExport;
virDomainObjListFindByID;
virDomainObjListFindByName;
5:42 a.m.
New subject: [libvirt] [PATCH 5/6] conf: Add helper to convert list of virDomains to a list of virDomainObjs
On Mon, May 11, 2015 at 06:33:00 -0400, John Ferlan wrote:
On 04/30/2015 08:44 AM, Peter Krempa wrote:
> Add virDomainObjListConvert that will take a list of virDomains, apply
> filters and return a list of virDomainObjs.
> ---
> src/conf/domain_conf.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_conf.h | 9 ++++++++
> src/libvirt_private.syms | 1 +
> 3 files changed, 64 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 66fe470..73dc33f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -23072,6 +23072,60 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
>
>
> int
> +virDomainObjListConvert(virDomainObjListPtr domlist,
> + virConnectPtr conn,
> + virDomainPtr *doms,
> + size_t ndoms,
> + virDomainObjPtr **vms,
> + size_t *nvms,
> + virDomainObjListACLFilter filter,
> + unsigned int flags,
> + bool skip_missing)
> +{
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virDomainObjPtr vm;
> + size_t i;
> +
> + *nvms = 0;
> + *vms = NULL;
> +
> + virObjectLock(domlist);
> + for (i = 0; i < ndoms; i++) {
> + virDomainPtr dom = doms[i];
> +
> + virUUIDFormat(dom->uuid, uuidstr);
> +
> + if (!(vm = virHashLookup(domlist->objs, uuidstr))) {
> + if (!skip_missing) {
> + virReportError(VIR_ERR_NO_DOMAIN,
> + _("no domain with matching uuid '%s'
(%s)"),
> + uuidstr, dom->name);
> + virObjectUnlock(domlist);
> + goto error;
> + }
> + } else {
> + virObjectRef(vm);
> +
> + if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0)
> + goto error;
> + }
> + }
> + virObjectUnlock(domlist);
> +
> + virDomainObjListFilter(vms, nvms, conn, filter, flags);
Coverity complains here too - if 'vms' is NULL, then ObjListFilter
derefs. Coverity shows an example of ndoms == 2 and for each iteration
through the loop it takes the skip_missing path.
False positive again. If every domain is skipped, then nvms will never
be incremented from it's 0 state where it was pre-set.
virDomainObjListFilter then iterates from 0 while it's less than the
value stored in ndoms ( which is 0) thus it never touches @vms.
Peter
7:44 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: Convert qemuConnectGetAllDomainStats to use new helpers
Use the new domain list collection helpers to avoid going through
virDomainPtrs.
This additionally implements filter capability when called through the
api that accepts domain list filters.
---
src/qemu/qemu_driver.c | 86 ++++++++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 49 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3032b7a..c4a989c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19815,26 +19815,25 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
unsigned int flags)
{
virQEMUDriverPtr driver = conn->privateData;
- virDomainPtr *domlist = NULL;
- virDomainObjPtr dom = NULL;
+ virDomainObjPtr *vms = NULL;
+ virDomainObjPtr vm;
+ size_t nvms;
virDomainStatsRecordPtr *tmpstats = NULL;
bool enforce = !!(flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS);
- int ntempdoms;
int nstats = 0;
size_t i;
int ret = -1;
unsigned int privflags = 0;
unsigned int domflags = 0;
+ unsigned int lflags = flags & (VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE);
- if (ndoms)
- virCheckFlags(VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
- VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
- else
- virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
- VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
- VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE |
- VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
- VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
+ virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE |
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
if (virConnectGetAllDomainStatsEnsureACL(conn) < 0)
return -1;
@@ -19842,58 +19841,53 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
if (qemuDomainGetStatsCheckSupport(&stats, enforce) < 0)
return -1;
- if (!ndoms) {
- unsigned int lflags = flags & (VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
- VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
- VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE);
-
- if ((ntempdoms = virDomainObjListExport(driver->domains,
- conn,
- &domlist,
- virConnectGetAllDomainStatsCheckACL,
- lflags)) < 0)
- goto cleanup;
-
- ndoms = ntempdoms;
- doms = domlist;
+ if (ndoms) {
+ if (virDomainObjListConvert(driver->domains, conn, doms, ndoms, &vms,
+ &nvms, virConnectGetAllDomainStatsCheckACL,
+ lflags, true) < 0)
+ return -1;
+ } else {
+ if (virDomainObjListCollect(driver->domains, conn, &vms, &nvms,
+ virConnectGetAllDomainStatsCheckACL,
+ lflags) < 0)
+ return -1;
}
- if (VIR_ALLOC_N(tmpstats, ndoms + 1) < 0)
- goto cleanup;
+ if (VIR_ALLOC_N(tmpstats, nvms + 1) < 0)
+ return -1;
if (qemuDomainGetStatsNeedMonitor(stats))
privflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
- for (i = 0; i < ndoms; i++) {
+ for (i = 0; i < nvms; i++) {
virDomainStatsRecordPtr tmp = NULL;
domflags = 0;
+ vm = vms[i];
- if (!(dom = qemuDomObjFromDomain(doms[i])))
- continue;
-
- if (doms != domlist &&
- !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) {
- virDomainObjEndAPI(&dom);
- continue;
- }
+ virObjectLock(vm);
if (HAVE_JOB(privflags) &&
- qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) == 0)
+ qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) == 0)
domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
/* else: without a job it's still possible to gather some data */
if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING)
domflags |= QEMU_DOMAIN_STATS_BACKING;
- if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
- goto endjob;
+ if (qemuDomainGetStats(conn, vm, stats, &tmp, domflags) < 0) {
+ if (HAVE_JOB(domflags) && vm)
+ qemuDomainObjEndJob(driver, vm);
+
+ virObjectUnlock(vm);
+ goto cleanup;
+ }
if (tmp)
tmpstats[nstats++] = tmp;
if (HAVE_JOB(domflags))
- qemuDomainObjEndJob(driver, dom);
+ qemuDomainObjEndJob(driver, vm);
- virDomainObjEndAPI(&dom);
+ virObjectUnlock(vm);
}
*retStats = tmpstats;
@@ -19901,15 +19895,9 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
ret = nstats;
- endjob:
- if (HAVE_JOB(domflags) && dom)
- qemuDomainObjEndJob(driver, dom);
-
cleanup:
- virDomainObjEndAPI(&dom);
-
virDomainStatsRecordListFree(tmpstats);
- virObjectListFree(domlist);
+ virObjectListFreeCount(vms, nvms);
return ret;
}
--
2.3.5
6:22 a.m.
On 30.04.2015 14:44, Peter Krempa wrote:
Peter Krempa (6):
util: Make the virDomainListFree helper more universal
conf: Extract code to filter domain list into a separate function
conf: Rename virDomainObjListFilter type to virDomainObjListACLFilter
conf: Refactor domain list collection critical section
conf: Add helper to convert list of virDomains to a list of
virDomainObjs
qemu: Convert qemuConnectGetAllDomainStats to use new helpers
daemon/remote.c | 2 +-
src/conf/domain_conf.c | 258 ++++++++++++++++++++++++++++---------------
src/conf/domain_conf.h | 29 +++--
src/libvirt_private.syms | 5 +-
src/qemu/qemu_driver.c | 86 +++++++--------
src/util/virobject.c | 41 +++++++
src/util/virobject.h | 2 +
tools/virsh-domain-monitor.c | 2 +-
8 files changed, 275 insertions(+), 150 deletions(-)
ACK series, but see my comment to 5/6 before pushing.
Michal
3519
days inactive
3530
days old
13 comments
3 participants
participants (3)
-
John Ferlan -
Michal Privoznik -
Peter Krempa