10:15 p.m.
Greg Padgett has uploaded a new change for review.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
vm: Fix exception when shutdown() called on vm in 'Down' state
Attempting to call the shutdown() method on a VM which is in 'Down'
state resulted in an exception if called through xmlrpclib. Instead,
adopt the convention used by similar APIs such as migrate() and return
errCode['noVM'].
Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Bug-Url: https://bugzilla.redhat.com/1027297
Signed-off-by: Greg Padgett <gpadgett(a)redhat.com>
---
M vdsm/vm.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/21937/1
diff --git a/vdsm/vm.py b/vdsm/vm.py
index e8d5954..4ff5e27 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -2488,7 +2488,7 @@
try:
now = time.time()
if self.lastStatus == 'Down':
- return
+ return errCode['noVM']
if self.guestAgent and self.guestAgent.isResponsive():
self._guestEventTime = now
self._guestEvent = 'Powering down'
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
3:23 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
vm: Fix exception when shutdown() called on vm in 'Down' state
Attempting to call the shutdown() method on a VM which is in 'Down'
state resulted in an exception if called through xmlrpclib. Instead,
adopt the convention used by similar APIs such as migrate() and return
errCode['noVM'].
Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Bug-Url: https://bugzilla.redhat.com/1027297
Signed-off-by: Greg Padgett <gpadgett(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/21937
Reviewed-by: Sandro Bonazzola <sbonazzo(a)redhat.com>
Reviewed-by: Doron Fediuck <dfediuck(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
Reviewed-by: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm/vm.py
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Sandro Bonazzola: Looks good to me, but someone else must approve
Greg Padgett: Verified
Federico Simoncelli: Looks good to me, but someone else must approve
Doron Fediuck: Looks good to me, but someone else must approve
Dan Kenigsberg: Looks good to me, approved
Michal Skrivanek: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
6:25 a.m.
Michal Skrivanek has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
note this patch is going to be obsoleted by http://gerrit.ovirt.org/#/c/15829/ so it
doesn't make much sense for master
however for the time being it's ok
....................................................
File vdsm/vm.py
Line 2487: def shutdown(self, timeout, message):
Line 2488: try:
Line 2489: now = time.time()
Line 2490: if self.lastStatus == 'Down':
Line 2491: return errCode['noVM']
OTOH after the mentione patch you'll be getting noVM as the VM obejct would be
destroyed as part of shutdown….and since the usage is not really consistent ATM it's
probably better to keep noVM here
Line 2492: if self.guestAgent and self.guestAgent.isResponsive():
Line 2493: self._guestEventTime = now
Line 2494: self._guestEvent = 'Powering down'
Line 2495: self.log.debug('guestAgent shutdown called')
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
1:21 a.m.
Michal Skrivanek has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
....................................................
Commit Message
Line 3: AuthorDate: 2013-12-02 23:04:36 -0500
Line 4: Commit: Greg Padgett <gpadgett(a)redhat.com>
Line 5: CommitDate: 2013-12-02 23:10:08 -0500
Line 6:
Line 7: vm: Fix exception when shutdown() called on vm in 'Down' state
Currently engine follows up with a destroy command once VM is down and that finally
deletes the VM object. API has a check for the VM object.
The change in http://gerrit.ovirt.org/#/c/15829/ actually won't need this patch
Line 8:
Line 9: Attempting to call the shutdown() method on a VM which is in 'Down'
Line 10: state resulted in an exception if called through xmlrpclib. Instead,
Line 11: adopt the convention used by similar APIs such as migrate() and return
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
1:43 a.m.
Sandro Bonazzola has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:51 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3:23 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:28 a.m.
Michal Skrivanek has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1:
(1 comment)
....................................................
File vdsm/vm.py
Line 2487: def shutdown(self, timeout, message):
Line 2488: try:
Line 2489: now = time.time()
Line 2490: if self.lastStatus == 'Down':
Line 2491: return errCode['noVM']
Might be better than raising an error in this case, yes.
However please take into consideration the whole shutdown is being rewritten in
http://gerrit.ovirt.org/#/c/15829/.
Line 2492: if self.guestAgent and self.guestAgent.isResponsive():
Line 2493: self._guestEventTime = now
Line 2494: self._guestEvent = 'Powering down'
Line 2495: self.log.debug('guestAgent shutdown called')
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
7:57 a.m.
Federico Simoncelli has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Minor question for Michal.
....................................................
File vdsm/vm.py
Line 2487: def shutdown(self, timeout, message):
Line 2488: try:
Line 2489: now = time.time()
Line 2490: if self.lastStatus == 'Down':
Line 2491: return errCode['noVM']
In this specific case I'd consider to use the doneCode, since what you're asking
is to shutdown a vm (that is already down).
Michal do you have any preference? Receiving doneCode would be better for you?
Line 2492: if self.guestAgent and self.guestAgent.isResponsive():
Line 2493: self._guestEventTime = now
Line 2494: self._guestEvent = 'Powering down'
Line 2495: self.log.debug('guestAgent shutdown called')
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
6:30 a.m.
Doron Fediuck has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:18 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5865/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5069/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5957/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:16 p.m.
Greg Padgett has posted comments on this change.
Change subject: vm: Fix exception when shutdown() called on vm in 'Down' state
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.ovirt.org/21937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b75d0b869354d503e5f187e33370b5837b3c698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-HasComments: No
3588
days inactive
3590
days old
vdsm-patches@lists.fedorahosted.org
11 comments
7 participants
participants (7)
-
Dan Kenigsberg -
dfediuck＠redhat.com -
Federico Simoncelli -
gpadgett＠redhat.com -
Jenkins CI RO -
michal.skrivanek＠redhat.com -
Sandro Bonazzola