11:19 a.m.
Ilias Stamatis (2):
test_driver: implement virDomainPMSuspendForDuration
test_driver: implement virDomainPMWakeup
src/test/test_driver.c | 116 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 116 insertions(+)
--
2.22.0
11:19 a.m.
New subject: [libvirt] [PATCH 1/2] test_driver: implement virDomainPMSuspendForDuration
Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
---
src/test/test_driver.c | 79 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 5f5c512571..d21a69c7ed 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4090,6 +4090,84 @@ static int testDomainSetMetadata(virDomainPtr dom,
return ret;
}
+
+static int
+testDomainPMSuspendForDuration(virDomainPtr dom,
+ unsigned int target,
+ unsigned long long duration,
+ unsigned int flags)
+{
+ virDomainObjPtr vm;
+ testDriverPtr privconn = dom->conn->privateData;
+ virObjectEventPtr event_suspend = NULL;
+ virObjectEventPtr event_shutdown = NULL;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (duration) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Duration not supported. Use 0 for now"));
+ return -1;
+ }
+
+ if (target >= VIR_NODE_SUSPEND_TARGET_LAST) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("Unknown suspend target: %u"),
+ target);
+ return -1;
+ }
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
+
+ if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (vm->def->pm.s3 || vm->def->pm.s4) {
+ if (vm->def->pm.s3 == VIR_TRISTATE_BOOL_NO &&
+ (target == VIR_NODE_SUSPEND_TARGET_MEM ||
+ target == VIR_NODE_SUSPEND_TARGET_HYBRID)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("S3 state is disabled for this domain"));
+ goto cleanup;
+ }
+
+ if (vm->def->pm.s4 == VIR_TRISTATE_BOOL_NO &&
+ target == VIR_NODE_SUSPEND_TARGET_DISK) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("S4 state is disabled for this domain"));
+ goto cleanup;
+ }
+ }
+
+ virDomainObjSetState(vm, VIR_DOMAIN_PMSUSPENDED,
+ VIR_DOMAIN_PMSUSPENDED_UNKNOWN);
+ event_suspend = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_PMSUSPENDED,
+ VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
+ virObjectEventStateQueue(privconn->eventState, event_suspend);
+
+ if (target == VIR_NODE_SUSPEND_TARGET_DISK) {
+ testDomainShutdownState(dom, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+ event_shutdown = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ if (!vm->persistent)
+ virDomainObjListRemove(privconn->domains, vm);
+ }
+
+ ret = 0;
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ virObjectEventStateQueue(privconn->eventState, event_shutdown);
+ return ret;
+}
+
+
#define TEST_TOTAL_CPUTIME 48772617035LL
static int
@@ -9415,6 +9493,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainSendKey = testDomainSendKey, /* 5.5.0 */
.domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */
.domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */
+ .domainPMSuspendForDuration = testDomainPMSuspendForDuration, /* 5.7.0 */
.domainGetCPUStats = testDomainGetCPUStats, /* 5.6.0 */
.domainSendProcessSignal = testDomainSendProcessSignal, /* 5.5.0 */
.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */
--
2.22.0
noon
New subject: [libvirt] [PATCH 1/2] test_driver: implement virDomainPMSuspendForDuration
On 8/13/19 1:19 PM, Ilias Stamatis wrote:
Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
---
src/test/test_driver.c | 79 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 5f5c512571..d21a69c7ed 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4090,6 +4090,84 @@ static int testDomainSetMetadata(virDomainPtr dom,
return ret;
}
+
+static int
+testDomainPMSuspendForDuration(virDomainPtr dom,
+ unsigned int target,
+ unsigned long long duration,
+ unsigned int flags)
+{
+ virDomainObjPtr vm;
+ testDriverPtr privconn = dom->conn->privateData;
+ virObjectEventPtr event_suspend = NULL;
+ virObjectEventPtr event_shutdown = NULL;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (duration) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Duration not supported. Use 0 for now"));
+ return -1;
+ }
+
+ if (target >= VIR_NODE_SUSPEND_TARGET_LAST) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("Unknown suspend target: %u"),
+ target);
+ return -1;
+ }
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
+
+ if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (vm->def->pm.s3 || vm->def->pm.s4) {
+ if (vm->def->pm.s3 == VIR_TRISTATE_BOOL_NO &&
+ (target == VIR_NODE_SUSPEND_TARGET_MEM ||
+ target == VIR_NODE_SUSPEND_TARGET_HYBRID)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("S3 state is disabled for this domain"));
+ goto cleanup;
+ }
+
+ if (vm->def->pm.s4 == VIR_TRISTATE_BOOL_NO &&
+ target == VIR_NODE_SUSPEND_TARGET_DISK) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("S4 state is disabled for this domain"));
+ goto cleanup;
+ }
+ }
+
+ virDomainObjSetState(vm, VIR_DOMAIN_PMSUSPENDED,
+ VIR_DOMAIN_PMSUSPENDED_UNKNOWN);
+ event_suspend = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_PMSUSPENDED,
+ VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
+ virObjectEventStateQueue(privconn->eventState, event_suspend);
+
+ if (target == VIR_NODE_SUSPEND_TARGET_DISK) {
+ testDomainShutdownState(dom, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+ event_shutdown = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ if (!vm->persistent)
+ virDomainObjListRemove(privconn->domains, vm);
+ }
+
+ ret = 0;
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ virObjectEventStateQueue(privconn->eventState, event_shutdown);
Unless you're OK with passing a NULL here, you need to check if
'event_shutdown' is NULL at this point, since there's no guarantee that
you set the var with something else in the 'if
target==VIR_NODE_SUSPEND_TARGET_DISK'
conditional.
I'll take a guess here and say that this is unintended, thus it's best
to move this
'virObjectEventStateQueue()' call that uses 'event_shutdown' inside the
"if"
right before the cleanup label.
Thanks,
DHB
+ return ret;
+}
+
+
#define TEST_TOTAL_CPUTIME 48772617035LL
static int
@@ -9415,6 +9493,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainSendKey = testDomainSendKey, /* 5.5.0 */
.domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */
.domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */
+ .domainPMSuspendForDuration = testDomainPMSuspendForDuration, /* 5.7.0 */
.domainGetCPUStats = testDomainGetCPUStats, /* 5.6.0 */
.domainSendProcessSignal = testDomainSendProcessSignal, /* 5.5.0 */
.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */
--
2.22.0
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
12:17 p.m.
New subject: [libvirt] [PATCH 1/2] test_driver: implement virDomainPMSuspendForDuration
On 9/3/19 2:00 PM, Daniel Henrique Barboza wrote:
On 8/13/19 1:19 PM, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
> ---
> src/test/test_driver.c | 79 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> [...]
> +
> + virDomainObjSetState(vm, VIR_DOMAIN_PMSUSPENDED,
> + VIR_DOMAIN_PMSUSPENDED_UNKNOWN);
> + event_suspend = virDomainEventLifecycleNewFromObj(vm,
> + VIR_DOMAIN_EVENT_PMSUSPENDED,
> + VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
> + virObjectEventStateQueue(privconn->eventState, event_suspend);
> +
> + if (target == VIR_NODE_SUSPEND_TARGET_DISK) {
> + testDomainShutdownState(dom, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
> + event_shutdown = virDomainEventLifecycleNewFromObj(vm,
> + VIR_DOMAIN_EVENT_STOPPED,
> + VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
> + if (!vm->persistent)
> + virDomainObjListRemove(privconn->domains, vm);
> + }
> +
> + ret = 0;
> + cleanup:
> + virDomainObjEndAPI(&vm);
> + virObjectEventStateQueue(privconn->eventState, event_shutdown);
Unless you're OK with passing a NULL here, you need to check if
'event_shutdown' is NULL at this point, since there's no guarantee that
you set the var with something else in the 'if
target==VIR_NODE_SUSPEND_TARGET_DISK'
conditional.
I'll take a guess here and say that this is unintended, thus it's best
to move this
'virObjectEventStateQueue()' call that uses 'event_shutdown' inside
the "if"
right before the cleanup label.
Just saw that patch 2/2 did something similar and decided to get more
informed.
Checking what virObjectEventStateQueue() does, I see that it's a wrapper to
virObjectEventStateQueueRemote(), which is a no-op if the second
argument - in this case, event_shutdown - is NULL. This means that the
usage above does not harm, which is good.
I'd still prefer to avoid using virObjectEventStateQueue() like you're doing
here, especially when it's easier to simply move the function call inside
the "if" in which you defined a non-NULL value to event_shutdown.
However, I see this same use of virObjectEventStateQueue() across the
board in test_driver.c. That said, I'll assume that this question was
already
dealt with in the first time this code pattern was introduced in the
code, and
this use is ok.
Rest of the code looks fine, so:
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
Thanks,
DHB
> + return ret;
> +}
> +
> +
> #define TEST_TOTAL_CPUTIME 48772617035LL
>
> static int
> @@ -9415,6 +9493,7 @@ static virHypervisorDriver testHypervisorDriver
> = {
> .domainSendKey = testDomainSendKey, /* 5.5.0 */
> .domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */
> .domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */
> + .domainPMSuspendForDuration = testDomainPMSuspendForDuration, /*
> 5.7.0 */
> .domainGetCPUStats = testDomainGetCPUStats, /* 5.6.0 */
> .domainSendProcessSignal = testDomainSendProcessSignal, /*
> 5.5.0 */
> .connectGetCPUModelNames = testConnectGetCPUModelNames, /*
> 1.1.3 */
> --
> 2.22.0
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
6:19 a.m.
New subject: [libvirt] [PATCH 1/2] test_driver: implement virDomainPMSuspendForDuration
On Tue, Sep 3, 2019 at 8:17 PM Daniel Henrique Barboza
<danielhb413(a)gmail.com> wrote:
On 9/3/19 2:00 PM, Daniel Henrique Barboza wrote:
>
>
> On 8/13/19 1:19 PM, Ilias Stamatis wrote:
>> Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
>> ---
>> src/test/test_driver.c | 79 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 79 insertions(+)
>>
>> [...]
>> +
>> + virDomainObjSetState(vm, VIR_DOMAIN_PMSUSPENDED,
>> + VIR_DOMAIN_PMSUSPENDED_UNKNOWN);
>> + event_suspend = virDomainEventLifecycleNewFromObj(vm,
>> + VIR_DOMAIN_EVENT_PMSUSPENDED,
>> + VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
>> + virObjectEventStateQueue(privconn->eventState, event_suspend);
>> +
>> + if (target == VIR_NODE_SUSPEND_TARGET_DISK) {
>> + testDomainShutdownState(dom, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
>> + event_shutdown = virDomainEventLifecycleNewFromObj(vm,
>> + VIR_DOMAIN_EVENT_STOPPED,
>> + VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
>> + if (!vm->persistent)
>> + virDomainObjListRemove(privconn->domains, vm);
>> + }
>> +
>> + ret = 0;
>> + cleanup:
>> + virDomainObjEndAPI(&vm);
>> + virObjectEventStateQueue(privconn->eventState, event_shutdown);
>
>
> Unless you're OK with passing a NULL here, you need to check if
> 'event_shutdown' is NULL at this point, since there's no guarantee that
> you set the var with something else in the 'if
> target==VIR_NODE_SUSPEND_TARGET_DISK'
> conditional.
>
> I'll take a guess here and say that this is unintended, thus it's best
> to move this
> 'virObjectEventStateQueue()' call that uses 'event_shutdown' inside
> the "if"
> right before the cleanup label.
Just saw that patch 2/2 did something similar and decided to get more
informed.
Checking what virObjectEventStateQueue() does, I see that it's a wrapper to
virObjectEventStateQueueRemote(), which is a no-op if the second
argument - in this case, event_shutdown - is NULL. This means that the
usage above does not harm, which is good.
I'd still prefer to avoid using virObjectEventStateQueue() like you're doing
here, especially when it's easier to simply move the function call inside
the "if" in which you defined a non-NULL value to event_shutdown.
However, I see this same use of virObjectEventStateQueue() across the
board in test_driver.c. That said, I'll assume that this question was
already
dealt with in the first time this code pattern was introduced in the
code, and
this use is ok.
I think not only in test_driver.c. I took a quick look and it's also
like that in some places in the QEMU driver. Plus I would say this
"pattern" is in general common in the codebase with all these free*()
functions on the cleanup sections which do the same thing - always
check for NULL first.
Additionally, if you want to check for NULL in that case you need to
do it explicitly because virDomainEventLifecycleNewFromObj might also
return NULL. This results in adding extra lines of code. Merely moving
the virObjectEventStateQueue above wouldn't change much. So, since
virObjectEventStateQueue already protects us against NULL I think
maybe it's fine to leave it like that?
In general I'm neutral to this, so feel free to alter it before merging.
Thanks for the review!
Ilias
Rest of the code looks fine, so:
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
>
>
>
> Thanks,
>
>
> DHB
>
>> + return ret;
>> +}
>> +
>> +
>> #define TEST_TOTAL_CPUTIME 48772617035LL
>>
>> static int
>> @@ -9415,6 +9493,7 @@ static virHypervisorDriver testHypervisorDriver
>> = {
>> .domainSendKey = testDomainSendKey, /* 5.5.0 */
>> .domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */
>> .domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */
>> + .domainPMSuspendForDuration = testDomainPMSuspendForDuration, /*
>> 5.7.0 */
>> .domainGetCPUStats = testDomainGetCPUStats, /* 5.6.0 */
>> .domainSendProcessSignal = testDomainSendProcessSignal, /*
>> 5.5.0 */
>> .connectGetCPUModelNames = testConnectGetCPUModelNames, /*
>> 1.1.3 */
>> --
>> 2.22.0
>>
>> --
>> libvir-list mailing list
>> libvir-list(a)redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
11:20 a.m.
New subject: [libvirt] [PATCH 2/2] test_driver: implement virDomainPMWakeup
Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
---
src/test/test_driver.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d21a69c7ed..b2a9baf361 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4168,6 +4168,42 @@ testDomainPMSuspendForDuration(virDomainPtr dom,
}
+static int
+testDomainPMWakeup(virDomainPtr dom,
+ unsigned int flags)
+{
+ virDomainObjPtr vm;
+ virObjectEventPtr event = NULL;
+ testDriverPtr privconn = dom->conn->privateData;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PMSUSPENDED) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Unable to wake up: guest is not in suspended
state"));
+ goto cleanup;
+ }
+
+ virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_WAKEUP);
+
+ event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
+ VIR_DOMAIN_EVENT_STARTED_WAKEUP);
+
+ ret = 0;
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ virObjectEventStateQueue(privconn->eventState, event);
+ return ret;
+}
+
+
#define TEST_TOTAL_CPUTIME 48772617035LL
static int
@@ -9494,6 +9530,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */
.domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */
.domainPMSuspendForDuration = testDomainPMSuspendForDuration, /* 5.7.0 */
+ .domainPMWakeup = testDomainPMWakeup, /* 5.7.0 */
.domainGetCPUStats = testDomainGetCPUStats, /* 5.6.0 */
.domainSendProcessSignal = testDomainSendProcessSignal, /* 5.5.0 */
.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */
--
2.22.0
12:20 p.m.
New subject: [libvirt] [PATCH 2/2] test_driver: implement virDomainPMWakeup
On 8/13/19 1:20 PM, Ilias Stamatis wrote:
Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
---
(My virObjectEventStateQueue rant in patch 1/2 applies here as well)
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
src/test/test_driver.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d21a69c7ed..b2a9baf361 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4168,6 +4168,42 @@ testDomainPMSuspendForDuration(virDomainPtr dom,
}
+static int
+testDomainPMWakeup(virDomainPtr dom,
+ unsigned int flags)
+{
+ virDomainObjPtr vm;
+ virObjectEventPtr event = NULL;
+ testDriverPtr privconn = dom->conn->privateData;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PMSUSPENDED) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Unable to wake up: guest is not in suspended
state"));
+ goto cleanup;
+ }
+
+ virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_WAKEUP);
+
+ event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
+ VIR_DOMAIN_EVENT_STARTED_WAKEUP);
+
+ ret = 0;
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ virObjectEventStateQueue(privconn->eventState, event);
+ return ret;
+}
+
+
#define TEST_TOTAL_CPUTIME 48772617035LL
static int
@@ -9494,6 +9530,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */
.domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */
.domainPMSuspendForDuration = testDomainPMSuspendForDuration, /* 5.7.0 */
+ .domainPMWakeup = testDomainPMWakeup, /* 5.7.0 */
.domainGetCPUStats = testDomainGetCPUStats, /* 5.6.0 */
.domainSendProcessSignal = testDomainSendProcessSignal, /* 5.5.0 */
.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */
--
2.22.0
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
1940
days inactive
1964
days old
6 comments
2 participants
participants (2)
-
Daniel Henrique Barboza -
Ilias Stamatis