2:10 p.m.
v2:
- Move the extracted functions to hypervisor/domain_driver.c
- Implement virDomainPinIOThread
- Implement virDomainSetIOThreadParams
- Implement virConnectGetAllDomainStats, currently only supports get
state and IOThread info
- Add more tests
Luke Yue (11):
domain_driver.c: Introduce and use virDomainDriverAddIOThreadCheck()
test_driver: Introduce testIOThreadInfo and generate IOThread infos
test_driver: Implement virDomainAddIOThread
test_driver: Implement virDomainDelIOThread
domain_driver.c: Introduce and use virDomainDriverGetIOThreadsConfig()
test_driver: Implement virDomainGetIOThreadInfo
test_driver: Implement virDomainPinIOThread
test_driver: Implement testDomainSetIOThreadParams
test_driver: Implement virConnectGetAllDomainStats
test_driver: Introduce testDomainGetStatsIOThread
tests: Test IOThread related functions for test driver
examples/xml/test/testdomfc4.xml | 5 +
src/hypervisor/domain_driver.c | 123 ++++++++
src/hypervisor/domain_driver.h | 9 +
src/libvirt_private.syms | 3 +
src/qemu/qemu_driver.c | 113 +------
src/test/meson.build | 1 +
src/test/test_driver.c | 509 +++++++++++++++++++++++++++++++
tests/virshtest.c | 90 ++++++
8 files changed, 745 insertions(+), 108 deletions(-)
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 01/11] domain_driver.c: Introduce and use virDomainDriverAddIOThreadCheck()
The test driver can share the same code with qemu driver when implement
testDomainAddIOThreadCheck and testDomainDelIOThreadCheck, so extract
them for test driver to use.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/hypervisor/domain_driver.c | 64 ++++++++++++++++++++++++++++++++++
src/hypervisor/domain_driver.h | 6 ++++
src/libvirt_private.syms | 2 ++
src/qemu/qemu_driver.c | 60 +++----------------------------
4 files changed, 76 insertions(+), 56 deletions(-)
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index 29e11c0447..0d4d9ae5de 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -511,3 +511,67 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
return virHostdevPCINodeDeviceDetach(hostdevMgr, pci);
}
+
+/**
+ * virDomainDriverAddIOThreadCheck:
+ * @def: domain definition
+ * @iothread_id: iothread id
+ *
+ * Returns -1 if an IOThread is already using the given iothread id
+ */
+int
+virDomainDriverAddIOThreadCheck(virDomainDef *def,
+ unsigned int iothread_id)
+{
+ if (virDomainIOThreadIDFind(def, iothread_id)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("an IOThread is already using iothread_id
'%u'"),
+ iothread_id);
+ return -1;
+ }
+
+ return 0;
+}
+
+/**
+ * virDomainDriverDelIOThreadCheck:
+ * @def: domain definition
+ * @iothread_id: iothread id
+ *
+ * Returns -1 if there is no IOThread using the given iothread id
+ */
+int
+virDomainDriverDelIOThreadCheck(virDomainDef *def,
+ unsigned int iothread_id)
+{
+ size_t i;
+
+ if (!virDomainIOThreadIDFind(def, iothread_id)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("cannot find IOThread '%u' in iothreadids
list"),
+ iothread_id);
+ return -1;
+ }
+
+ for (i = 0; i < def->ndisks; i++) {
+ if (def->disks[i]->iothread == iothread_id) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("cannot remove IOThread %u since it "
+ "is being used by disk '%s'"),
+ iothread_id, def->disks[i]->dst);
+ return -1;
+ }
+ }
+
+ for (i = 0; i < def->ncontrollers; i++) {
+ if (def->controllers[i]->iothread == iothread_id) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("cannot remove IOThread '%u' since it "
+ "is being used by controller"),
+ iothread_id);
+ return -1;
+ }
+ }
+
+ return 0;
+}
diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h
index 5970eef082..d91d21bc91 100644
--- a/src/hypervisor/domain_driver.h
+++ b/src/hypervisor/domain_driver.h
@@ -60,3 +60,9 @@ int virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev,
int virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
virHostdevManager *hostdevMgr,
const char *driverName);
+
+int virDomainDriverAddIOThreadCheck(virDomainDef *def,
+ unsigned int iothread_id);
+
+int virDomainDriverDelIOThreadCheck(virDomainDef *def,
+ unsigned int iothread_id);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 43493ea76e..fa2412c0dc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1531,6 +1531,8 @@ virDomainCgroupSetupMemtune;
# hypervisor/domain_driver.h
+virDomainDriverAddIOThreadCheck;
+virDomainDriverDelIOThreadCheck;
virDomainDriverGenerateMachineName;
virDomainDriverGenerateRootHash;
virDomainDriverMergeBlkioDevice;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0b35bbc15c..b96ac985bf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5477,58 +5477,6 @@ qemuDomainHotplugDelIOThread(virQEMUDriver *driver,
}
-static int
-qemuDomainAddIOThreadCheck(virDomainDef *def,
- unsigned int iothread_id)
-{
- if (virDomainIOThreadIDFind(def, iothread_id)) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("an IOThread is already using iothread_id
'%u'"),
- iothread_id);
- return -1;
- }
-
- return 0;
-}
-
-
-static int
-qemuDomainDelIOThreadCheck(virDomainDef *def,
- unsigned int iothread_id)
-{
- size_t i;
-
- if (!virDomainIOThreadIDFind(def, iothread_id)) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("cannot find IOThread '%u' in iothreadids
list"),
- iothread_id);
- return -1;
- }
-
- for (i = 0; i < def->ndisks; i++) {
- if (def->disks[i]->iothread == iothread_id) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("cannot remove IOThread %u since it "
- "is being used by disk '%s'"),
- iothread_id, def->disks[i]->dst);
- return -1;
- }
- }
-
- for (i = 0; i < def->ncontrollers; i++) {
- if (def->controllers[i]->iothread == iothread_id) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("cannot remove IOThread '%u' since it "
- "is being used by controller"),
- iothread_id);
- return -1;
- }
- }
-
- return 0;
-}
-
-
/**
* @params: Pointer to params list
* @nparams: Number of params to be parsed
@@ -5662,7 +5610,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver,
switch (action) {
case VIR_DOMAIN_IOTHREAD_ACTION_ADD:
- if (qemuDomainAddIOThreadCheck(def, iothread.iothread_id) < 0)
+ if (virDomainDriverAddIOThreadCheck(def, iothread.iothread_id) < 0)
goto endjob;
if (qemuDomainHotplugAddIOThread(driver, vm, iothread.iothread_id) < 0)
@@ -5671,7 +5619,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver,
break;
case VIR_DOMAIN_IOTHREAD_ACTION_DEL:
- if (qemuDomainDelIOThreadCheck(def, iothread.iothread_id) < 0)
+ if (virDomainDriverDelIOThreadCheck(def, iothread.iothread_id) < 0)
goto endjob;
if (qemuDomainHotplugDelIOThread(driver, vm, iothread.iothread_id) < 0)
@@ -5701,7 +5649,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver,
if (persistentDef) {
switch (action) {
case VIR_DOMAIN_IOTHREAD_ACTION_ADD:
- if (qemuDomainAddIOThreadCheck(persistentDef, iothread.iothread_id) < 0)
+ if (virDomainDriverAddIOThreadCheck(persistentDef, iothread.iothread_id) <
0)
goto endjob;
if (!virDomainIOThreadIDAdd(persistentDef, iothread.iothread_id))
@@ -5710,7 +5658,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver,
break;
case VIR_DOMAIN_IOTHREAD_ACTION_DEL:
- if (qemuDomainDelIOThreadCheck(persistentDef, iothread.iothread_id) < 0)
+ if (virDomainDriverDelIOThreadCheck(persistentDef, iothread.iothread_id) <
0)
goto endjob;
virDomainIOThreadIDDel(persistentDef, iothread.iothread_id);
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 02/11] test_driver: Introduce testIOThreadInfo and generate IOThread infos
Introduce testIOThreadInfo to store IOThread infos: iothread_id,
poll_max_ns, poll_grow and poll_shrink for future usage.
Add an example of IOThread configuration to testdomfc4.xml, we also want
to generate default testIOThreadInfo for the IOThread configured in the
xml, so introduce testDomainGenerateIOThreadInfos, the values are taken
from QEMU.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
examples/xml/test/testdomfc4.xml | 5 +++++
src/test/test_driver.c | 37 ++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
diff --git a/examples/xml/test/testdomfc4.xml b/examples/xml/test/testdomfc4.xml
index 26b7f25a06..cb4dd0cf70 100644
--- a/examples/xml/test/testdomfc4.xml
+++ b/examples/xml/test/testdomfc4.xml
@@ -11,6 +11,11 @@
<memory>261072</memory>
<currentMemory>131072</currentMemory>
<vcpu>1</vcpu>
+ <iothreads>2</iothreads>
+ <iothreadids>
+ <iothread id="2"/>
+ <iothread id="4"/>
+ </iothreadids>
<devices>
<disk type='file'>
<source file='/u/fc4.img'/>
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 892dc978f2..7d9a81de54 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -182,6 +182,14 @@ struct _testDomainNamespaceDef {
xmlNodePtr *snap_nodes;
};
+typedef struct _testIOThreadInfo testIOThreadInfo;
+struct _testIOThreadInfo {
+ unsigned int iothread_id;
+ unsigned long long poll_max_ns;
+ unsigned int poll_grow;
+ unsigned int poll_shrink;
+};
+
static void
testDomainDefNamespaceFree(void *data)
{
@@ -380,6 +388,9 @@ struct _testDomainObjPrivate {
/* used by get/set time APIs */
long long seconds;
unsigned int nseconds;
+
+ /* used by IOThread APIs */
+ GArray *iothreads;
};
@@ -396,6 +407,8 @@ testDomainObjPrivateAlloc(void *opaque)
priv->seconds = 627319920;
priv->nseconds = 0;
+ priv->iothreads = g_array_new(FALSE, FALSE, sizeof(testIOThreadInfo));
+
return priv;
}
@@ -426,6 +439,8 @@ static void
testDomainObjPrivateFree(void *data)
{
testDomainObjPrivate *priv = data;
+
+ g_array_free(priv->iothreads, TRUE);
g_free(priv);
}
@@ -695,6 +710,26 @@ testDomainGenerateIfnames(virDomainDef *domdef)
return 0;
}
+static void
+testDomainGenerateIOThreadInfos(virDomainObj *obj)
+{
+ size_t i;
+ testDomainObjPrivate *priv;
+
+ if (!obj->def->iothreadids || !obj->def->niothreadids)
+ return;
+
+ priv = obj->privateData;
+
+ for (i = 0; i < obj->def->niothreadids; i++) {
+ testIOThreadInfo iothread;
+ iothread.iothread_id = obj->def->iothreadids[i]->iothread_id;
+ iothread.poll_max_ns = 32768;
+ iothread.poll_grow = 0;
+ iothread.poll_shrink = 0;
+ g_array_append_val(priv->iothreads, iothread);
+ }
+}
static void
testDomainShutdownState(virDomainPtr domain,
@@ -968,6 +1003,8 @@ testParseDomains(testDriver *privconn,
}
virDomainObjSetState(obj, nsdata->runstate, 0);
+ testDomainGenerateIOThreadInfos(obj);
+
virDomainObjEndAPI(&obj);
}
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 03/11] test_driver: Implement virDomainAddIOThread
Introduce testDomainChgIOThread at the same time
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/meson.build | 1 +
src/test/test_driver.c | 77 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+)
diff --git a/src/test/meson.build b/src/test/meson.build
index f54585adfd..c0174ad856 100644
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -17,6 +17,7 @@ if conf.has('WITH_TEST')
],
include_directories: [
conf_inc_dir,
+ hypervisor_inc_dir,
],
)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 7d9a81de54..3cabd6dc46 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -40,6 +40,7 @@
#include "interface_conf.h"
#include "checkpoint_conf.h"
#include "domain_conf.h"
+#include "domain_driver.h"
#include "domain_event.h"
#include "network_event.h"
#include "snapshot_conf.h"
@@ -9390,6 +9391,81 @@ testDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
return ret;
}
+typedef enum {
+ VIR_DOMAIN_IOTHREAD_ACTION_ADD,
+} virDomainIOThreadAction;
+
+static int
+testDomainChgIOThread(virDomainObj *vm,
+ unsigned int iothread_id,
+ virDomainIOThreadAction action,
+ unsigned int flags)
+{
+ virDomainDef *def;
+ int ret = -1;
+
+ if (!(def = virDomainObjGetOneDef(vm, flags)))
+ return ret;
+
+ if (def) {
+ switch (action) {
+ case VIR_DOMAIN_IOTHREAD_ACTION_ADD:
+ if (virDomainDriverAddIOThreadCheck(def, iothread_id) < 0)
+ return ret;
+
+ if (!virDomainIOThreadIDAdd(def, iothread_id))
+ return ret;
+
+ break;
+ }
+ }
+
+ ret = 0;
+
+ return ret;
+}
+
+static int
+testDomainAddIOThread(virDomainPtr dom,
+ unsigned int iothread_id,
+ unsigned int flags)
+{
+ virDomainObj *vm = NULL;
+ testDomainObjPrivate *priv;
+ testIOThreadInfo iothread;
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+ if (iothread_id == 0) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("invalid value of 0 for iothread_id"));
+ return -1;
+ }
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (testDomainChgIOThread(vm, iothread_id,
+ VIR_DOMAIN_IOTHREAD_ACTION_ADD, flags) < 0)
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ iothread.iothread_id = iothread_id;
+ iothread.poll_max_ns = 32768;
+ iothread.poll_grow = 0;
+ iothread.poll_shrink = 0;
+
+ g_array_append_val(priv->iothreads, iothread);
+
+ ret = 0;
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
/*
* Test driver
*/
@@ -9456,6 +9532,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetVcpus = testDomainGetVcpus, /* 0.7.3 */
.domainGetVcpuPinInfo = testDomainGetVcpuPinInfo, /* 1.2.18 */
.domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
+ .domainAddIOThread = testDomainAddIOThread, /* 7.7.0 */
.domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
.nodeGetSecurityModel = testNodeGetSecurityModel, /* 7.5.0 */
.domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 04/11] test_driver: Implement virDomainDelIOThread
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 55 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 3cabd6dc46..99d5d5b67b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9393,6 +9393,7 @@ testDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
typedef enum {
VIR_DOMAIN_IOTHREAD_ACTION_ADD,
+ VIR_DOMAIN_IOTHREAD_ACTION_DEL,
} virDomainIOThreadAction;
static int
@@ -9417,6 +9418,14 @@ testDomainChgIOThread(virDomainObj *vm,
return ret;
break;
+
+ case VIR_DOMAIN_IOTHREAD_ACTION_DEL:
+ if (virDomainDriverDelIOThreadCheck(def, iothread_id) < 0)
+ return ret;
+
+ virDomainIOThreadIDDel(def, iothread_id);
+
+ break;
}
}
@@ -9466,6 +9475,51 @@ testDomainAddIOThread(virDomainPtr dom,
virDomainObjEndAPI(&vm);
return ret;
}
+
+static int
+testDomainDelIOThread(virDomainPtr dom,
+ unsigned int iothread_id,
+ unsigned int flags)
+{
+ virDomainObj *vm = NULL;
+ testDomainObjPrivate *priv;
+ size_t i;
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+ if (iothread_id == 0) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("invalid value of 0 for iothread_id"));
+ return -1;
+ }
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (testDomainChgIOThread(vm, iothread_id,
+ VIR_DOMAIN_IOTHREAD_ACTION_DEL, flags) < 0)
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ for (i = 0; i < priv->iothreads->len; i++) {
+ testIOThreadInfo iothread = g_array_index(priv->iothreads,
+ testIOThreadInfo, i);
+ if (iothread.iothread_id == iothread_id) {
+ g_array_remove_index(priv->iothreads, i);
+ break;
+ }
+ }
+
+ ret = 0;
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
/*
* Test driver
*/
@@ -9533,6 +9587,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetVcpuPinInfo = testDomainGetVcpuPinInfo, /* 1.2.18 */
.domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
.domainAddIOThread = testDomainAddIOThread, /* 7.7.0 */
+ .domainDelIOThread = testDomainDelIOThread, /* 7.7.0 */
.domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
.nodeGetSecurityModel = testNodeGetSecurityModel, /* 7.5.0 */
.domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 05/11] domain_driver.c: Introduce and use virDomainDriverGetIOThreadsConfig()
The test driver can share the same code with qemu driver when implement
testDomainGetIOThreadsConfig, so extract it for test driver to use.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/hypervisor/domain_driver.c | 59 ++++++++++++++++++++++++++++++++++
src/hypervisor/domain_driver.h | 3 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 53 +-----------------------------
4 files changed, 64 insertions(+), 52 deletions(-)
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index 0d4d9ae5de..fd0665daaf 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -575,3 +575,62 @@ virDomainDriverDelIOThreadCheck(virDomainDef *def,
return 0;
}
+
+/**
+ * virDomainDriverGetIOThreadsConfig:
+ * @targetDef: domain definition
+ * @info: information about the IOThread in a domain
+ *
+ * Returns the number of IOThreads in the given domain or -1 in case of error
+ */
+int
+virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef,
+ virDomainIOThreadInfoPtr **info)
+{
+ virDomainIOThreadInfoPtr *info_ret = NULL;
+ virBitmap *bitmap = NULL;
+ virBitmap *cpumask = NULL;
+ size_t i;
+ int ret = -1;
+
+ if (targetDef->niothreadids == 0)
+ return 0;
+
+ info_ret = g_new0(virDomainIOThreadInfoPtr, targetDef->niothreadids);
+
+ for (i = 0; i < targetDef->niothreadids; i++) {
+ info_ret[i] = g_new0(virDomainIOThreadInfo, 1);
+
+ /* IOThread ID's are taken from the iothreadids list */
+ info_ret[i]->iothread_id = targetDef->iothreadids[i]->iothread_id;
+
+ cpumask = targetDef->iothreadids[i]->cpumask;
+ if (!cpumask) {
+ if (targetDef->cpumask) {
+ cpumask = targetDef->cpumask;
+ } else {
+ if (!(bitmap = virHostCPUGetAvailableCPUsBitmap()))
+ goto cleanup;
+ cpumask = bitmap;
+ }
+ }
+ if (virBitmapToData(cpumask, &info_ret[i]->cpumap,
+ &info_ret[i]->cpumaplen) < 0)
+ goto cleanup;
+ virBitmapFree(bitmap);
+ bitmap = NULL;
+ }
+
+ *info = g_steal_pointer(&info_ret);
+ ret = targetDef->niothreadids;
+
+ cleanup:
+ if (info_ret) {
+ for (i = 0; i < targetDef->niothreadids; i++)
+ virDomainIOThreadInfoFree(info_ret[i]);
+ VIR_FREE(info_ret);
+ }
+ virBitmapFree(bitmap);
+
+ return ret;
+}
diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h
index d91d21bc91..638f01274b 100644
--- a/src/hypervisor/domain_driver.h
+++ b/src/hypervisor/domain_driver.h
@@ -66,3 +66,6 @@ int virDomainDriverAddIOThreadCheck(virDomainDef *def,
int virDomainDriverDelIOThreadCheck(virDomainDef *def,
unsigned int iothread_id);
+
+int virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef,
+ virDomainIOThreadInfoPtr **info);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fa2412c0dc..4db2c5362d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1535,6 +1535,7 @@ virDomainDriverAddIOThreadCheck;
virDomainDriverDelIOThreadCheck;
virDomainDriverGenerateMachineName;
virDomainDriverGenerateRootHash;
+virDomainDriverGetIOThreadsConfig;
virDomainDriverMergeBlkioDevice;
virDomainDriverNodeDeviceDetachFlags;
virDomainDriverNodeDeviceGetPCIInfo;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b96ac985bf..81e3f94f8b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5079,57 +5079,6 @@ qemuDomainGetIOThreadsLive(virQEMUDriver *driver,
return ret;
}
-static int
-qemuDomainGetIOThreadsConfig(virDomainDef *targetDef,
- virDomainIOThreadInfoPtr **info)
-{
- virDomainIOThreadInfoPtr *info_ret = NULL;
- virBitmap *bitmap = NULL;
- virBitmap *cpumask = NULL;
- size_t i;
- int ret = -1;
-
- if (targetDef->niothreadids == 0)
- return 0;
-
- info_ret = g_new0(virDomainIOThreadInfoPtr, targetDef->niothreadids);
-
- for (i = 0; i < targetDef->niothreadids; i++) {
- info_ret[i] = g_new0(virDomainIOThreadInfo, 1);
-
- /* IOThread ID's are taken from the iothreadids list */
- info_ret[i]->iothread_id = targetDef->iothreadids[i]->iothread_id;
-
- cpumask = targetDef->iothreadids[i]->cpumask;
- if (!cpumask) {
- if (targetDef->cpumask) {
- cpumask = targetDef->cpumask;
- } else {
- if (!(bitmap = virHostCPUGetAvailableCPUsBitmap()))
- goto cleanup;
- cpumask = bitmap;
- }
- }
- if (virBitmapToData(cpumask, &info_ret[i]->cpumap,
- &info_ret[i]->cpumaplen) < 0)
- goto cleanup;
- virBitmapFree(bitmap);
- bitmap = NULL;
- }
-
- *info = g_steal_pointer(&info_ret);
- ret = targetDef->niothreadids;
-
- cleanup:
- if (info_ret) {
- for (i = 0; i < targetDef->niothreadids; i++)
- virDomainIOThreadInfoFree(info_ret[i]);
- VIR_FREE(info_ret);
- }
- virBitmapFree(bitmap);
-
- return ret;
-}
static int
qemuDomainGetIOThreadInfo(virDomainPtr dom,
@@ -5156,7 +5105,7 @@ qemuDomainGetIOThreadInfo(virDomainPtr dom,
if (!targetDef)
ret = qemuDomainGetIOThreadsLive(driver, vm, info);
else
- ret = qemuDomainGetIOThreadsConfig(targetDef, info);
+ ret = virDomainDriverGetIOThreadsConfig(targetDef, info);
cleanup:
virDomainObjEndAPI(&vm);
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 06/11] test_driver: Implement virDomainGetIOThreadInfo
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 99d5d5b67b..d4958e4818 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9391,6 +9391,31 @@ testDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
return ret;
}
+static int
+testDomainGetIOThreadInfo(virDomainPtr dom,
+ virDomainIOThreadInfoPtr **info,
+ unsigned int flags)
+{
+ virDomainObj *vm;
+ virDomainDef *targetDef = NULL;
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (!(targetDef = virDomainObjGetOneDef(vm, flags)))
+ goto cleanup;
+
+ ret = virDomainDriverGetIOThreadsConfig(targetDef, info);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
typedef enum {
VIR_DOMAIN_IOTHREAD_ACTION_ADD,
VIR_DOMAIN_IOTHREAD_ACTION_DEL,
@@ -9586,6 +9611,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetVcpus = testDomainGetVcpus, /* 0.7.3 */
.domainGetVcpuPinInfo = testDomainGetVcpuPinInfo, /* 1.2.18 */
.domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
+ .domainGetIOThreadInfo = testDomainGetIOThreadInfo, /* 7.7.0 */
.domainAddIOThread = testDomainAddIOThread, /* 7.7.0 */
.domainDelIOThread = testDomainDelIOThread, /* 7.7.0 */
.domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 07/11] test_driver: Implement virDomainPinIOThread
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d4958e4818..f192e800bf 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9545,6 +9545,55 @@ testDomainDelIOThread(virDomainPtr dom,
return ret;
}
+static int
+testDomainPinIOThread(virDomainPtr dom,
+ unsigned int iothread_id,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags)
+{
+ int ret = -1;
+ virDomainObj *vm;
+ virDomainDef *def;
+ virDomainIOThreadIDDef *iothrid;
+ virBitmap *cpumask = NULL;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (!(def = virDomainObjGetOneDef(vm, flags)))
+ goto cleanup;
+
+ if (!(cpumask = virBitmapNewData(cpumap, maplen)))
+ goto cleanup;
+
+ if (virBitmapIsAllClear(cpumask)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Empty iothread cpumap list for pinning"));
+ goto cleanup;
+ }
+
+ if (!(iothrid = virDomainIOThreadIDFind(def, iothread_id))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("iothreadid %d not found"), iothread_id);
+ goto cleanup;
+ }
+
+ virBitmapFree(iothrid->cpumask);
+ iothrid->cpumask = g_steal_pointer(&cpumask);
+ iothrid->autofill = false;
+
+ ret = 0;
+
+ cleanup:
+ virBitmapFree(cpumask);
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
/*
* Test driver
*/
@@ -9614,6 +9663,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetIOThreadInfo = testDomainGetIOThreadInfo, /* 7.7.0 */
.domainAddIOThread = testDomainAddIOThread, /* 7.7.0 */
.domainDelIOThread = testDomainDelIOThread, /* 7.7.0 */
+ .domainPinIOThread = testDomainPinIOThread, /* 7.7.0 */
.domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
.nodeGetSecurityModel = testNodeGetSecurityModel, /* 7.5.0 */
.domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 08/11] test_driver: Implement testDomainSetIOThreadParams
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 90 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index f192e800bf..ca36e655f4 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9419,6 +9419,7 @@ testDomainGetIOThreadInfo(virDomainPtr dom,
typedef enum {
VIR_DOMAIN_IOTHREAD_ACTION_ADD,
VIR_DOMAIN_IOTHREAD_ACTION_DEL,
+ VIR_DOMAIN_IOTHREAD_ACTION_MOD,
} virDomainIOThreadAction;
static int
@@ -9450,6 +9451,16 @@ testDomainChgIOThread(virDomainObj *vm,
virDomainIOThreadIDDel(def, iothread_id);
+ break;
+
+ case VIR_DOMAIN_IOTHREAD_ACTION_MOD:
+ if (!(virDomainIOThreadIDFind(def, iothread_id))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("cannot find IOThread '%u' in
iothreadids"),
+ iothread_id);
+ return ret;
+ }
+
break;
}
}
@@ -9594,6 +9605,84 @@ testDomainPinIOThread(virDomainPtr dom,
return ret;
}
+static int
+testDomainIOThreadParseParams(virTypedParameterPtr params,
+ int nparams,
+ testIOThreadInfo *iothread)
+{
+ if (virTypedParamsValidate(params, nparams,
+ VIR_DOMAIN_IOTHREAD_POLL_MAX_NS,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_IOTHREAD_POLL_GROW,
+ VIR_TYPED_PARAM_UINT,
+ VIR_DOMAIN_IOTHREAD_POLL_SHRINK,
+ VIR_TYPED_PARAM_UINT,
+ NULL) < 0)
+ return -1;
+
+ if (virTypedParamsGetULLong(params, nparams,
+ VIR_DOMAIN_IOTHREAD_POLL_MAX_NS,
+ &iothread->poll_max_ns) < 0)
+ return -1;
+
+ if (virTypedParamsGetUInt(params, nparams,
+ VIR_DOMAIN_IOTHREAD_POLL_GROW,
+ &iothread->poll_grow) < 0)
+ return -1;
+
+ if (virTypedParamsGetUInt(params, nparams,
+ VIR_DOMAIN_IOTHREAD_POLL_SHRINK,
+ &iothread->poll_shrink) < 0)
+ return -1;
+
+ return 0;
+}
+
+static int
+testDomainSetIOThreadParams(virDomainPtr dom,
+ unsigned int iothread_id,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
+{
+ virDomainObj *vm = NULL;
+ testDomainObjPrivate *priv;
+ size_t i;
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE, -1);
+
+ if (iothread_id == 0) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("invalid value of 0 for iothread_id"));
+ goto cleanup;
+ }
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (testDomainChgIOThread(vm, iothread_id,
+ VIR_DOMAIN_IOTHREAD_ACTION_MOD, flags) < 0)
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ for (i = 0; i < priv->iothreads->len; i++) {
+ testIOThreadInfo *iothread = &g_array_index(priv->iothreads,
+ testIOThreadInfo, i);
+ if (iothread->iothread_id == iothread_id) {
+ if (testDomainIOThreadParseParams(params, nparams, iothread) < 0)
+ goto cleanup;
+ ret = 0;
+ break;
+ }
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
/*
* Test driver
*/
@@ -9664,6 +9753,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainAddIOThread = testDomainAddIOThread, /* 7.7.0 */
.domainDelIOThread = testDomainDelIOThread, /* 7.7.0 */
.domainPinIOThread = testDomainPinIOThread, /* 7.7.0 */
+ .domainSetIOThreadParams = testDomainSetIOThreadParams, /* 7.7.0 */
.domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
.nodeGetSecurityModel = testNodeGetSecurityModel, /* 7.5.0 */
.domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 09/11] test_driver: Implement virConnectGetAllDomainStats
Implement virConnectGetAllDomainStats in a modular way just like QEMU
driver, though remove some params in GetStatsWorker that we don't need
in test driver currently.
Only add the worker to get state so far, more worker will be added
in the future.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 132 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 132 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ca36e655f4..5da7f23a9b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9683,6 +9683,137 @@ testDomainSetIOThreadParams(virDomainPtr dom,
return ret;
}
+static int
+testDomainGetStatsState(virDomainObj *dom,
+ virTypedParamList *params)
+{
+ if (virTypedParamListAddInt(params, dom->state.state, "state.state")
< 0)
+ return -1;
+
+ if (virTypedParamListAddInt(params, dom->state.reason, "state.reason")
< 0)
+ return -1;
+
+ return 0;
+}
+
+typedef int
+(*testDomainGetStatsFunc)(virDomainObj *dom,
+ virTypedParamList *list);
+
+struct testDomainGetStatsWorker {
+ testDomainGetStatsFunc func;
+ unsigned int stats;
+};
+
+static struct testDomainGetStatsWorker testDomainGetStatsWorkers[] = {
+ { testDomainGetStatsState, VIR_DOMAIN_STATS_STATE },
+ { NULL, 0 }
+};
+
+static int
+testDomainGetStats(virConnectPtr conn,
+ virDomainObj *dom,
+ unsigned int stats,
+ virDomainStatsRecordPtr *record)
+{
+ g_autofree virDomainStatsRecordPtr tmp = NULL;
+ g_autoptr(virTypedParamList) params = NULL;
+ size_t i;
+
+ params = g_new0(virTypedParamList, 1);
+
+ for (i = 0; testDomainGetStatsWorkers[i].func; i++) {
+ if (stats & testDomainGetStatsWorkers[i].stats) {
+ if (testDomainGetStatsWorkers[i].func(dom, params) < 0)
+ return -1;
+ }
+ }
+
+ tmp = g_new0(virDomainStatsRecord, 1);
+
+ if (!(tmp->dom = virGetDomain(conn, dom->def->name,
+ dom->def->uuid, dom->def->id)))
+ return -1;
+
+ tmp->nparams = virTypedParamListStealParams(params, &tmp->params);
+ *record = g_steal_pointer(&tmp);
+ return 0;
+}
+
+static int
+testConnectGetAllDomainStats(virConnectPtr conn,
+ virDomainPtr *doms,
+ unsigned int ndoms,
+ unsigned int stats,
+ virDomainStatsRecordPtr **retStats,
+ unsigned int flags)
+{
+ testDriver *driver = conn->privateData;
+ unsigned int lflags = flags & (VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE);
+
+ unsigned int supported = VIR_DOMAIN_STATS_STATE;
+ virDomainObj **vms = NULL;
+ size_t nvms;
+ virDomainStatsRecordPtr *tmpstats = NULL;
+ int nstats = 0;
+ int ret = -1;
+ size_t i;
+
+ 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_NOWAIT |
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
+
+ if (!stats) {
+ stats = supported;
+ } else if ((flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS) &&
+ (stats & ~supported)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("Stats types bits 0x%x are not supported by this
daemon"),
+ stats & ~supported);
+ return -1;
+ }
+
+ if (ndoms) {
+ if (virDomainObjListConvert(driver->domains, conn, doms, ndoms, &vms,
+ &nvms, NULL, lflags, true) < 0)
+ return -1;
+ } else {
+ if (virDomainObjListCollect(driver->domains, conn, &vms, &nvms,
+ NULL, lflags) < 0)
+ return -1;
+ }
+
+ tmpstats = g_new0(virDomainStatsRecordPtr, nvms + 1);
+
+ for (i = 0; i < nvms; i++) {
+ virDomainStatsRecordPtr tmp;
+ virDomainObj *vm = vms[i];
+
+ virObjectLock(vm);
+ testDomainGetStats(conn, vm, stats, &tmp);
+ virObjectUnlock(vm);
+
+ if (!tmp)
+ goto cleanup;
+
+ tmpstats[nstats++] = tmp;
+ }
+
+ *retStats = g_steal_pointer(&tmpstats);
+ ret = nstats;
+
+ cleanup:
+ virDomainStatsRecordListFree(tmpstats);
+ virObjectListFreeCount(vms, nvms);
+
+ return ret;
+}
+
/*
* Test driver
*/
@@ -9697,6 +9828,7 @@ static virHypervisorDriver testHypervisorDriver = {
.nodeGetCPUStats = testNodeGetCPUStats, /* 2.3.0 */
.nodeGetFreeMemory = testNodeGetFreeMemory, /* 2.3.0 */
.nodeGetFreePages = testNodeGetFreePages, /* 2.3.0 */
+ .connectGetAllDomainStats = testConnectGetAllDomainStats, /* 7.7.0 */
.connectGetCapabilities = testConnectGetCapabilities, /* 0.2.1 */
.connectGetSysinfo = testConnectGetSysinfo, /* 2.3.0 */
.connectGetType = testConnectGetType, /* 2.3.0 */
--
2.32.0
3:23 p.m.
New subject: [PATCH v2 09/11] test_driver: Implement virConnectGetAllDomainStats
On Thu, Jul 29, 2021 at 08:10:56PM +0800, Luke Yue wrote:
Implement virConnectGetAllDomainStats in a modular way just like QEMU
driver, though remove some params in GetStatsWorker that we don't need
in test driver currently.
Only add the worker to get state so far, more worker will be added
in the future.
Sorry for not getting to this earlier. I did not review the series yet,
but there were some conflicts when applying and the patches did not
compile for me, so I figured I'll let you know at least.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 132 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 132 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ca36e655f4..5da7f23a9b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
[...]
+static int
+testConnectGetAllDomainStats(virConnectPtr conn,
+ virDomainPtr *doms,
+ unsigned int ndoms,
+ unsigned int stats,
+ virDomainStatsRecordPtr **retStats,
+ unsigned int flags)
+{
+ testDriver *driver = conn->privateData;
+ unsigned int lflags = flags & (VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE);
+
+ unsigned int supported = VIR_DOMAIN_STATS_STATE;
+ virDomainObj **vms = NULL;
+ size_t nvms;
+ virDomainStatsRecordPtr *tmpstats = NULL;
+ int nstats = 0;
+ int ret = -1;
+ size_t i;
+
+ 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_NOWAIT |
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
+
+ if (!stats) {
+ stats = supported;
+ } else if ((flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS) &&
+ (stats & ~supported)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("Stats types bits 0x%x are not supported by this
daemon"),
+ stats & ~supported);
+ return -1;
+ }
+
+ if (ndoms) {
+ if (virDomainObjListConvert(driver->domains, conn, doms, ndoms, &vms,
+ &nvms, NULL, lflags, true) < 0)
+ return -1;
+ } else {
+ if (virDomainObjListCollect(driver->domains, conn, &vms, &nvms,
+ NULL, lflags) < 0)
+ return -1;
+ }
+
+ tmpstats = g_new0(virDomainStatsRecordPtr, nvms + 1);
+
+ for (i = 0; i < nvms; i++) {
+ virDomainStatsRecordPtr tmp;
+ virDomainObj *vm = vms[i];
+
+ virObjectLock(vm);
+ testDomainGetStats(conn, vm, stats, &tmp);
This function can fail and there is no way of knowing without checking
the error code. It might look like it is fine because &tmp will be
NULL, but the function does not clear it and you did not initialise it
here. Initialisation to sensible defaults is almost always good, even
if you just override it later on.
+ virObjectUnlock(vm);
+
+ if (!tmp)
+ goto cleanup;
+
+ tmpstats[nstats++] = tmp;
And due to above reasons I get this here:
../src/test/test_driver.c: In function ‘testConnectGetAllDomainStats’:
../src/test/test_driver.c:9952:28: error: ‘tmp’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
9952 | tmpstats[nstats++] = tmp;
| ~~~~~~~~~~~~~~~~~~~^~~~~
I am compiling it with GCC 11.2.0. But it should also fail in our CI,
which would make life easier for you, so I suggest you set up a GitLab
account and setup a CI for your branches there. You might go the extra
mile and set up cirrus testing as described somewhere under ci/, if you
want.
I would suggest checking the return value _and_ initialising the pointer
to NULL. And I promise I'll have a look at the series ASAP if you
rebase it to current master ;)
Thanks,
Martin
4:48 p.m.
New subject: [PATCH v2 09/11] test_driver: Implement virConnectGetAllDomainStats
On Wed, 2021-08-11 at 15:23 +0200, Martin Kletzander wrote:
On Thu, Jul 29, 2021 at 08:10:56PM +0800, Luke Yue wrote:
> Implement virConnectGetAllDomainStats in a modular way just like
> QEMU
> driver, though remove some params in GetStatsWorker that we don't
> need
> in test driver currently.
>
> Only add the worker to get state so far, more worker will be added
> in the future.
>
Sorry for not getting to this earlier. I did not review the series
yet,
but there were some conflicts when applying and the patches did not
compile for me, so I figured I'll let you know at least.
> Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
> ---
> src/test/test_driver.c | 132
> +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 132 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index ca36e655f4..5da7f23a9b 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
[...]
> +static int
> +testConnectGetAllDomainStats(virConnectPtr conn,
> + virDomainPtr *doms,
> + unsigned int ndoms,
> + unsigned int stats,
> + virDomainStatsRecordPtr **retStats,
> + unsigned int flags)
> +{
> + testDriver *driver = conn->privateData;
> + unsigned int lflags = flags &
> (VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
> +
> VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
> +
> VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE);
> +
> + unsigned int supported = VIR_DOMAIN_STATS_STATE;
> + virDomainObj **vms = NULL;
> + size_t nvms;
> + virDomainStatsRecordPtr *tmpstats = NULL;
> + int nstats = 0;
> + int ret = -1;
> + size_t i;
> +
> + 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_NOWAIT |
> + VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
> + VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS,
> -1);
> +
> + if (!stats) {
> + stats = supported;
> + } else if ((flags &
> VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS) &&
> + (stats & ~supported)) {
> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> + _("Stats types bits 0x%x are not supported
> by this daemon"),
> + stats & ~supported);
> + return -1;
> + }
> +
> + if (ndoms) {
> + if (virDomainObjListConvert(driver->domains, conn, doms,
> ndoms, &vms,
> + &nvms, NULL, lflags, true) <
> 0)
> + return -1;
> + } else {
> + if (virDomainObjListCollect(driver->domains, conn, &vms,
> &nvms,
> + NULL, lflags) < 0)
> + return -1;
> + }
> +
> + tmpstats = g_new0(virDomainStatsRecordPtr, nvms + 1);
> +
> + for (i = 0; i < nvms; i++) {
> + virDomainStatsRecordPtr tmp;
> + virDomainObj *vm = vms[i];
> +
> + virObjectLock(vm);
> + testDomainGetStats(conn, vm, stats, &tmp);
This function can fail and there is no way of knowing without
checking
the error code. It might look like it is fine because &tmp will be
NULL, but the function does not clear it and you did not initialise
it
here. Initialisation to sensible defaults is almost always good,
even
if you just override it later on.
> + virObjectUnlock(vm);
> +
> + if (!tmp)
> + goto cleanup;
> +
> + tmpstats[nstats++] = tmp;
And due to above reasons I get this here:
../src/test/test_driver.c: In function
‘testConnectGetAllDomainStats’:
../src/test/test_driver.c:9952:28: error: ‘tmp’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
9952 | tmpstats[nstats++] = tmp;
| ~~~~~~~~~~~~~~~~~~~^~~~~
I am compiling it with GCC 11.2.0. But it should also fail in our
CI,
which would make life easier for you, so I suggest you set up a
GitLab
account and setup a CI for your branches there. You might go the
extra
mile and set up cirrus testing as described somewhere under ci/, if
you
want.
Sorry for that, I will fix that and remember to initialise these in the
future. I just find out that when configured with `meson build -
Dbuildtype=debug`, it won't give this error message, but with `meson
build -Dbuildtype=release` will. I do have set up my GitLab account,
though sometimes I forgot to push my local branches to remote, I will
push them before I try to submit patches, so I can find out if the CI
passed or not.
I would suggest checking the return value _and_ initialising the
pointer
to NULL. And I promise I'll have a look at the series ASAP if you
rebase it to current master ;)
Thanks, I will rebase the series and apply your suggestions in v3.
Thanks,
Martin
2:10 p.m.
New subject: [PATCH v2 10/11] test_driver: Introduce testDomainGetStatsIOThread
Introduce testDomainGetStatsIOThread to add support for
testConnectGetAllDomainStats to get IOThread infos.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 44 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 5da7f23a9b..24f6062b42 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9696,6 +9696,46 @@ testDomainGetStatsState(virDomainObj *dom,
return 0;
}
+static int
+testDomainGetStatsIOThread(virDomainObj *dom,
+ virTypedParamList *params)
+{
+ testDomainObjPrivate *priv = dom->privateData;
+ size_t i;
+ int niothreads = 0;
+
+ if (!virDomainObjIsActive(dom))
+ return 0;
+
+ niothreads = priv->iothreads->len;
+
+ if (niothreads == 0) {
+ return 0;
+ }
+
+ if (virTypedParamListAddUInt(params, niothreads, "iothread.count") < 0)
+ return -1;
+
+ for (i = 0; i < niothreads; i++) {
+ testIOThreadInfo iothread = g_array_index(priv->iothreads,
+ testIOThreadInfo, i);
+ if (virTypedParamListAddULLong(params, iothread.poll_max_ns,
+ "iothread.%u.poll-max-ns",
+ iothread.iothread_id) < 0)
+ return -1;
+ if (virTypedParamListAddUInt(params, iothread.poll_grow,
+ "iothread.%u.poll-grow",
+ iothread.iothread_id) < 0)
+ return -1;
+ if (virTypedParamListAddUInt(params, iothread.poll_shrink,
+ "iothread.%u.poll-shrink",
+ iothread.iothread_id) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
typedef int
(*testDomainGetStatsFunc)(virDomainObj *dom,
virTypedParamList *list);
@@ -9707,6 +9747,7 @@ struct testDomainGetStatsWorker {
static struct testDomainGetStatsWorker testDomainGetStatsWorkers[] = {
{ testDomainGetStatsState, VIR_DOMAIN_STATS_STATE },
+ { testDomainGetStatsIOThread, VIR_DOMAIN_STATS_IOTHREAD },
{ NULL, 0 }
};
@@ -9753,7 +9794,8 @@ testConnectGetAllDomainStats(virConnectPtr conn,
VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE);
- unsigned int supported = VIR_DOMAIN_STATS_STATE;
+ unsigned int supported = VIR_DOMAIN_STATS_STATE |
+ VIR_DOMAIN_STATS_IOTHREAD;
virDomainObj **vms = NULL;
size_t nvms;
virDomainStatsRecordPtr *tmpstats = NULL;
--
2.32.0
2:10 p.m.
New subject: [PATCH v2 11/11] tests: Test IOThread related functions for test driver
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
tests/virshtest.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index fe0c420958..4b10be9c0c 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -22,6 +22,7 @@ main(void)
# define DOM_UUID "ef861801-45b9-11cb-88e3-afbfe5370493"
# define SECURITY_LABEL "libvirt-test (enforcing)"
+# define EQUAL "="
static const char *dominfo_fc4 = "\
Id: 2\n\
@@ -43,6 +44,17 @@ static const char *domuuid_fc4 = DOM_UUID "\n\n";
static const char *domid_fc4 = "2\n\n";
static const char *domname_fc4 = "fc4\n\n";
static const char *domstate_fc4 = "running\n\n";
+static const char *domstats_fc4 = "\
+Domain: 'fc4'\n\
+ state.state" EQUAL "1\n\
+ state.reason" EQUAL "0\n\
+ iothread.count" EQUAL "2\n\
+ iothread.2.poll-max-ns" EQUAL "32768\n\
+ iothread.2.poll-grow" EQUAL "0\n\
+ iothread.2.poll-shrink" EQUAL "0\n\
+ iothread.4.poll-max-ns" EQUAL "32768\n\
+ iothread.4.poll-grow" EQUAL "0\n\
+ iothread.4.poll-shrink" EQUAL "0\n\n";
static int testFilterLine(char *buffer,
const char *toRemove)
@@ -257,6 +269,60 @@ static int testCompareDomControlInfoByName(const void *data
G_GNUC_UNUSED)
return testCompareOutputLit(exp, NULL, argv);
}
+static int testIOThreadAdd(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "iothreadadd",
"--domain",
+ "fc4", "--id", "1",
NULL};
+ const char *exp = "\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
+static int testIOThreadDel(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "iothreaddel",
"--domain",
+ "fc4", "--id", "2",
NULL};
+ const char *exp = "\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
+static int testIOThreadInfo(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "iothreadinfo",
"--domain",
+ "fc4", NULL};
+ const char *exp = "\
+ IOThread ID CPU Affinity\n\
+-----------------------------\n\
+ 2 0-7\n\
+ 4 0-7\n\
+\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
+static int testIOThreadSet(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "iothreadset",
"--domain",
+ "fc4", "--id", "2",
"--poll-max-ns", "100",
+ "--poll-shrink", "10",
"--poll-grow",
+ "10", NULL};
+ const char *exp = "\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
+static int testIOThreadPin(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "iothreadpin", "fc4",
+ "--iothread", "2",
"--cpulist", "0", NULL};
+ const char *exp = "\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
+static int testDomStats(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "domstats", "fc4",
NULL };
+ const char *exp = domstats_fc4;
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
struct testInfo {
const char *const *argv;
const char *result;
@@ -345,6 +411,30 @@ mymain(void)
testCompareDomControlInfoByName, NULL) != 0)
ret = -1;
+ if (virTestRun("virsh iothreadadd",
+ testIOThreadAdd, NULL) != 0)
+ ret = -1;
+
+ if (virTestRun("virsh iothreaddel",
+ testIOThreadDel, NULL) != 0)
+ ret = -1;
+
+ if (virTestRun("virsh iothreadinfo",
+ testIOThreadInfo, NULL) != 0)
+ ret = -1;
+
+ if (virTestRun("virsh iothreadset",
+ testIOThreadSet, NULL) != 0)
+ ret = -1;
+
+ if (virTestRun("virsh iothreadpin",
+ testIOThreadPin, NULL) != 0)
+ ret = -1;
+
+ if (virTestRun("virsh domstats (by name)",
+ testDomStats, NULL) != 0)
+ ret = -1;
+
/* It's a bit awkward listing result before argument, but that's a
* limitation of C99 vararg macros. */
# define DO_TEST(i, result, ...) \
--
2.32.0
1135
days inactive
1148
days old
13 comments
2 participants
participants (2)
-
Luke Yue -
Martin Kletzander