Change in vdsm[master]: virt: Move VM status check from vm.cont() to API level

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

Change subject: virt: Move VM status check from vm.cont() to API level
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/47527/4//COMMIT_MSG
Commit Message:

Line 10: occured while hibernating a VM, in SourceThread._recover() vm.cont()
Line 11: will fail and the VM will be left paused.
Line 12: 
Line 13: To prevent client from calling vm.cont() in VM states that don't allow
Line 14: this operation, check the state of the VM an API level.
> That was my first solution that I posted in the first version of this patch
Why internal usage does not need this check?

This kind of checks is certainly needed in the object performing the operation, 
and the place for this code is the object holding the state.
Line 15: 
Line 16: Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Line 17: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1238536


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: Move VM status check from vm.cont() to API level

2016-05-15 Thread smelamud 
Shmuel Leib Melamud has posted comments on this change.

Change subject: virt: Move VM status check from vm.cont() to API level
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/47527/4//COMMIT_MSG
Commit Message:

Line 10: occured while hibernating a VM, in SourceThread._recover() vm.cont()
Line 11: will fail and the VM will be left paused.
Line 12: 
Line 13: To prevent client from calling vm.cont() in VM states that don't allow
Line 14: this operation, check the state of the VM an API level.
> I don't understand why we need to do this in API.py.
That was my first solution that I posted in the first version of this patch. 
But reviewers noted correctly, that adding a parameter for one specific case to 
override the check is not a beautiful solution.

The question is: why we need to check VM status in vm.cont()? What does it 
prevent? The only possible cause is to prevent client from using cont() on a VM 
that is not down/paused/hibernated. If so, we need to check the VM state only 
when cont() is called by client, the internal usages don't need this check. 
Thus, it is logical to move this check to API layer.
Line 15: 
Line 16: Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Line 17: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1238536


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: Move VM status check from vm.cont() to API level

2016-05-11 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: virt: Move VM status check from vm.cont() to API level
..


Patch Set 4: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/47527/4//COMMIT_MSG
Commit Message:

Line 7: virt: Move VM status check from vm.cont() to API level
Line 8: 
Line 9: Allow vm.cont() to resume the VM in any state. Otherwise, if error
Line 10: occured while hibernating a VM, in SourceThread._recover() vm.cont()
Line 11: will fail and the VM will be left paused.
Please provide a method or an option in the Vm.cont, so SourceThread._recover 
can call  to unpause the vm in all states.

For example

vm.cont(force=True)
Line 12: 
Line 13: To prevent client from calling vm.cont() in VM states that don't allow
Line 14: this operation, check the state of the VM an API level.
Line 15: 


Line 10: occured while hibernating a VM, in SourceThread._recover() vm.cont()
Line 11: will fail and the VM will be left paused.
Line 12: 
Line 13: To prevent client from calling vm.cont() in VM states that don't allow
Line 14: this operation, check the state of the VM an API level.
I don't understand why we need to do this in API.py.

Vm state is available to the vm in Vm.cont, why not use it there?

We want to kill the API layer, not add more funcionallity.
Line 15: 
Line 16: Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Line 17: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1238536


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: Move VM status check from vm.cont() to API level

2016-05-11 Thread mpolednik 
Martin Polednik has posted comments on this change.

Change subject: virt: Move VM status check from vm.cont() to API level
..


Patch Set 4: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: Move VM status check from vm.cont() to API level

2016-05-11 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: virt: Move VM status check from vm.cont() to API level
..


Patch Set 4:

Rerun-Hooks: all

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: Move VM status check from vm.cont() to API level

2016-05-11 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: virt: Move VM status check from vm.cont() to API level
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: Move VM status check from vm.cont() to API level

2016-05-05 Thread smelamud 
Shmuel Leib Melamud has posted comments on this change.

Change subject: virt: Move VM status check from vm.cont() to API level
..


Patch Set 4: Verified+1

As discussed, moved VM status check from vm.cont() to the API level.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: Move VM status check from vm.cont() to API level

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

Change subject: virt: Move VM status check from vm.cont() to API level
..


Patch Set 4:

* #1238536::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1238536::OK, public bug
* Check Product::#1238536::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches