6:13 a.m.
v2:
- Change to priv->jobState to store job type directly instead of using
time
- Use priv->jobOperation to store job operation for virDomainGetJobStats
Luke Yue (4):
test_driver: Implement virDomainGetJobInfo
test_driver: Implement virDomainGetJobStats
test_driver: Implement virDomainAbortJob
virshtest: add test for domjobinfo
src/test/test_driver.c | 223 +++++++++++++++++++++++++++++++++++++++++
tests/virshtest.c | 11 ++
2 files changed, 234 insertions(+)
--
2.32.0
6:13 a.m.
New subject: [PATCH v2 1/4] test_driver: Implement virDomainGetJobInfo
priv-jobState is used to store dummy job type, and priv->jobOperation is
used to store dummy job operation, they are initialized to
VIR_DOMAIN_JOB_NONE and VIR_DOMAIN_JOB_OPERATION_UNKNOWN, we can
just change them in other job related APIs as there is no real job in test
driver.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 89 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 00cc13511a..9306f0e104 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -380,6 +380,10 @@ struct _testDomainObjPrivate {
/* used by get/set time APIs */
long long seconds;
unsigned int nseconds;
+
+ /* used by Job Info APIs */
+ unsigned int jobState;
+ unsigned int jobOperation;
};
@@ -396,6 +400,9 @@ testDomainObjPrivateAlloc(void *opaque)
priv->seconds = 627319920;
priv->nseconds = 0;
+ priv->jobState = VIR_DOMAIN_JOB_NONE;
+ priv->jobOperation = VIR_DOMAIN_JOB_OPERATION_UNKNOWN;
+
return priv;
}
@@ -2681,6 +2688,87 @@ testDomainGetOSType(virDomainPtr dom G_GNUC_UNUSED)
return ret;
}
+static int
+testDomainGetJobInfoImpl(virDomainObj *dom,
+ virDomainJobInfoPtr info)
+{
+ testDomainObjPrivate *priv = dom->privateData;
+
+ memset(info, 0, sizeof(*info));
+
+ info->type = priv->jobState;
+
+ switch (priv->jobState) {
+ case VIR_DOMAIN_JOB_NONE:
+ break;
+
+ case VIR_DOMAIN_JOB_BOUNDED:
+ info->dataTotal = 30 * 1024;
+ info->dataProcessed = 10 * 1024;
+ info->dataRemaining = 20 * 1024;
+ info->timeRemaining = 6000;
+ info->timeElapsed = 4000;
+ info->memTotal = 3 * 1024 * 1024;
+ info->memProcessed = 1024 * 1024;
+ info->memRemaining = 2 * 1024 * 1024;
+ info->fileTotal = 2 * 1024 * 1024;
+ info->fileProcessed = 1024 * 1024 / 2;
+ info->fileRemaining = 3 * 1024 * 1024 / 2;
+ break;
+
+ case VIR_DOMAIN_JOB_UNBOUNDED:
+ info->dataTotal = 30 * 1024;
+ info->dataProcessed = 10 * 1024;
+ info->dataRemaining = 20 * 1024;
+ info->timeElapsed = 4000;
+ info->memTotal = 3 * 1024 * 1024;
+ info->memProcessed = 1024 * 1024;
+ info->memRemaining = 2 * 1024 * 1024;
+ info->fileTotal = 2 * 1024 * 1024;
+ info->fileProcessed = 1024 * 1024 / 2;
+ info->fileRemaining = 3 * 1024 * 1024 / 2;
+ break;
+
+ case VIR_DOMAIN_JOB_COMPLETED:
+ info->timeElapsed = 10000;
+ info->dataTotal = 30 * 1024;
+ info->dataProcessed = 30 * 1024;
+ info->memTotal = 3 * 1024 * 1024;
+ info->memProcessed = 3 * 1024 * 1024;
+ info->fileTotal = 2 * 1024 * 1024;
+ info->fileProcessed = 2 * 1024 * 1024;
+ break;
+
+ case VIR_DOMAIN_JOB_FAILED:
+ break;
+
+ case VIR_DOMAIN_JOB_CANCELLED:
+ break;
+ }
+
+ return 0;
+}
+
+static int
+testDomainGetJobInfo(virDomainPtr dom,
+ virDomainJobInfoPtr info)
+{
+ virDomainObj *vm;
+ int ret = -1;
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ ret = testDomainGetJobInfoImpl(vm, info);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
static int
testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
@@ -9583,6 +9671,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainMemoryPeek = testDomainMemoryPeek, /* 5.4.0 */
.domainGetBlockInfo = testDomainGetBlockInfo, /* 5.7.0 */
.domainSetLifecycleAction = testDomainSetLifecycleAction, /* 5.7.0 */
+ .domainGetJobInfo = testDomainGetJobInfo, /* 7.7.0 */
.domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
.domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */
--
2.32.0
6:45 a.m.
New subject: [PATCH v2 1/4] test_driver: Implement virDomainGetJobInfo
On Mon, Aug 16, 2021 at 07:13:34PM +0800, Luke Yue wrote:
priv-jobState is used to store dummy job type, and
priv->jobOperation is
used to store dummy job operation, they are initialized to
VIR_DOMAIN_JOB_NONE and VIR_DOMAIN_JOB_OPERATION_UNKNOWN, we can
just change them in other job related APIs as there is no real job in test
driver.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 89 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 00cc13511a..9306f0e104 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -380,6 +380,10 @@ struct _testDomainObjPrivate {
/* used by get/set time APIs */
long long seconds;
unsigned int nseconds;
+
+ /* used by Job Info APIs */
+ unsigned int jobState;
+ unsigned int jobOperation;
};
@@ -396,6 +400,9 @@ testDomainObjPrivateAlloc(void *opaque)
priv->seconds = 627319920;
priv->nseconds = 0;
+ priv->jobState = VIR_DOMAIN_JOB_NONE;
+ priv->jobOperation = VIR_DOMAIN_JOB_OPERATION_UNKNOWN;
+
return priv;
}
@@ -2681,6 +2688,87 @@ testDomainGetOSType(virDomainPtr dom G_GNUC_UNUSED)
return ret;
}
+static int
+testDomainGetJobInfoImpl(virDomainObj *dom,
+ virDomainJobInfoPtr info)
+{
+ testDomainObjPrivate *priv = dom->privateData;
+
+ memset(info, 0, sizeof(*info));
+
+ info->type = priv->jobState;
+
+ switch (priv->jobState) {
+ case VIR_DOMAIN_JOB_NONE:
+ break;
+
+ case VIR_DOMAIN_JOB_BOUNDED:
+ info->dataTotal = 30 * 1024;
+ info->dataProcessed = 10 * 1024;
+ info->dataRemaining = 20 * 1024;
+ info->timeRemaining = 6000;
+ info->timeElapsed = 4000;
+ info->memTotal = 3 * 1024 * 1024;
+ info->memProcessed = 1024 * 1024;
+ info->memRemaining = 2 * 1024 * 1024;
+ info->fileTotal = 2 * 1024 * 1024;
+ info->fileProcessed = 1024 * 1024 / 2;
+ info->fileRemaining = 3 * 1024 * 1024 / 2;
+ break;
+
+ case VIR_DOMAIN_JOB_UNBOUNDED:
+ info->dataTotal = 30 * 1024;
+ info->dataProcessed = 10 * 1024;
+ info->dataRemaining = 20 * 1024;
+ info->timeElapsed = 4000;
+ info->memTotal = 3 * 1024 * 1024;
+ info->memProcessed = 1024 * 1024;
+ info->memRemaining = 2 * 1024 * 1024;
+ info->fileTotal = 2 * 1024 * 1024;
+ info->fileProcessed = 1024 * 1024 / 2;
+ info->fileRemaining = 3 * 1024 * 1024 / 2;
+ break;
+
+ case VIR_DOMAIN_JOB_COMPLETED:
+ info->timeElapsed = 10000;
+ info->dataTotal = 30 * 1024;
+ info->dataProcessed = 30 * 1024;
+ info->memTotal = 3 * 1024 * 1024;
+ info->memProcessed = 3 * 1024 * 1024;
+ info->fileTotal = 2 * 1024 * 1024;
+ info->fileProcessed = 2 * 1024 * 1024;
+ break;
+
This could be cleared up with some constants and branch fallthroughs,
but that's a minor details, for now this is fine as it is.
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
6:13 a.m.
New subject: [PATCH v2 2/4] test_driver: Implement virDomainGetJobStats
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 105 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 9306f0e104..93aeec7105 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2769,6 +2769,110 @@ testDomainGetJobInfo(virDomainPtr dom,
return ret;
}
+static int
+testDomainJobInfoToParams(testDomainObjPrivate *priv,
+ virDomainJobInfoPtr info,
+ int *type,
+ virTypedParameterPtr *params,
+ int *nparams)
+{
+ virTypedParameterPtr par = NULL;
+ int maxpar = 0;
+ int npar = 0;
+
+ if (virTypedParamsAddInt(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_OPERATION,
+ priv->jobOperation) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DATA_TOTAL,
+ info->dataTotal) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DATA_PROCESSED,
+ info->dataProcessed) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DATA_REMAINING,
+ info->dataRemaining) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_TIME_ELAPSED,
+ info->timeElapsed) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_TIME_REMAINING,
+ info->timeRemaining) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_MEMORY_TOTAL,
+ info->memTotal) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_MEMORY_PROCESSED,
+ info->memProcessed) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_MEMORY_REMAINING,
+ info->memRemaining) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DISK_TOTAL,
+ info->fileTotal) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DISK_PROCESSED,
+ info->fileProcessed) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DISK_REMAINING,
+ info->fileRemaining) < 0)
+ goto error;
+
+ *type = info->type;
+ *params = par;
+ *nparams = npar;
+ return 0;
+
+ error:
+ virTypedParamsFree(par, npar);
+ return -1;
+}
+
+static int
+testDomainGetJobStats(virDomainPtr domain,
+ int *type,
+ virTypedParameterPtr *params,
+ int *nparams,
+ unsigned int flags)
+{
+ virDomainJobInfo jobInfo;
+ virDomainObj *dom;
+ testDomainObjPrivate *priv;
+ bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED);
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED |
+ VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED, -1);
+
+ if (!(dom = testDomObjFromDomain(domain)))
+ return -1;
+
+ priv = dom->privateData;
+
+ if (testDomainGetJobInfoImpl(dom, &jobInfo) < 0)
+ goto cleanup;
+
+ if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
+ *type = VIR_DOMAIN_JOB_NONE;
+ *params = NULL;
+ *nparams = 0;
+ ret = 0;
+ goto cleanup;
+ }
+
+ ret = testDomainJobInfoToParams(priv, &jobInfo, type, params, nparams);
+
+ if (completed && ret == 0 &&
+ !(flags & VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED)) {
+ priv->jobState = VIR_DOMAIN_JOB_NONE;
+ priv->jobOperation = VIR_DOMAIN_JOB_OPERATION_UNKNOWN;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&dom);
+
+ return ret;
+}
static int
testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
@@ -9672,6 +9776,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetBlockInfo = testDomainGetBlockInfo, /* 5.7.0 */
.domainSetLifecycleAction = testDomainSetLifecycleAction, /* 5.7.0 */
.domainGetJobInfo = testDomainGetJobInfo, /* 7.7.0 */
+ .domainGetJobStats = testDomainGetJobStats, /* 7.7.0 */
.domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
.domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */
--
2.32.0
6:48 a.m.
New subject: [PATCH v2 2/4] test_driver: Implement virDomainGetJobStats
On Mon, Aug 16, 2021 at 07:13:35PM +0800, Luke Yue wrote:
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 105 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 9306f0e104..93aeec7105 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2769,6 +2769,110 @@ testDomainGetJobInfo(virDomainPtr dom,
return ret;
}
+static int
+testDomainJobInfoToParams(testDomainObjPrivate *priv,
+ virDomainJobInfoPtr info,
+ int *type,
+ virTypedParameterPtr *params,
+ int *nparams)
+{
+ virTypedParameterPtr par = NULL;
+ int maxpar = 0;
+ int npar = 0;
+
+ if (virTypedParamsAddInt(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_OPERATION,
+ priv->jobOperation) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DATA_TOTAL,
+ info->dataTotal) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DATA_PROCESSED,
+ info->dataProcessed) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DATA_REMAINING,
+ info->dataRemaining) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_TIME_ELAPSED,
+ info->timeElapsed) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_TIME_REMAINING,
+ info->timeRemaining) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_MEMORY_TOTAL,
+ info->memTotal) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_MEMORY_PROCESSED,
+ info->memProcessed) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_MEMORY_REMAINING,
+ info->memRemaining) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DISK_TOTAL,
+ info->fileTotal) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DISK_PROCESSED,
+ info->fileProcessed) < 0 ||
+ virTypedParamsAddULLong(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_DISK_REMAINING,
+ info->fileRemaining) < 0)
+ goto error;
+
+ *type = info->type;
+ *params = par;
+ *nparams = npar;
+ return 0;
+
+ error:
+ virTypedParamsFree(par, npar);
+ return -1;
+}
+
+static int
+testDomainGetJobStats(virDomainPtr domain,
+ int *type,
+ virTypedParameterPtr *params,
+ int *nparams,
+ unsigned int flags)
+{
+ virDomainJobInfo jobInfo;
+ virDomainObj *dom;
+ testDomainObjPrivate *priv;
+ bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED);
Unnecessary cast, just:
bool completed = flags & VIR_DOMAIN_JOB_STATS_COMPLETED;
would be fine.
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED |
+ VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED, -1);
+
+ if (!(dom = testDomObjFromDomain(domain)))
+ return -1;
+
+ priv = dom->privateData;
+
+ if (testDomainGetJobInfoImpl(dom, &jobInfo) < 0)
+ goto cleanup;
+
Unfortunately I do not see how you return different data if you get here
with completed == true. Either we should return some dummy values or
get an extra field in priv for some last completed job.
Otherwise looks fine.
+ if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
+ *type = VIR_DOMAIN_JOB_NONE;
+ *params = NULL;
+ *nparams = 0;
+ ret = 0;
+ goto cleanup;
+ }
+
+ ret = testDomainJobInfoToParams(priv, &jobInfo, type, params, nparams);
+
+ if (completed && ret == 0 &&
+ !(flags & VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED)) {
+ priv->jobState = VIR_DOMAIN_JOB_NONE;
+ priv->jobOperation = VIR_DOMAIN_JOB_OPERATION_UNKNOWN;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&dom);
+
+ return ret;
+}
static int
testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
@@ -9672,6 +9776,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetBlockInfo = testDomainGetBlockInfo, /* 5.7.0 */
.domainSetLifecycleAction = testDomainSetLifecycleAction, /* 5.7.0 */
.domainGetJobInfo = testDomainGetJobInfo, /* 7.7.0 */
+ .domainGetJobStats = testDomainGetJobStats, /* 7.7.0 */
.domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
.domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */
--
2.32.0
7:29 a.m.
New subject: [PATCH v2 2/4] test_driver: Implement virDomainGetJobStats
On Tue, 2021-08-17 at 13:48 +0200, Martin Kletzander wrote:
On Mon, Aug 16, 2021 at 07:13:35PM +0800, Luke Yue wrote:
> Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
> ---
> src/test/test_driver.c | 105
> +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 9306f0e104..93aeec7105 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -2769,6 +2769,110 @@ testDomainGetJobInfo(virDomainPtr dom,
> return ret;
> }
>
> +static int
> +testDomainJobInfoToParams(testDomainObjPrivate *priv,
> + virDomainJobInfoPtr info,
> + int *type,
> + virTypedParameterPtr *params,
> + int *nparams)
> +{
> + virTypedParameterPtr par = NULL;
> + int maxpar = 0;
> + int npar = 0;
> +
> + if (virTypedParamsAddInt(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_OPERATION,
> + priv->jobOperation) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_DATA_TOTAL,
> + info->dataTotal) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_DATA_PROCESSED,
> + info->dataProcessed) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_DATA_REMAINING,
> + info->dataRemaining) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_TIME_ELAPSED,
> + info->timeElapsed) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_TIME_REMAINING,
> + info->timeRemaining) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_MEMORY_TOTAL,
> + info->memTotal) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_MEMORY_PROCESSED,
> + info->memProcessed) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_MEMORY_REMAINING,
> + info->memRemaining) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_DISK_TOTAL,
> + info->fileTotal) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_DISK_PROCESSED,
> + info->fileProcessed) < 0 ||
> + virTypedParamsAddULLong(&par, &npar, &maxpar,
> + VIR_DOMAIN_JOB_DISK_REMAINING,
> + info->fileRemaining) < 0)
> + goto error;
> +
> + *type = info->type;
> + *params = par;
> + *nparams = npar;
> + return 0;
> +
> + error:
> + virTypedParamsFree(par, npar);
> + return -1;
> +}
> +
> +static int
> +testDomainGetJobStats(virDomainPtr domain,
> + int *type,
> + virTypedParameterPtr *params,
> + int *nparams,
> + unsigned int flags)
> +{
> + virDomainJobInfo jobInfo;
> + virDomainObj *dom;
> + testDomainObjPrivate *priv;
> + bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED);
Unnecessary cast, just:
bool completed = flags & VIR_DOMAIN_JOB_STATS_COMPLETED;
would be fine.
> + int ret = -1;
> +
> + virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED |
> + VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED, -1);
> +
> + if (!(dom = testDomObjFromDomain(domain)))
> + return -1;
> +
> + priv = dom->privateData;
> +
> + if (testDomainGetJobInfoImpl(dom, &jobInfo) < 0)
> + goto cleanup;
> +
Unfortunately I do not see how you return different data if you get
here
with completed == true. Either we should return some dummy values or
get an extra field in priv for some last completed job.
Otherwise looks fine.
Thanks for the review! I thought we could use priv->jobOperation as
both complete job or running job for testing, though it may not be
pretty reasonable. Will try to add another variable to store the
completed job in v3.
> + if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
> + *type = VIR_DOMAIN_JOB_NONE;
> + *params = NULL;
> + *nparams = 0;
> + ret = 0;
> + goto cleanup;
> + }
> +
> + ret = testDomainJobInfoToParams(priv, &jobInfo, type, params,
> nparams);
> +
> + if (completed && ret == 0 &&
> + !(flags & VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED)) {
> + priv->jobState = VIR_DOMAIN_JOB_NONE;
> + priv->jobOperation = VIR_DOMAIN_JOB_OPERATION_UNKNOWN;
> + }
> +
> + cleanup:
> + virDomainObjEndAPI(&dom);
> +
> + return ret;
> +}
>
> static int
> testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
> @@ -9672,6 +9776,7 @@ static virHypervisorDriver
> testHypervisorDriver = {
> .domainGetBlockInfo = testDomainGetBlockInfo, /* 5.7.0 */
> .domainSetLifecycleAction = testDomainSetLifecycleAction, /*
> 5.7.0 */
> .domainGetJobInfo = testDomainGetJobInfo, /* 7.7.0 */
> + .domainGetJobStats = testDomainGetJobStats, /* 7.7.0 */
>
> .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
> .domainSnapshotListNames = testDomainSnapshotListNames, /*
> 1.1.4 */
> --
> 2.32.0
>
6:13 a.m.
New subject: [PATCH v2 3/4] test_driver: Implement virDomainAbortJob
As we are using jobState to store dummy job type, so just change it to
VIR_DOMAIN_JOB_CANCELLED when try to abort a job.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 93aeec7105..5043c57fe2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2874,6 +2874,34 @@ testDomainGetJobStats(virDomainPtr domain,
return ret;
}
+static int
+testDomainAbortJob(virDomainPtr dom)
+{
+ virDomainObj *vm;
+ int ret = -1;
+ testDomainObjPrivate *priv;
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ if (priv->jobState == VIR_DOMAIN_JOB_NONE) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("no job is active on the domain"));
+ } else {
+ priv->jobState = VIR_DOMAIN_JOB_CANCELLED;
+ ret = 0;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
static int
testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
virTypedParameterPtr *params G_GNUC_UNUSED,
@@ -9777,6 +9805,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainSetLifecycleAction = testDomainSetLifecycleAction, /* 5.7.0 */
.domainGetJobInfo = testDomainGetJobInfo, /* 7.7.0 */
.domainGetJobStats = testDomainGetJobStats, /* 7.7.0 */
+ .domainAbortJob = testDomainAbortJob, /* 7.7.0 */
.domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
.domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */
--
2.32.0
6:49 a.m.
New subject: [PATCH v2 3/4] test_driver: Implement virDomainAbortJob
On Mon, Aug 16, 2021 at 07:13:36PM +0800, Luke Yue wrote:
As we are using jobState to store dummy job type, so just change it
to
VIR_DOMAIN_JOB_CANCELLED when try to abort a job.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
6:13 a.m.
New subject: [PATCH v2 4/4] virshtest: add test for domjobinfo
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
tests/virshtest.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 53db2aa19a..0c46496c74 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -291,6 +291,13 @@ static int testCompareDomControlInfoByName(const void *data
G_GNUC_UNUSED)
return testCompareOutputLit(exp, NULL, argv);
}
+static int testCompareDomJobInfoByName(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "domjobinfo", "fc4",
NULL };
+ const char *exp = "Job type: None \n\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
struct testInfo {
const char *const *argv;
const char *result;
@@ -383,6 +390,10 @@ mymain(void)
testCompareDomControlInfoByName, NULL) != 0)
ret = -1;
+ if (virTestRun("virsh domjobinfo (by name)",
+ testCompareDomJobInfoByName, 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
6:52 a.m.
New subject: [PATCH v2 4/4] virshtest: add test for domjobinfo
On Mon, Aug 16, 2021 at 07:13:37PM +0800, Luke Yue wrote:
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
tests/virshtest.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 53db2aa19a..0c46496c74 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -291,6 +291,13 @@ static int testCompareDomControlInfoByName(const void *data
G_GNUC_UNUSED)
return testCompareOutputLit(exp, NULL, argv);
}
+static int testCompareDomJobInfoByName(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "domjobinfo", "fc4",
NULL };
You should be able to do something like:
argv[] = { VIRSH_CUSTOM, "domjobinfo fc4; domjobabort fc4; domjobinfo fc4",
NULL };
to test the behaviour of aborting and similarly for --completed and
--keep-completed.
Maybe you can also pick one VM name that gets initialised with a dummy
job. You can base it on a name or uuid or some other random data for
now.
+ const char *exp = "Job type: None
\n\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
struct testInfo {
const char *const *argv;
const char *result;
@@ -383,6 +390,10 @@ mymain(void)
testCompareDomControlInfoByName, NULL) != 0)
ret = -1;
+ if (virTestRun("virsh domjobinfo (by name)",
+ testCompareDomJobInfoByName, 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
7:34 a.m.
New subject: [PATCH v2 4/4] virshtest: add test for domjobinfo
On Tue, 2021-08-17 at 13:52 +0200, Martin Kletzander wrote:
On Mon, Aug 16, 2021 at 07:13:37PM +0800, Luke Yue wrote:
> Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
> ---
> tests/virshtest.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tests/virshtest.c b/tests/virshtest.c
> index 53db2aa19a..0c46496c74 100644
> --- a/tests/virshtest.c
> +++ b/tests/virshtest.c
> @@ -291,6 +291,13 @@ static int
> testCompareDomControlInfoByName(const void *data G_GNUC_UNUSED)
> return testCompareOutputLit(exp, NULL, argv);
> }
>
> +static int testCompareDomJobInfoByName(const void *data
> G_GNUC_UNUSED)
> +{
> + const char *const argv[] = { VIRSH_CUSTOM, "domjobinfo",
> "fc4", NULL };
You should be able to do something like:
argv[] = { VIRSH_CUSTOM, "domjobinfo fc4; domjobabort fc4;
domjobinfo fc4", NULL };
to test the behaviour of aborting and similarly for --completed and
--keep-completed.
Thanks for telling me that! I was wondering how I can keep the state of
the domain in virshtest, just like using interactive shell, and this
answered my question. Will do these in v3 and other unmerged patches'
tests can be improved with this, too. Thanks!
Maybe you can also pick one VM name that gets initialised with a
dummy
job. You can base it on a name or uuid or some other random data for
now.
> + const char *exp = "Job type: None \n\n";
> + return testCompareOutputLit(exp, NULL, argv);
> +}
> +
> struct testInfo {
> const char *const *argv;
> const char *result;
> @@ -383,6 +390,10 @@ mymain(void)
> testCompareDomControlInfoByName, NULL) != 0)
> ret = -1;
>
> + if (virTestRun("virsh domjobinfo (by name)",
> + testCompareDomJobInfoByName, 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
>
1183
days inactive
1184
days old
10 comments
2 participants
participants (2)
-
Luke Yue -
Martin Kletzander