Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-12 Thread Eric Blake
On 03/12/2012 08:07 AM, Paolo Bonzini wrote:
> Il 12/03/2012 14:58, Eric Blake ha scritto:
 If not, perhaps a
>> different state should be added so that "virsh resume" can look at the
>> state and issue the appropriate monitor command (or alternatively, it
>> could be considered a QEMU bug).

 We have the paused reason. And the patch which should be
 squashed in in the attachment.

 But I'm interested in trying if "virsh resume" wakeup a
 domain pasued by SUSPEND event.
>> It won't.  To wake up a guest in S3 suspension, you need the pmwakeup
>> command, or else a timer or input device wakeup trigger.  'virsh resume'
>> only wakes up a paused guest, not a power-management suspended guest.
> 
> Also considering the autowakeup, I think PAUSED is the wrong state for a
> pm-suspended domain.  It should just stay RUNNING.

Either it stays RUNNING (because we add no new states), or we add a new
state (perhaps PMSUSPEND), and leave the new state when we receive the
event telling us that an autowakeup occurred.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-12 Thread Paolo Bonzini
Il 12/03/2012 14:58, Eric Blake ha scritto:
>> > If not, perhaps a
>>> >> different state should be added so that "virsh resume" can look at the
>>> >> state and issue the appropriate monitor command (or alternatively, it
>>> >> could be considered a QEMU bug).
>> > 
>> > We have the paused reason. And the patch which should be
>> > squashed in in the attachment.
>> > 
>> > But I'm interested in trying if "virsh resume" wakeup a
>> > domain pasued by SUSPEND event.
> It won't.  To wake up a guest in S3 suspension, you need the pmwakeup
> command, or else a timer or input device wakeup trigger.  'virsh resume'
> only wakes up a paused guest, not a power-management suspended guest.

Also considering the autowakeup, I think PAUSED is the wrong state for a
pm-suspended domain.  It should just stay RUNNING.

Paolo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-12 Thread Eric Blake
On 03/11/2012 11:13 PM, Osier Yang wrote:
> On 03/11/2012 10:37 PM, Paolo Bonzini wrote:
>> Il 05/03/2012 11:25, Osier Yang ha scritto:
>>> This introduces a new paused reason VIR_DOMAIN_PAUSED_SUSPEND,
>>> and new suspend event type VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
>>>
>>> While a SUSPEND event occurs, the running domain status will be
>>> transferred to "paused" with reason "VIR_DOMAIN_PAUSED_SUSPEND",
>>> and a new domain lifecycle event emitted with type
>>> VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
>>> ---
>>
>> Does "virsh resume" correctly wakeup such a domain?
> 
> Ah, yes, I prohibited the situation waking up a domain which
> wasn't paused by SUSPEND event. But here I forgot it.
> 
> If not, perhaps a
>> different state should be added so that "virsh resume" can look at the
>> state and issue the appropriate monitor command (or alternatively, it
>> could be considered a QEMU bug).
> 
> We have the paused reason. And the patch which should be
> squashed in in the attachment.
> 
> But I'm interested in trying if "virsh resume" wakeup a
> domain pasued by SUSPEND event.

It won't.  To wake up a guest in S3 suspension, you need the pmwakeup
command, or else a timer or input device wakeup trigger.  'virsh resume'
only wakes up a paused guest, not a power-management suspended guest.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-12 Thread Eric Blake
On 03/05/2012 03:25 AM, Osier Yang wrote:
> This introduces a new paused reason VIR_DOMAIN_PAUSED_SUSPEND,
> and new suspend event type VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
> 
> While a SUSPEND event occurs, the running domain status will be
> transferred to "paused" with reason "VIR_DOMAIN_PAUSED_SUSPEND",
> and a new domain lifecycle event emitted with type
> VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.

NACK.

VIR_DOMAIN_EVENT_SUSPENDED_* is reserved for events where the domain is
in a paused state, due to virDomainSuspend.

