Allon Mureinik has posted comments on this change.

Change subject: core : correct message when suspend vm witout image 
disks(#841504)
......................................................................


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

(4 inline comments)

See inline.

Sepecifically, there seems to be a regression in the commit message - I 
commented on some errors in #1, and you fixed them for #2, but now they are 
back.
Please check there aren't any more regressions due to some git foobar.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java
Line 241:         if (getStorageDomainId().equals(Guid.Empty)) {
Generally, when comparing to constants, I prefer yoda style ifs, but I 
understand it was extracted from the old code - not nescessary for /this/ patch.

Line 264:         if (getVm().getis_stateless() ||
I'd move this if up a block, before the async check for two reasons:

1. There is a possibility to save a DB access here (ig getVm.getis_stateless() 
returns true).
2. It's related to the vm's definitions (a static state that requires editing 
the VM to change) and not to what's going on in the system right now (e.g., 
tasks are running). If a VM fails hibernation because it has running tasks, 
it's possible you'll just wait a couple of minutes and then successfully retry. 
If it fails because it's stateless, no matter how long you wait, hibernation 
won't work unless you actively edit the VM.

....................................................
Commit Message
Line 7: core : correct message when suspend vm witout image disks(#841504)
s/suspend/suspending/ s/witout/without/

Line 12: to. Before this message the message that was presented to the user was
s/this message/this patch/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I11fb32b54cf159fb63e172504342861493d2aa77
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to