Change in vdsm[master]: Try to destroy VM gracefully

[Igor Lvovsky]

Igor Lvovsky has uploaded a new change for review.

Change subject: Try to destroy VM gracefully
......................................................................

Try to destroy VM gracefully

Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
---
M vdsm/libvirtvm.py
1 file changed, 12 insertions(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/2138/1
--
To view, visit http://gerrit.ovirt.org/2138
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky

Adam Litke has posted comments on this change.

Change subject: Try to destroy VM gracefully
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

Please address my comment regarding the 30 second in-line sleep.

....................................................
File vdsm/libvirtvm.py
Line 1864:                             time.sleep(30)
I wonder if you should schedule an asyncTask to wait and forcibly destroy the VM.  Sleeping in-line just seems wrong to me.

--
To view, visit http://gerrit.ovirt.org/2138
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky 
Gerrit-Reviewer: Adam Litke

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Try to destroy VM gracefully
......................................................................

Patch Set 1: Looks good to me, but someone else must approve

(1 inline comment)

....................................................
File vdsm/libvirtvm.py
Line 1864:                             time.sleep(30)
I believe that Engine would not be able to accommodate such a change in semantics. It expects the Vm to be dead when destroy() returns successfully.

Sleeping for a hardcoded 30 seconds is ugly (but less dangerous than the 2 seconds' sleep virDomainDestory(UNGRACEFULL) gives us.

--
To view, visit http://gerrit.ovirt.org/2138
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg

Adam Litke has posted comments on this change.

Change subject: Try to destroy VM gracefully
......................................................................

Patch Set 1: No score

(1 inline comment)

Removing my -1 since danken pointed out that it's a compatibility issue at this point.

....................................................
File vdsm/libvirtvm.py
Line 1864:                             time.sleep(30)
Hmm.  Ok.  Also since it's in an error flow I guess it is not as big of a deal either.

--
To view, visit http://gerrit.ovirt.org/2138
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli

Rami Vaknin has posted comments on this change.

Change subject: Try to destroy VM gracefully
......................................................................

Patch Set 2: Verified

--
To view, visit http://gerrit.ovirt.org/2138
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Rami Vaknin

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Try to destroy VM gracefully
......................................................................

Patch Set 2: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/2138
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Rami Vaknin

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Try to destroy VM gracefully
......................................................................

Try to destroy VM gracefully

Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
---
M vdsm/libvirtvm.py
1 file changed, 8 insertions(+), 1 deletion(-)

Approvals:
  Dan Kenigsberg: Looks good to me, approved
  Rami Vaknin: Verified

--
To view, visit http://gerrit.ovirt.org/2138
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If58f1b0e7208242aef0b81d9ea59d05bd3d5d99e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Rami Vaknin