Allon Mureinik has posted comments on this change.
Change subject: core: OvfDataUpdater - removal of update/remove OVF vdsm calls
......................................................................
Patch Set 20: I would prefer that you didn't submit this
(12 inline comments)
....................................................
File backend/manager/dbscripts/upgrade/03_01_1530_add_vm_generation_columns.sql
Line 1
How is deleting this flie relevant to this patch?
And generally speaking - you are NOT supposed to delete upgrade scripts. I'm
assuming this is a git foobar?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 939
Line 940
Line 941
Line 942
Line 943
where do we increment the version? in the endAction?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java
Line 397
Line 398
Line 399
Line 400
Line 401
so where do we increment the version?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java
Line 177:
Line 178: }
Line 179:
Line 180: protected void incrementDbGeneration() {
Line 181: getVmStaticDAO().incrementDbGeneration(getVm().getId());
Ins't this already declared in MoveOrCopyTemplateCommand?
Line 182: }
Line 183:
Line 184: @Override
Line 185: protected void moveOrCopyAllImageGroups() {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
Line 314: private void endCommand() {
Line 315: List<VM> listVms = getVmsForDiskId();
Line 316: for (VM vm : listVms) {
Line 317: getVmStaticDAO().incrementDbGeneration(vm.getId());
Line 318: }
You aren't done with the VM yet, IIUC.
why not move this down a line?
Line 319: Backend.getInstance().EndAction(VdcActionType.RemoveImage,
getParameters().getImagesParameters().get(0));
Line 320: setSucceeded(true);
Line 321: }
Line 322:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java
Line 68
Line 69
Line 70
Line 71
Line 72
just to be sure, the OvdUpdater recognizes VMs with generations and no data,
right?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
Line 79:
Line 80: @Override
Line 81: protected void executeCommand() {
Line 82: if (getVmTemplate() != null) {
Line 83:
getVmStaticDAO().incrementDbGeneration(getVmTemplate().getId());
what about the
VmTemplateHandler.BlankVmTemplateId.equals(getVmTemplate().getId() condition?
Line 84: UpdateVmTemplate();
Line 85: setSucceeded(true);
Line 86: }
Line 87: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
Line 79: "auto_startup",
Line 80: "dedicated_vm_for_vds", "default_display_type",
"priority", "default_boot_sequence", "initrd_url",
Line 81: "kernel_url", "kernel_params", "migrationSupport",
"minAllocatedMem", "quotaId", "quotaName",
Line 82: "quotaEnforcementType", "cpuPinning",
Line 83: "vmPayload", "balloonEnabled",
"smartcardEnabled","deleteProtected","db_generation",
should be part of the previous patch
Line 84: "images", // images list is relational entity - ignore
value changes
Line 85: "interfaces" // interfaces is relational entity -
ignore value changes
Line 86: });
Line 87: mUpdateVmsStatic.AddFields(
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java
Line 16: import org.ovirt.engine.core.utils.transaction.TransactionSupport;
Line 17:
Line 18: public class VmTemplateHandler {
Line 19: public static Guid BlankVmTemplateId = new Guid();
Line 20: public static String BlankVmTemplateName = "Blank";
How is this related to this patch?
Line 21: public static ObjectIdentityChecker mUpdateVmTemplate;
Line 22:
Line 23: /**
Line 24: * Initialize static list containers, for identity and permission
check. The initialization should be executed
....................................................
Commit Message
Line 11: given vm/template has been updated.
Line 12:
Line 13: Generally, most commands are performed under lock to a specific
Line 14: vm/template, but not all of them - as the row which contains the db
Line 15: generation of a vm/template is
misleading \n here
Line 16: accessed from many commands that perform operation on it , the update
Line 17: statement to this row has been placed in the start of the db update
Line 18: transaction in order to avoid possible db deadlocks and errors .
Line 19:
Line 14: vm/template, but not all of them - as the row which contains the db
Line 15: generation of a vm/template is
Line 16: accessed from many commands that perform operation on it , the update
Line 17: statement to this row has been placed in the start of the db update
Line 18: transaction in order to avoid possible db deadlocks and errors .
remove space before "."
Line 19:
Line 20: Change-Id: Iedebceb9809dc0b11c0bbe8a2d4af63b0d848df1
Line 15: generation of a vm/template is
Line 16: accessed from many commands that perform operation on it , the update
Line 17: statement to this row has been placed in the start of the db update
Line 18: transaction in order to avoid possible db deadlocks and errors .
Line 19:
Also please add a link to the feature page.
Line 20: Change-Id: Iedebceb9809dc0b11c0bbe8a2d4af63b0d848df1
--
To view, visit http://gerrit.ovirt.org/9340
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iedebceb9809dc0b11c0bbe8a2d4af63b0d848df1
Gerrit-PatchSet: 20
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
