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 <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Gal Hammer <[email protected]>
Gerrit-Reviewer: Haim Ateya <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches
