Change in vdsm[master]: Added a support for a guest hibernate command.

Gal Hammer has uploaded a new change for review.

Change subject: Added a support for a guest hibernate command.
......................................................................

Added a support for a guest hibernate command.

Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
---
M vdsm/clientIF.py
M vdsm/guestIF.py
M vdsm_cli/vdsClient.py
3 files changed, 32 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 12: Fails

Build Failed 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/517/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/552/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 12:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/552/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 12:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/517/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 11: (3 inline comments)

....................................................
File vdsm/BindingXMLRPC.py
Line 327:     def vmDesktopSendHcCommand(self, vmId, message):
Line 328:         vm = API.VM(vmId)
Line 329:         return vm.desktopSendHcCommand(message)
Line 330: 
Line 331:     def vmHibernate(self, vmId, target=None):
Yes, this is the old behaviour, here was just a name change of the variable to reflect the
purpose better. It's a target now and not only volume handle
Line 332:         vm = API.VM(vmId)
Line 333:         return vm.hibernate(target)
Line 334: 
Line 335:     def vmMonitorCommand(self, vmId, cmd):


....................................................
File vdsm_cli/vdsClient.py
Line 61: LEAF_VOL = 8
Line 62: 
Line 63: # Guest Hibernate
Line 64: HIBERNATE_TARGET_DISK = "disk"
Line 65: HIBERNATE_TARGET_MEMORY = "mem"
These are the constants for vdsClient, there's no import possible from vdsm
they're not installed together so far.
Line 66: HIBERNATE_TARGET_DEFAULT = HIBERNATE_TARGET_DISK
Line 67: 
Line 68: 
Line 69: def validateArgTypes(args, conv, requiredArgsNumber=0):


....................................................
File vdsm/vm.py
Line 976:                                      'sys_shutdown_timeout',
Line 977:                                      self.guestAgent.desktopShutdown, timeout,
Line 978:                                      message)
Line 979: 
Line 980:     def hibernate(self, target='disk'):
This should be the constant
Line 981:         return self._shutdownCommand('Prepare for hibernate', 0,
Line 982:                                      'sys_hibernate_timeout',
Line 983:                                      self.guestAgent.hibernate, target)
Line 984: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Peter V. Saveliev has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 11: (3 inline comments)

....................................................
File vdsm/BindingXMLRPC.py
Line 327:     def vmDesktopSendHcCommand(self, vmId, message):
Line 328:         vm = API.VM(vmId)
Line 329:         return vm.desktopSendHcCommand(message)
Line 330: 
Line 331:     def vmHibernate(self, vmId, target=None):
by the way, is it correct to provide None to vm.hibernate?
Line 332:         vm = API.VM(vmId)
Line 333:         return vm.hibernate(target)
Line 334: 
Line 335:     def vmMonitorCommand(self, vmId, cmd):


....................................................
File vdsm_cli/vdsClient.py
Line 61: LEAF_VOL = 8
Line 62: 
Line 63: # Guest Hibernate
Line 64: HIBERNATE_TARGET_DISK = "disk"
Line 65: HIBERNATE_TARGET_MEMORY = "mem"
again, is it ok to have it hardcoded, if we have constants?
Line 66: HIBERNATE_TARGET_DEFAULT = HIBERNATE_TARGET_DISK
Line 67: 
Line 68: 
Line 69: def validateArgTypes(args, conv, requiredArgsNumber=0):


....................................................
File vdsm/vm.py
Line 976:                                      'sys_shutdown_timeout',
Line 977:                                      self.guestAgent.desktopShutdown, timeout,
Line 978:                                      message)
Line 979: 
Line 980:     def hibernate(self, target='disk'):
«target: Either HibernateTarget.DISK, HibernateTarget.MEMORY or …»

why here the target default value is hardcoded, not a constant?
Line 981:         return self._shutdownCommand('Prepare for hibernate', 0,
Line 982:                                      'sys_hibernate_timeout',
Line 983:                                      self.guestAgent.hibernate, target)
Line 984: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

ShaoHe Feng has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(1 inline comment)

....................................................
File vdsm_cli/vdsClient.py
Line 2324:                        'channel'
Line 2325:                        )),
Line 2326:         'hibernate': (serv.hibernate,
Line 2327:                       ('<vmId> [<target>]\n\t<target> is
"disk" (default), '
Line 2328:                        '"mem" or a valid volume handle',
what is "a valid volume handle"

should be also added in the schema.json?  no description about it.
Line 2329:                        'Hibernates the desktop'
Line 2330:                        )),
Line 2331:         'monitorCommand': (serv.monitorCommand,
Line 2332:                            ('<vmId> <string>',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

Gal, the comparison to shutdown shows how much this patch has yet to improve:

* shutdown gives am immediate visible change in the Vm state ("Powering Done").
Currently your patch leaves the Vm in UP until it just dies.
* shutdown ends when the Vm reports exitMessage "User shut down". I think this
would be also the case after this version of guestHibernate, but it SHOULD be
"SaveState succeeded"
* not that shutdown has a timeout - if the Vm still exists after 2 minutes, it is declared
to be Up again.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;

Gal Hammer has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 2:

ovirt-guest-agent code: http://gerrit.ovirt.org/#change,1122

Why do you want to know if the hibernation succeeded? It is the same as shutdown. The
logic which check if qemu was shutdown after some time since the command was send belong
to the backend.

I am not familiar with the vdms's hooks. It probably should be added as a standalone
patch if this patch (hibernate) will be accepted.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;

Gal Hammer has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File vdsm_cli/vdsClient.py
Line 1923:                             )),                            
Done

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File vdsm_cli/vdsClient.py
Line 1923:                             )),                            
looks like a missed blank space here.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 11:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/313/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/347/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 11:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/347/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 11:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/313/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

Build Unstable 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/311/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/345/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 9: (2 inline comments)

....................................................
File vdsm/API.py
Line 347:         """
Line 348:         Hibernate a VM.
Line 349: 
Line 350:         :param target: Either "disk", "mem" or an opaque
string, indicating
Line 351:             the location of hibernation images.
Done
Line 352:         """
Line 353:         if target == HibernateTarget.DISK or target == HibernateTarget.MEM:
Line 354:             try:
Line 355:                 vm = self._cif.vmContainer[self._UUID]


....................................................
File vdsm_cli/vdsClient.py
Line 60: LEAF_VOL = 8
Line 61: 
Line 62: # Guest Hibernate
Line 63: HIBERNATE_TARGET_DISK    = 'disk'
Line 64: HIBERNATE_TARGET_MEMORY  = 'mem'
I see now that this is currently not an option therefore we'll have to leave it for
the revamp of the API
Line 65: HIBERNATE_TARGET_DEFAULT = HIBERNATE_TARGET_DISK
Line 66: 
Line 67: def validateArgTypes(args, conv, requiredArgsNumber=0):
Line 68:     if len(args) > len(conv) or len(args) < requiredArgsNumber:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 10:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/311/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 10:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/345/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Michal Skrivanek has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 9: (1 inline comment)

