Change in vdsm[master]: API: modernize VM methods

2017-02-17 Thread Code Review 
From Dan Kenigsberg :

Dan Kenigsberg has submitted this change and it was merged.

Change subject: API: modernize VM methods
..


API: modernize VM methods

This patch adds the wrapping using @api.method around most VM methods.
It also refactors how VM instances are obtained, saving quite a lot
of repetitive boilerplate code.
Few VM methods are left out, and those are the one which have special
cases which could not be trivially refactored yet.

Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Signed-off-by: Francesco Romani 
---
M vdsm/API.py
1 file changed, 99 insertions(+), 205 deletions(-)

Approvals:
  Piotr Kliczewski: Looks good to me, but someone else must approve
  Nir Soffer: Looks good to me, approved
  Jenkins CI: Passed CI tests
  Francesco Romani: Verified
  Milan Zamazal: Looks good to me, but someone else must approve



-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-10-25 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 20:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-29 Thread mzamazal 
Milan Zamazal has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 19: Code-Review+1

(2 comments)

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 125: 
Line 126: @property
Line 127: def vm(self):
Line 128: v = self._cif.vmContainer.get(self._UUID)
Line 129: if not v:
> Yes, it could be
It's a good practice to use the idiomatic check for None (is / is not) whether 
the object can be false or not.
Line 130: raise exception.NoSuchVM()
Line 131: return v
Line 132: 
Line 133: @api.method


https://gerrit.ovirt.org/#/c/61475/19/vdsm/API.py
File vdsm/API.py:

PS19, Line 338: to run
running ?


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-28 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(2 comments)

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 346
Line 347
Line 348
Line 349
Line 350
> Right, I missed this. Will fix.
You can fix the places which changed the semantics, or split them to the next 
patch, I guess most need the same fix like:

   vm = self.vm  # First check that the vm is running

Then the current code will probably work without change.


Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
> Let's keep this version as-is, I have further (minor) improvements in the p
+1 for current code.
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-28 Thread piotr . kliczewski 
Piotr Kliczewski has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 19: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-28 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 19:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-28 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(1 comment)

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 333: """
Line 334: return self._getStats()
Line 335: 
Line 336: def _getStats(self, runHooks=True):
Line 337: if runHooks:
> No, no changes with respect to the hold code.
Ah, now I see qhat you mean. Fixing.
Line 338: try:
Line 339: hooks.before_get_vm_stats()
Line 340: except exception.HookError as e:
Line 341: return response.error('hookError',


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-28 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(1 comment)

https://gerrit.ovirt.org/#/c/61475/18//COMMIT_MSG
Commit Message:

PS18, Line 9: all
> Actually not all.
most :)


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-28 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(6 comments)

The problem is that I don't know how the meaningfully split this patch without 
using one patch per method, which is unmanageable as well.

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 346
Line 347
Line 348
Line 349
Line 350
> and since the hooks are typically quite stupid it may cause a problem indee
Right, I missed this. Will fix.


Line 125: 
Line 126: @property
Line 127: def vm(self):
Line 128: v = self._cif.vmContainer.get(self._UUID)
Line 129: if not v:
> isn't it the opposite?
Yes, it could be

  if v is None

not sure what's more readable, we can't have a falsey VM.
Line 130: raise exception.NoSuchVM()
Line 131: return v
Line 132: 
Line 133: @api.method


Line 139: :type vmId: UUID
Line 140: :param driveSpec: specification of the new CD image. Either an
Line 141: image path or a `storage`-centric quartet.
Line 142: """
Line 143: self.vm.changeCD(driveSpec)
> No return value?
Right, will fix.
Line 144: 
Line 145: @api.method
Line 146: def changeFloppy(self, driveSpec):
Line 147: """


Line 151: :type vmId: UUID
Line 152: :param driveSpec: specification of the new CD image. Either an
Line 153: image path or a `storage`-centric quartet.
Line 154: """
Line 155: self.vm.changeFloppy(driveSpec)
> No return value?
Right, will fix.
Line 156: 
Line 157: @api.method
Line 158: def cont(self):
Line 159: return self.vm.cont()


Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
> +1 on current version
Let's keep this version as-is, I have further (minor) improvements in the 
pipeline
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


Line 333: """
Line 334: return self._getStats()
Line 335: 
Line 336: def _getStats(self, runHooks=True):
Line 337: if runHooks:
> Nothing to do when runHooks is false?
No, no changes with respect to the hold code.
Line 338: try:
Line 339: hooks.before_get_vm_stats()
Line 340: except exception.HookError as e:
Line 341: return response.error('hookError',


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-28 Thread michal . skrivanek 
Michal Skrivanek has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(3 comments)

indeed we should avoid semantics change, but please do not split into hundred 
little patches, that's way more difficult to see the bigger picture. If there 
are only handful of places where we consider semantic change is acceptable it 
is better to keep it in a single patch

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 346
Line 347
Line 348
Line 349
Line 350
> This changed the semantics - before we would raise before we run the hook, 
and since the hooks are typically quite stupid it may cause a problem indeed. 
We can likely live with it on master, but if it can be fixed easily it is a 
better option


Line 125: 
Line 126: @property
Line 127: def vm(self):
Line 128: v = self._cif.vmContainer.get(self._UUID)
Line 129: if not v:
> if v is not None: ?
isn't it the opposite?
Line 130: raise exception.NoSuchVM()
Line 131: return v
Line 132: 
Line 133: @api.method


Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
> If you create a variable, I would do:
+1 on current version
-1 on needless extra indirection
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(2 comments)

I think this patch is too big, lets change first the easy places where 
replacing the duplicate code with the new getter does not change the semantics.

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 346
Line 347
Line 348
Line 349
Line 350
This changed the semantics - before we would raise before we run the hook, now 
we would run the hook when no vm is running, and then fail in getStats()


Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
> +1
If you create a variable, I would do:

   ga = self.vm.guestAgent
   ga.desktopLock()
   if ga.isResponsive():
   ...

But I think the version Francesco submitted is more readable. We so such 
optimizations for tight loops since accessing local variables is much faster 
then globals or objects attributes.
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread ahadas 
Arik Hadas has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(1 comment)

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
> Would using a variable really make the code less readable? Not that importa
+1
I personally find:

vm = self.vm
vm.guestAgent.desktopLock()
if vm.guestAgent.isResponsive():

to be as easy to read as lines 266-267 above and less confusing since calling 
vm() twice could make people wonder "why the hell do they query the VM twice? 
can the VM change in the middle?"
but I don't really mind - it is really not that important here..
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread mzamazal 
Milan Zamazal has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18: Code-Review-1

(6 comments)

Nice change, but we should be careful and double check. I'm afraid there are 
some oversights.

https://gerrit.ovirt.org/#/c/61475/18//COMMIT_MSG
Commit Message:

PS18, Line 9: all
Actually not all.


https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 125: 
Line 126: @property
Line 127: def vm(self):
Line 128: v = self._cif.vmContainer.get(self._UUID)
Line 129: if not v:
if v is not None: ?
Line 130: raise exception.NoSuchVM()
Line 131: return v
Line 132: 
Line 133: @api.method


Line 139: :type vmId: UUID
Line 140: :param driveSpec: specification of the new CD image. Either an
Line 141: image path or a `storage`-centric quartet.
Line 142: """
Line 143: self.vm.changeCD(driveSpec)
No return value?
Line 144: 
Line 145: @api.method
Line 146: def changeFloppy(self, driveSpec):
Line 147: """


Line 151: :type vmId: UUID
Line 152: :param driveSpec: specification of the new CD image. Either an
Line 153: image path or a `storage`-centric quartet.
Line 154: """
Line 155: self.vm.changeFloppy(driveSpec)
No return value?
Line 156: 
Line 157: @api.method
Line 158: def cont(self):
Line 159: return self.vm.cont()


Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
> If we have a tight loop, yes for this code readability is most important.
Would using a variable really make the code less readable? Not that important 
here, just wondering.
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


