9:10 p.m.
In qemuDomainRevertToSnapshot(), it will check snap->def->state.
But when the state is PMSUSPENDED/NOSTATE/BLOCKED, it forgets to
call qemuDomainObjEndJob.
Signed-off-by: Jincheng Miao <jmiao(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f5b17ba..2db507f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14294,7 +14294,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("qemu doesn't support reversion of snapshot taken in
"
"PMSUSPENDED state"));
- goto cleanup;
+ goto endjob;
case VIR_DOMAIN_NOSTATE:
case VIR_DOMAIN_BLOCKED:
@@ -14303,7 +14303,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
_("Invalid target domain state '%s'. Refusing "
"snapshot reversion"),
virDomainStateTypeToString(snap->def->state));
- goto cleanup;
+ goto endjob;
}
ret = 0;
--
1.7.1
10:02 p.m.
New subject: [libvirt] [PATCH] qemu: call endjob in RevertToSnapshot
On 08/26/2014 08:10 PM, Jincheng Miao wrote:
In qemuDomainRevertToSnapshot(), it will check
snap->def->state.
But when the state is PMSUSPENDED/NOSTATE/BLOCKED, it forgets to
call qemuDomainObjEndJob.
Signed-off-by: Jincheng Miao <jmiao(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
This looks like the fix for
https://bugzilla.redhat.com/show_bug.cgi?id=1134154 - thanks for chasing
that!
ACK, and I'll push soon.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:20 a.m.
New subject: [libvirt] [PATCH] qemu: call endjob in RevertToSnapshot
On 08/27/2014 11:02 AM, Eric Blake wrote:
On 08/26/2014 08:10 PM, Jincheng Miao wrote:
> In qemuDomainRevertToSnapshot(), it will check snap->def->state.
> But when the state is PMSUSPENDED/NOSTATE/BLOCKED, it forgets to
> call qemuDomainObjEndJob.
>
> Signed-off-by: Jincheng Miao <jmiao(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
This looks like the fix for
https://bugzilla.redhat.com/show_bug.cgi?id=1134154 - thanks for chasing
that!
This patch fix the part of that bug, but it doesn't touch the race
condition:
Why the snap->def->state is NOSTATE? It should be RUNNING.
The only reasonable explanation is that snapshot object is free()ed.
IMHO, the SnapshotObjList should be clocked during snapshot job running.
ACK, and I'll push soon.
6:52 a.m.
New subject: [libvirt] [PATCH] qemu: call endjob in RevertToSnapshot
On 08/27/2014 03:20 AM, Jincheng Miao wrote:
On 08/27/2014 11:02 AM, Eric Blake wrote:
> On 08/26/2014 08:10 PM, Jincheng Miao wrote:
>> In qemuDomainRevertToSnapshot(), it will check snap->def->state.
>> But when the state is PMSUSPENDED/NOSTATE/BLOCKED, it forgets to
>> call qemuDomainObjEndJob.
>>
>> Signed-off-by: Jincheng Miao <jmiao(a)redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
> This looks like the fix for
> https://bugzilla.redhat.com/show_bug.cgi?id=1134154 - thanks for chasing
> that!
This patch fix the part of that bug, but it doesn't touch the race
condition:
Why the snap->def->state is NOSTATE? It should be RUNNING.
The only reasonable explanation is that snapshot object is free()ed.
IMHO, the SnapshotObjList should be clocked during snapshot job running.
Okay, I see what you are saying; we need to make sure two parallel
snapshot operations can't stomp on one another (probably by making them
all guarded by the vm job lock). I hope to have time to look into a fix
before 1.2.8; but as it has been a long-standing issue, it's not a new
regression in this release if I miss that goal (my priority today is
getting 1.2.8 APIs to be feature-complete).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:09 p.m.
New subject: [libvirt] [PATCH] qemu: call endjob in RevertToSnapshot
On 08/27/2014 07:52 PM, Eric Blake wrote:
On 08/27/2014 03:20 AM, Jincheng Miao wrote:
> On 08/27/2014 11:02 AM, Eric Blake wrote:
>> On 08/26/2014 08:10 PM, Jincheng Miao wrote:
>>> In qemuDomainRevertToSnapshot(), it will check snap->def->state.
>>> But when the state is PMSUSPENDED/NOSTATE/BLOCKED, it forgets to
>>> call qemuDomainObjEndJob.
>>>
>>> Signed-off-by: Jincheng Miao <jmiao(a)redhat.com>
>>> ---
>>> src/qemu/qemu_driver.c | 4 ++--
>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>> This looks like the fix for
>> https://bugzilla.redhat.com/show_bug.cgi?id=1134154 - thanks for chasing
>> that!
> This patch fix the part of that bug, but it doesn't touch the race
> condition:
> Why the snap->def->state is NOSTATE? It should be RUNNING.
> The only reasonable explanation is that snapshot object is free()ed.
>
> IMHO, the SnapshotObjList should be clocked during snapshot job running.
Okay, I see what you are saying; we need to make sure two parallel
snapshot operations can't stomp on one another (probably by making them
all guarded by the vm job lock).
Yes, wrapper the snapshot operations by vm job lock good choice,
and I will check some other functions, and compose a patch for it.
I hope to have time to look into a fix
before 1.2.8; but as it has been a long-standing issue, it's not a new
regression in this release if I miss that goal (my priority today is
getting 1.2.8 APIs to be feature-complete).
9:11 p.m.
New subject: [libvirt] [PATCH] qemu: call endjob in RevertToSnapshot
On 08/28/2014 10:09 AM, Jincheng Miao wrote:
On 08/27/2014 07:52 PM, Eric Blake wrote:
> On 08/27/2014 03:20 AM, Jincheng Miao wrote:
>> On 08/27/2014 11:02 AM, Eric Blake wrote:
>>> On 08/26/2014 08:10 PM, Jincheng Miao wrote:
>>>> In qemuDomainRevertToSnapshot(), it will check snap->def->state.
>>>> But when the state is PMSUSPENDED/NOSTATE/BLOCKED, it forgets to
>>>> call qemuDomainObjEndJob.
>>>>
>>>> Signed-off-by: Jincheng Miao <jmiao(a)redhat.com>
>>>> ---
>>>> src/qemu/qemu_driver.c | 4 ++--
>>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>> This looks like the fix for
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1134154 - thanks for
>>> chasing
>>> that!
>> This patch fix the part of that bug, but it doesn't touch the race
>> condition:
>> Why the snap->def->state is NOSTATE? It should be RUNNING.
>> The only reasonable explanation is that snapshot object is free()ed.
>>
>> IMHO, the SnapshotObjList should be clocked during snapshot job
>> running.
> Okay, I see what you are saying; we need to make sure two parallel
> snapshot operations can't stomp on one another (probably by making them
> all guarded by the vm job lock).
Yes, wrapper the snapshot operations by vm job lock good choice,
s/good choice/is a good choice/
and I will check some other functions, and compose a patch for it.
> I hope to have time to look into a fix
> before 1.2.8; but as it has been a long-standing issue, it's not a new
> regression in this release if I miss that goal (my priority today is
> getting 1.2.8 APIs to be feature-complete).
>
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
3739
days inactive
3740
days old
5 comments
2 participants
participants (2)
-
Eric Blake -
Jincheng Miao