....................................................
File vdsm/vm.py
Line 928:                 self._acpiShutdown()
Line 929:             # No tools, no ACPI
Line 930:             else:
Line 931:                 return {'status': {'code':
errCode['exist']['status']['code'],
Line 932:                         'message': 'VM without ACPI or active oVirt
tools. Try forced shutdown.'}}
"guest tools" would be better
Line 933:         except:
Line 934:             self.log.error("Shutdown failed", exc_info=True)
Line 935:         return {'status': {'code': doneCode['code'],
Line 936:                 'message': 'Machine shut down'}}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Barak Azulay has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 4: (1 inline comment)

....................................................
File vdsm/API.py
Line 335:     def guestHibernate(self, vmId, state='disk'):
The same API can be used for S4 and S3, but the default is S4 (suspend to disk)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(3 inline comments)

....................................................
File vdsm/API.py
Line 347:         """
Line 348:         Hibernate a VM.
Line 349: 
Line 350:         :param target: Either "disk", "mem" or an opaque
string, indicating
Line 351:             the location of hibernation images.
I would mention here also HibernateTarget.DISK and HibernateTarget.MEM as values and not
the plain strings. If we for whatever reason later decide to rename them we might break
something else which relied on this documentation.
Line 352:         """
Line 353:         if target == HibernateTarget.DISK or target == HibernateTarget.MEM:
Line 354:             try:
Line 355:                 vm = self._cif.vmContainer[self._UUID]


....................................................
File vdsm_cli/vdsClient.py
Line 60: LEAF_VOL = 8
Line 61: 
Line 62: # Guest Hibernate
Line 63: HIBERNATE_TARGET_DISK    = 'disk'
Line 64: HIBERNATE_TARGET_MEMORY  = 'mem'
I would prefer if we could move out such constants into some module which is commonly used
between VDSM and vdsmCli
Line 65: HIBERNATE_TARGET_DEFAULT = HIBERNATE_TARGET_DISK
Line 66: 
Line 67: def validateArgTypes(args, conv, requiredArgsNumber=0):
Line 68:     if len(args) > len(conv) or len(args) < requiredArgsNumber:


....................................................
File vdsm/vm.py
Line 928:                 self._acpiShutdown()
Line 929:             # No tools, no ACPI
Line 930:             else:
Line 931:                 return {'status': {'code':
errCode['exist']['status']['code'],
Line 932:                         'message': 'VM without ACPI or active oVirt
tools. Try forced shutdown.'}}
Should we really have the string 'oVirt' in here?
Line 933:         except:
Line 934:             self.log.error("Shutdown failed", exc_info=True)
Line 935:         return {'status': {'code': doneCode['code'],
Line 936:                 'message': 'Machine shut down'}}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Michal Skrivanek has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

since engine is always invoking it with volume handle, I don't mind much changing the
default...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Gal Hammer has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 8: (1 inline comment)

>  Now it is: def vmHibernate(self, vmId,
> target=API.VM.HibernateTarget.DISK) 
I guess it should be target=None as before.

....................................................
File vdsm/vm.py
Line 848:         return self._shutdownCommand('Powering down', int(timeout),
Line 849:             'sys_shutdown_timeout', self.guestAgent.desktopShutdown,
timeout,
Line 850:             message)
Line 851: 
Line 852:     def hibernate(self, target='disk'):
I don't remember. This patch was written months ago.

I can guess that it is either because I didn't update the code after I've added
the HibernateTarget constants or because HibernateTarget is not imported in this file.
Line 853:         return self._shutdownCommand('Prepare for hibernate', 0,
Line 854:             'sys_hibernate_timeout', self.guestAgent.hibernate, target)
Line 855: 
Line 856:     def _shutdownCommand(self, event, timeout, timeout_var, command, *args):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 8:

Just to get that straight. You have had before:

in BindingXMLRPC.py:
def vmHibernate(self, vmId, hiberVolHandle=None)

Now it is:
def vmHibernate(self, vmId, target=API.VM.HibernateTarget.DISK)

If the second parameter is not passed, is VDSM still doing the same as before? Or does it
do something else?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(1 inline comment)

....................................................
File vdsm/vm.py
Line 848:         return self._shutdownCommand('Powering down', int(timeout),
Line 849:             'sys_shutdown_timeout', self.guestAgent.desktopShutdown,
timeout,
Line 850:             message)
Line 851: 
Line 852:     def hibernate(self, target='disk'):
Why don't you use here HibernateTarget.DISK as well? I don't understand why you
introduced the constant and then use a literal here.
Line 853:         return self._shutdownCommand('Prepare for hibernate', 0,
Line 854:             'sys_hibernate_timeout', self.guestAgent.hibernate, target)
Line 855: 
Line 856:     def _shutdownCommand(self, event, timeout, timeout_var, command, *args):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Gal Hammer has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 8:

I don't understand. Do you want me to re-write the patch in such a way that it will
break backward compatibility and then fix that?!

Will someone explain to me what am I missing and how did I break compatibility?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Michal Skrivanek has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

please make the old behavior(migrate to file) still the default until/unless the engine
side is implemented. We need to support hibernation for all guest types and keep the most
safe default. The decision which method to use should come from the engine - either as
depending on OS type or user configurable.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(1 inline comment)

....................................................
File vdsm/API.py
Line 342:         stats = v.getStats().copy()
Line 343:         stats['vmId'] = self._UUID
Line 344:         return {'status': doneCode, 'statsList': [stats]}
Line 345: 
Line 346:     def hibernate(self, target):
iiuc = if I understand correctly
Line 347:         """
Line 348:         Hibernate a VM.
Line 349: 
Line 350:         :param target: Either "disk", "mem" or an opaque
string, indicating


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Gal Hammer has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 8: (1 inline comment)

....................................................
File vdsm/API.py
Line 342:         stats = v.getStats().copy()
Line 343:         stats['vmId'] = self._UUID
Line 344:         return {'status': doneCode, 'statsList': [stats]}
Line 345: 
Line 346:     def hibernate(self, target):
International Islamic University Chittagong?

No. It does not expected to breaks backwards compatibility. I've added a
"special" targets called "disk" and "mem". These were
illegal target names in the old code.
Line 347:         """
Line 348:         Hibernate a VM.
Line 349: 
Line 350:         :param target: Either "disk", "mem" or an opaque
string, indicating


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(1 inline comment)

....................................................
File vdsm/API.py
Line 342:         stats = v.getStats().copy()
Line 343:         stats['vmId'] = self._UUID
Line 344:         return {'status': doneCode, 'statsList': [stats]}
Line 345: 
Line 346:     def hibernate(self, target):
iiuc this breaks backwards compatibility.
We still need to support hibernating to an external device.
It's quite likely that we'll want to do so in the long run as well (i.e. it's
not even deprecated)
Line 347:         """
Line 348:         Hibernate a VM.
Line 349: 
Line 350:         :param target: Either "disk", "mem" or an opaque
string, indicating


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Itamar Heim has abandoned this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Abandoned

abandoning - old. no reply. not touched for a while. please restore if relevant

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Better Saggi <bettersaggi(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <haim.ateya(a)gmail.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <svinota.saveliev(a)gmail.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(3 inline comments)

....................................................
Commit Message
Line 6: 
Line 7: Added a support for a guest hibernate command.
Line 8: 
Line 9: The guestHibernate command can put the guest to either sleep
Line 10: (suspend to memory) or hibernation (suspend to disk) state.
Can you add a comment on how you wake up the guest from the hibernation to disk/memory?
Line 11: 
Line 12: Signed-off-by: Gal Hammer <ghammer(a)redhat.com&gt;
Line 13: Signed-off-by: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;


....................................................
File vdsm/config.py.in
Line 84:         ('sys_shutdown_timeout', '10',
Line 85:             'Destroy and shutdown timeouts (in sec) before completing the
'
Line 86:             'action.'),
Line 87: 
Line 88:         ('sys_hibernate_timeout', '120',
This seems a little bit low as timeout since it might not be fair in all cases. Maybe
it's better to define the timeout per Gb (of guest memory) in the disk hibernation
case. E.g. 40 seconds timeout per Gb (or any other value that you see fit).
Line 89:             'Timeout (in sec) before assuming that operating system did not
'
Line 90:             'completed its hibernation process.'),
Line 91: 
Line 92:         ('user_shutdown_timeout', '30',


....................................................
File vdsm/vm.py
Line 991:                                      self.guestAgent.desktopShutdown, timeout,
Line 992:                                      message)
Line 993: 
Line 994:     def hibernate(self, target=HibernateTarget.DISK):
Line 995:         return self._shutdownCommand('Prepare for hibernate', 0,
There is actually a support for this in libvirt (and their guest agent: qemu-guest-agent).
I suggest you to take a look at: libvirt.virDomain.pMSuspendForDuration.
We might want to try to leverage that.
Line 996:                                      'sys_hibernate_timeout',
Line 997:                                      self.guestAgent.hibernate, target)
Line 998: 
Line 999:     def _shutdownCommand(self, event, timeout, timeout_var, command, *args):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 8:

Build Successful 

http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/209/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 8: No score

Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/209/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Peter V. Saveliev has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 15:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/563/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 15:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/528/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/563/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 15:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/528/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 14:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/527/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 14:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/562/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 13:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/526/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/561/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 13:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/526/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 13:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/561/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Adam Litke has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

Thanks for updating the schema!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;

Gal Hammer has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 7: (1 inline comment)

....................................................
File vdsm/BindingXMLRPC.py
Line 322:     def vmHibernate(self, vmId, target='disk'):
Done

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;

Adam Litke has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 7: (1 inline comment)

Hi Gal, we are in the process of formalizing the API with a schema (see
vdsm_api/vdsmapi-schema.json).  Since your patch modifies the API you will have to modify
the API schema accordingly.

You have to modify the VM.hibernate command definition to account for the expanded set of
values allowed in the target parameter.

Your API change essentially defines the 'target' parameter as a new union type. 
It can be either a HibernationVolumeHandle (string) or a HibernationMode (enum).

So you need to create the HibernationMode enum (containing 'disk' and
'memory') and a HibernationTarget union that can be either a HibernationMode or a
HibernationVolumeHandle.  Then change the VM.Hibernate command definition to reflect the
changes in the types as well.

....................................................
File vdsm/BindingXMLRPC.py
Line 322:     def vmHibernate(self, vmId, target='disk'):
target=<...>.DISK in order to use your new class definition

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;

Itamar Heim has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 16:

ping

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Better Saggi <bettersaggi(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <haim.ateya(a)gmail.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(2 inline comments)

....................................................
File vdsm/API.py
Line 335:     def guestHibernate(self, vmId, state='disk'):
what is the point of the 'state' argument here? which values are expected for it?

....................................................
File vdsm/vm.py
Line 824:     def _shutdownCommand(event_str, timeout_var, guest_command, *kargs):
no 'self' in arg list.

please test your code before submitting it.

 *args
 **kwargs

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;

Peter V. Saveliev has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Michal Skrivanek has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 16:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1245/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1280/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/391/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 15: (2 inline comments)

....................................................
File vdsm_api/vdsmapi-schema.json
Line 5158: # Since: 4.10.0
Line 5159: ##
Line 5160: {'union': 'HibernateTarget',
Line 5161:  'data': ['HibernationVolumeHandle',
'HibernateState']}
Line 5162: 
Done
Line 5163: ##
Line 5164: # @VM.hibernate:
Line 5165: #
Line 5166: # Save the live state of the VM to disk and stop it.


....................................................
File vdsm_cli/vdsClient.py
Line 2324:                        'channel'
Line 2325:                        )),
Line 2326:         'hibernate': (serv.hibernate,
Line 2327:                       ('<vmId> [<target>]\n\t<target> is
"disk" (default), '
Line 2328:                        '"mem" or a valid volume handle',
Done
Line 2329:                        'Hibernates the desktop'
Line 2330:                        )),
Line 2331:         'monitorCommand': (serv.monitorCommand,
Line 2332:                            ('<vmId> <string>',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 16:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/391/ (3/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 16:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1245/ (1/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 16:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1280/ (2/3)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Adam Litke has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(1 inline comment)

....................................................
File vdsm_api/vdsmapi-schema.json
Line 5158: # Since: 4.10.0
Line 5159: ##
Line 5160: {'union': 'HibernateTarget',
Line 5161:  'data': ['HibernationVolumeHandle',
'HibernateState']}
Line 5162: 
This is invalid syntax for a union (but I think it may have been valid in an earlier
incarnation of the schema).  Take a look at other unions in the schema for a correct
example:

##
# @NetInfoNetwork:
#
# A discriminated record to store information about a VDSM network.
#
# @bridged:  If True, the union is a 'NetInfoBridgedNetwork'
#            If False, the union is a 'NetInfoBridgelessNetwork'
#
# Since: 4.10.0
##
{'type': 'NetInfoNetwork',
 'data': {'bridged': 'bool'},
 'union': ['NetInfoBridgedNetwork', 'NetInfoBridgelessNetwork']}
Line 5163: ##
Line 5164: # @VM.hibernate:
Line 5165: #
Line 5166: # Save the live state of the VM to disk and stop it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Michal Skrivanek has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

please rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 5: (1 inline comment)

....................................................
File vdsm/BindingXMLRPC.py
Line 330:     def vmGuestHibernate(self, vmId, state='disk'):
Actually hibernate implies S4, but this function is for both S4 and S3.
I agree that vmGuest... is redundant.
How about vmSuspend? (S3 is suspend to ram, S4 is suspend to disk).  It also complies with
Fedora's naming.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;

Adam Litke has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 5: (1 inline comment)

....................................................
File vdsm/API.py
Line 408:     def guestHibernate(self, state='disk'):
That is not enough though.  For new APIs, the strings need to be defined in API.py.  See
http://gerrit.ovirt.org/#change,2262 for an example of what I am asking for.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;

Gal Hammer has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 5: (8 inline comments)

....................................................
File vdsm/API.py
Line 408:     def guestHibernate(self, state='disk'):
The name "hibernate" for the function is already un used by the
"hibernate" function.

Dan rejected my offer to add the S4 feature the existing hibernate function.

The values are defined in the vdsClient.py file.

....................................................
File vdsm/BindingXMLRPC.py
Line 330:     def vmGuestHibernate(self, vmId, state='disk'):
Like the one on line 326?

Line 330:     def vmGuestHibernate(self, vmId, state='disk'):
See vdsClient.py for the allowed values.

The name state come from the Windows' API SetSystemState that put the guest into S4.

Line 755:                 (self.vmGuestHibernate, 'guestHibernate'),
(self.vmHibernate, 'hibernate'), ... already in use.

....................................................
File vdsm_cli/vdsClient.py
Line 57: # Guest Hibernate 
Don't you just love them? :-)

Line 59: HIBERNATE_DEFAULT = 'disk'
I don't see that API.py is used in vdsClient.py.

....................................................
File vdsm/vm.py
Line 850:         return self._shutdownCommand('Powering down', int(timeout),
The casting to int was the exiting code (including the config.getint). So I saw no reason
to change it in this patch.

Line 854:     def hibernate(self, state='disk'):
I just follow the same naming convernsion as other commands like shutdown (and not
guestShutdown).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;

[Haim Ateya]

Haim Ateya has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 5: Verified

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(5 inline comments)

mostly minor comments, thanks!

....................................................
File vdsm/BindingXMLRPC.py
Line 330:     def vmGuestHibernate(self, vmId, state='disk'):
What are the acceptable 'state's? What are their meanings?
Hard-coding 'disk' sounds wrong - better define a set of CONSTANTS for them.

Isn't "target" a better name from the arg?

....................................................
File vdsm_cli/vdsClient.py
Line 57: # Guest Hibernate 
redspace

Line 59: HIBERNATE_DEFAULT = 'disk'
ok, I see that you define the "states" here, but this have a proper definition
in the server side, too.

....................................................
File vdsm/vm.py
Line 850:         return self._shutdownCommand('Powering down', int(timeout),
why int(timeout)?! just to spite me?

Line 854:     def hibernate(self, state='disk'):
guestHibernate would be less confusing to me

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


Patch Set 15: (1 inline comment)

....................................................
File vdsm_cli/vdsClient.py
Line 2324:                        'channel'
Line 2325:                        )),
Line 2326:         'hibernate': (serv.hibernate,
Line 2327:                       ('<vmId> [<target>]\n\t<target> is
"disk" (default), '
Line 2328:                        '"mem" or a valid volume handle',
It is documented there...
Line 2329:                        'Hibernates the desktop'
Line 2330:                        )),
Line 2331:         'monitorCommand': (serv.monitorCommand,
Line 2332:                            ('<vmId> <string>',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com&gt;
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

Thanks, Gal. Could you point me to the ovirt-guest-agent code?

After guestHibernate is called, qemu is expected to dies after some time. I would like
Vdsm to report if hibernation succeeded (or that it was an untimely death of qemu).

Could this be done? Could qemu and the guest agent help differentiate the two cases?

Worse case, we can assume that qemu dying within 5 minutes of a hibernation request means
a successful hibernation.

We would also need to trigger the hibernate/dehibernate hooks accordingly.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;

Adam Litke has posted comments on this change.

Change subject: Added a support for a guest hibernate command.
......................................................................


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

(5 inline comments)

Just a few function naming issues and you also need to formally define the valid
parameters to hibernate.

....................................................
File vdsm/API.py
Line 408:     def guestHibernate(self, state='disk'):
I would prefer that you name the function 'hibernate' to conform with the naming
standards of the rest of the API functions in the VM class.  The word 'guest' is
redundant.

Dan has asked you to define the valid values for the 'state' parameter.  Those
should be defined in the VM class in this file since they are a part of the public API. 
Please add them as public constants in this class.  Perhaps:

HIBERNATE_MODE = {
    0: 'disk',
    1: 'ram'
}

....................................................
File vdsm/BindingXMLRPC.py
Line 330:     def vmGuestHibernate(self, vmId, state='disk'):
This should be named vmHibernate

Line 755:                 (self.vmGuestHibernate, 'guestHibernate'),
How about:
(self.vmHibernate, 'vmHibernate'),

?

....................................................
File vdsm_cli/vdsClient.py
Line 59: HIBERNATE_DEFAULT = 'disk'
Yes. I think it needs to go into API.py in the actual VM class so that it is a part of the
public API.

....................................................
File vdsm/vm.py
Line 854:     def hibernate(self, state='disk'):
I think 'guest' is implied since we are in vm.py and dealing with the vm.  I would
prefer it stay named hibernate.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad6836e61e9d91ec6f46a599541f61ff12e9737
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com&gt;
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com&gt;
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com&gt;