Line 333: """
Line 334: return self._getStats()
Line 335: 
Line 336: def _getStats(self, runHooks=True):
Line 337: if runHooks:
Nothing to do when runHooks is false?
Line 338: try:
Line 339: hooks.before_get_vm_stats()
Line 340: except exception.HookError as e:
Line 341: return response.error('hookError',


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(1 comment)

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
> when self.vm is called twice within the same method, would it be better to 
If we have a tight loop, yes for this code readability is most important.
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread ahadas 
Arik Hadas has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(1 comment)

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
when self.vm is called twice within the same method, would it be better to 
assign it to some variable instead of triggering the vm() method twice?
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

Can we get reviews from the virt guys?

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread michal . skrivanek 
Michal Skrivanek has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

2:1 for line removals, that's always a good thing!

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18: Verified+1

fixed a bug in VM.migrate (unrelated hunk sneaked in). Now seems to work with 
the basic VM flows, hence V+1

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-27 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 17:

basic VM flows works, but migrations is broken. Investigating.

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-21 Thread piotr . kliczewski 
Piotr Kliczewski has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 17: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-20 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 17:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-20 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 9:

(2 comments)

https://gerrit.ovirt.org/#/c/61475/9/vdsm/API.py
File vdsm/API.py:

Line 37: from vdsm import supervdsm
Line 38: from vdsm import throttledlog
Line 39: from vdsm import jobs
Line 40: from vdsm import v2v
Line 41: from vdsm.common import api as wrapapi
> Can we use the original name?
I had a name clash. Not sure if it applies nowadays. Will try, and if still 
relevant I'll add a comment to explain.
Line 42: from vdsm.host import api as hostapi
Line 43: from vdsm.logUtils import AllVmStatsValue, Suppressed
Line 44: from vdsm.storage import clusterlock
Line 45: from vdsm.storage import misc


Line 127: @contextmanager
Line 128: def _vm_instance(self):
Line 129: v = self._cif.vmContainer.get(self._UUID)
Line 130: if not v:
Line 131: raise exception.NoSuchVM()
> Nice cleanup!
Good idea, will implement.
Line 132: yield v
Line 133: 
Line 134: @wrapapi.method
Line 135: def changeCD(self, driveSpec):


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-19 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 16:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-16 Thread piotr . kliczewski 
Piotr Kliczewski has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 15:

Nice cleanup, please respond to Nir's comments.

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-15 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 15: Code-Review-1

Please address comments from version 9:
https://gerrit.ovirt.org/#/c/61475/9/vdsm/API.py

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-15 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 15:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 14:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 13:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 12:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 11:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 10:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 9:

(2 comments)

https://gerrit.ovirt.org/#/c/61475/9/vdsm/API.py
File vdsm/API.py:

Line 37: from vdsm import supervdsm
Line 38: from vdsm import throttledlog
Line 39: from vdsm import jobs
Line 40: from vdsm import v2v
Line 41: from vdsm.common import api as wrapapi
Can we use the original name?
Line 42: from vdsm.host import api as hostapi
Line 43: from vdsm.logUtils import AllVmStatsValue, Suppressed
Line 44: from vdsm.storage import clusterlock
Line 45: from vdsm.storage import misc


Line 127: @contextmanager
Line 128: def _vm_instance(self):
Line 129: v = self._cif.vmContainer.get(self._UUID)
Line 130: if not v:
Line 131: raise exception.NoSuchVM()
Nice cleanup!

But we don't need a context manager for this, since we need any cleanup. The 
best place to raise this is in vmContainer.get.

If we cannot move the exception to the correct place in once step, lets have 
this vm property:

@property
def vm():
v = self._cif.vmContainer.get(self._UUID)
if not v:
raise exception.NoSuchVM()

Typical usage is:

self.vm.method(arg, ...)
Line 132: yield v
Line 133: 
Line 134: @wrapapi.method
Line 135: def changeCD(self, driveSpec):


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 9:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread igoihman 
Irit Goihman has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 8: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 8:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 7:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/61475/5/vdsm/API.py
File vdsm/API.py:

Line 37: from vdsm import supervdsm
Line 38: from vdsm import throttledlog
Line 39: from vdsm import jobs
Line 40: from vdsm import v2v
Line 41: from vdsm.common import api as wrapapi
just to avoid name clashes.
Line 42: from vdsm.host import api as hostapi
Line 43: from vdsm.logUtils import AllVmStatsValue, Suppressed
Line 44: from vdsm.storage import clusterlock
Line 45: from vdsm.storage import misc


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-09-14 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-08-02 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-07-28 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

2016-07-27 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org