We need a _new_ state (perhaps named pmsuspend), along with a new prefix
for events that move us into this new state (so far, we know of
guest-agent S3 requests that move us into this state, and pmwakeup that
moves us out of this state), since the state is very much distinct from
libvirt's notion of suspended (which really means paused).

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-12 Thread Paolo Bonzini
Il 12/03/2012 10:46, Osier Yang ha scritto:
>> Then there would have been no need for
>> "virsh dompmwakeup" at all.
>>
>> There is still time to change it, but it needs to be done quite soon.
>>
> 
> No luck, the patches for dompmsuspend were submitted Jan 26 (7c74176),
> we already have a realease with it (0.9.10: Feb 13 2012).

Yeah, but not for dompmwakeup right?

Paolo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-12 Thread Osier Yang

On 03/12/2012 04:35 PM, Paolo Bonzini wrote:

Il 12/03/2012 06:13, Osier Yang ha scritto:

On 03/11/2012 10:37 PM, Paolo Bonzini wrote:

Il 05/03/2012 11:25, Osier Yang ha scritto:

This introduces a new paused reason VIR_DOMAIN_PAUSED_SUSPEND,
and new suspend event type VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.

While a SUSPEND event occurs, the running domain status will be
transferred to "paused" with reason "VIR_DOMAIN_PAUSED_SUSPEND",
and a new domain lifecycle event emitted with type
VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
---


Does "virsh resume" correctly wakeup such a domain?


Ah, yes, I prohibited the situation waking up a domain which
wasn't paused by SUSPEND event. But here I forgot it.

If not, perhaps a

different state should be added so that "virsh resume" can look at the
state and issue the appropriate monitor command (or alternatively, it
could be considered a QEMU bug).


We have the paused reason. And the patch which should be
squashed in in the attachment.


 From the patch:


+qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("domain paused from guest side can't be resumed, 
"
+  "you might want to wakeup it"));
  goto endjob;


This is not an internal error.  It should have a precise error code so
that for example virsh can refer the user to dompmwakeup.


Yes, it was just a demo, I won't put it in the real code in the end, :-)



Personally I think that using just "virsh suspend"/"virsh resume" would
have been a fine user interface if you use the PAUSED state (with "virsh
suspend --pm" for example).  Then there would have been no need for
"virsh dompmwakeup" at all.

There is still time to change it, but it needs to be done quite soon.



No luck, the patches for dompmsuspend were submitted Jan 26 (7c74176),
we already have a realease with it (0.9.10: Feb 13 2012).



BTW, another thing to test is what happens if you send "virsh suspend"
after putting the guest into S3.



The guest behaves no difference in this case, but I'm thinking if we
should prohibit a guest which was already put into s3 from guest side
too, in case of the confusion. Yeah, I think we need to do that.

Osier



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-12 Thread Paolo Bonzini
Il 12/03/2012 06:13, Osier Yang ha scritto:
> On 03/11/2012 10:37 PM, Paolo Bonzini wrote:
>> Il 05/03/2012 11:25, Osier Yang ha scritto:
>>> This introduces a new paused reason VIR_DOMAIN_PAUSED_SUSPEND,
>>> and new suspend event type VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
>>>
>>> While a SUSPEND event occurs, the running domain status will be
>>> transferred to "paused" with reason "VIR_DOMAIN_PAUSED_SUSPEND",
>>> and a new domain lifecycle event emitted with type
>>> VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
>>> ---
>>
>> Does "virsh resume" correctly wakeup such a domain?
> 
> Ah, yes, I prohibited the situation waking up a domain which
> wasn't paused by SUSPEND event. But here I forgot it.
> 
> If not, perhaps a
>> different state should be added so that "virsh resume" can look at the
>> state and issue the appropriate monitor command (or alternatively, it
>> could be considered a QEMU bug).
> 
> We have the paused reason. And the patch which should be
> squashed in in the attachment.

>From the patch:

> +qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +_("domain paused from guest side can't be 
> resumed, "
> +  "you might want to wakeup it"));
>  goto endjob;

This is not an internal error.  It should have a precise error code so
that for example virsh can refer the user to dompmwakeup.

Personally I think that using just "virsh suspend"/"virsh resume" would
have been a fine user interface if you use the PAUSED state (with "virsh
suspend --pm" for example).  Then there would have been no need for
"virsh dompmwakeup" at all.

There is still time to change it, but it needs to be done quite soon.

BTW, another thing to test is what happens if you send "virsh suspend"
after putting the guest into S3.

Paolo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-12 Thread Osier Yang

On 03/12/2012 01:13 PM, Osier Yang wrote:

On 03/11/2012 10:37 PM, Paolo Bonzini wrote:

Il 05/03/2012 11:25, Osier Yang ha scritto:

This introduces a new paused reason VIR_DOMAIN_PAUSED_SUSPEND,
and new suspend event type VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.

While a SUSPEND event occurs, the running domain status will be
transferred to "paused" with reason "VIR_DOMAIN_PAUSED_SUSPEND",
and a new domain lifecycle event emitted with type
VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
---


Does "virsh resume" correctly wakeup such a domain?


Ah, yes, I prohibited the situation waking up a domain which
wasn't paused by SUSPEND event. But here I forgot it.

If not, perhaps a

different state should be added so that "virsh resume" can look at the
state and issue the appropriate monitor command (or alternatively, it
could be considered a QEMU bug).


We have the paused reason. And the patch which should be
squashed in in the attachment.

But I'm interested in trying if "virsh resume" wakeup a
domain pasued by SUSPEND event.


Tried, and the guest paused by "pm-suspend" inside guest can't
be waken up successfully using "virsh resume". (the guest actually
is still paused, but "virsh resume" will change the domain status
into "running"). That's not what we want.

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-11 Thread Osier Yang

On 03/11/2012 10:37 PM, Paolo Bonzini wrote:

Il 05/03/2012 11:25, Osier Yang ha scritto:

This introduces a new paused reason VIR_DOMAIN_PAUSED_SUSPEND,
and new suspend event type VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.

While a SUSPEND event occurs, the running domain status will be
transferred to "paused" with reason "VIR_DOMAIN_PAUSED_SUSPEND",
and a new domain lifecycle event emitted with type
VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
---


Does "virsh resume" correctly wakeup such a domain?


Ah, yes, I prohibited the situation waking up a domain which
wasn't paused by SUSPEND event. But here I forgot it.

If not, perhaps a

different state should be added so that "virsh resume" can look at the
state and issue the appropriate monitor command (or alternatively, it
could be considered a QEMU bug).


We have the paused reason. And the patch which should be
squashed in in the attachment.

But I'm interested in trying if "virsh resume" wakeup a
domain pasued by SUSPEND event.

Osier
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7b6e747..594c774 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1480,6 +1480,7 @@ static int qemudDomainResume(virDomainPtr dom) {
 virDomainObjPtr vm;
 int ret = -1;
 virDomainEventPtr event = NULL;
+int reason;
 
 qemuDriverLock(driver);
 vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -1500,18 +1501,25 @@ static int qemudDomainResume(virDomainPtr dom) {
 "%s", _("domain is not running"));
 goto endjob;
 }
-if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
-if (qemuProcessStartCPUs(driver, vm, dom->conn,
- VIR_DOMAIN_RUNNING_UNPAUSED,
- QEMU_ASYNC_JOB_NONE) < 0) {
-if (virGetLastError() == NULL)
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-"%s", _("resume operation failed"));
+if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED) {
+if (reason != VIR_DOMAIN_PAUSED_SUSPEND) {
+if (qemuProcessStartCPUs(driver, vm, dom->conn,
+ VIR_DOMAIN_RUNNING_UNPAUSED,
+ QEMU_ASYNC_JOB_NONE) < 0) {
+if (virGetLastError() == NULL)
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+"%s", _("resume operation failed"));
+goto endjob;
+}
+event = virDomainEventNewFromObj(vm,
+ VIR_DOMAIN_EVENT_RESUMED,
+ 
VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
+} else {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("domain paused from guest side can't be resumed, 
"
+  "you might want to wakeup it"));
 goto endjob;
 }
-event = virDomainEventNewFromObj(vm,
- VIR_DOMAIN_EVENT_RESUMED,
- VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
 }
 if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
 goto endjob;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-11 Thread Paolo Bonzini
Il 05/03/2012 11:25, Osier Yang ha scritto:
> This introduces a new paused reason VIR_DOMAIN_PAUSED_SUSPEND,
> and new suspend event type VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
> 
> While a SUSPEND event occurs, the running domain status will be
> transferred to "paused" with reason "VIR_DOMAIN_PAUSED_SUSPEND",
> and a new domain lifecycle event emitted with type
> VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
> ---

Does "virsh resume" correctly wakeup such a domain?  If not, perhaps a
different state should be added so that "virsh resume" can look at the
state and issue the appropriate monitor command (or alternatively, it
could be considered a QEMU bug).

Paolo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 8/9] qemu: Update domain status to paused while suspend event is emitted

2012-03-05 Thread Osier Yang
This introduces a new paused reason VIR_DOMAIN_PAUSED_SUSPEND,
and new suspend event type VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.

While a SUSPEND event occurs, the running domain status will be
transferred to "paused" with reason "VIR_DOMAIN_PAUSED_SUSPEND",
and a new domain lifecycle event emitted with type
VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND.
---
 examples/domain-events/events-c/event-test.c |3 +++
 include/libvirt/libvirt.h.in |2 ++
 src/qemu/qemu_process.c  |   23 +++
 tools/virsh.c|2 ++
 4 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/examples/domain-events/events-c/event-test.c 
b/examples/domain-events/events-c/event-test.c
index 1629358..9231604 100644
--- a/examples/domain-events/events-c/event-test.c
+++ b/examples/domain-events/events-c/event-test.c
@@ -115,6 +115,9 @@ static const char *eventDetailToString(int event, int 
detail) {
 case VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT:
 ret = "Snapshot";
 break;
+case VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND:
+ret = "Event suspend";
+break;
 }
 break;
 case VIR_DOMAIN_EVENT_RESUMED:
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index aa02bcb..11b7388 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -145,6 +145,7 @@ typedef enum {
 VIR_DOMAIN_PAUSED_WATCHDOG = 6, /* paused due to a watchdog event */
 VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* paused after restoring from 
snapshot */
 VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */
+VIR_DOMAIN_PAUSED_SUSPEND = 9,   /* paused due to a suspend event */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_PAUSED_LAST
@@ -2682,6 +2683,7 @@ typedef enum {
 VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG = 3,  /* Suspended due to a watchdog 
firing */
 VIR_DOMAIN_EVENT_SUSPENDED_RESTORED = 4,  /* Restored from paused state 
file */
 VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT = 5, /* Restored from paused 
snapshot */
+VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND = 6, /* Suspended due to a suspend 
event */
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_EVENT_SUSPENDED_LAST
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2cff63e..19e6e03 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1085,10 +1085,33 @@ qemuProcessHandleSuspend(qemuMonitorPtr mon 
ATTRIBUTE_UNUSED,
 {
 struct qemud_driver *driver = qemu_driver;
 virDomainEventPtr event = NULL;
+virDomainEventPtr lifecycleEvent = NULL;
 
 virDomainObjLock(vm);
 event = virDomainEventSuspendNewFromObj(vm);
 
+if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
+qemuDomainObjPrivatePtr priv = vm->privateData;
+VIR_DEBUG("Transitioned guest %s to paused state due to "
+  "QMP suspend event", vm->def->name);
+
+virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
+ VIR_DOMAIN_PAUSED_SUSPEND);
+lifecycleEvent = virDomainEventNewFromObj(vm,
+  VIR_DOMAIN_EVENT_SUSPENDED,
+  
VIR_DOMAIN_EVENT_SUSPENDED_SUSPEND);
+
+VIR_FREE(priv->lockState);
+if (virDomainLockProcessPause(driver->lockManager, vm, 
&priv->lockState) < 0)
+VIR_WARN("Unable to release lease on %s", vm->def->name);
+VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
+
+if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) {
+VIR_WARN("Unable to save status on vm %s after suspend event",
+ vm->def->name);
+}
+}
+
 virDomainObjUnlock(vm);
 
 if (event) {
diff --git a/tools/virsh.c b/tools/virsh.c
index aef050f..9063131 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -18628,6 +18628,8 @@ vshDomainStateReasonToString(int state, int reason)
 return N_("from snapshot");
 case VIR_DOMAIN_PAUSED_SHUTTING_DOWN:
 return N_("shutting down");
+case VIR_DOMAIN_PAUSED_SUSPEND:
+return N_("Event suspend");
 case VIR_DOMAIN_PAUSED_UNKNOWN:
 case VIR_DOMAIN_PAUSED_LAST:
 ;
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list