Ayal Baron has posted comments on this change. Change subject: core: export/import of diskless VM/VM with no snappable disks(#838937) ......................................................................
Patch Set 3: I would prefer that you didn't submit this (6 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java Line 183: getDisksBasedOnImage(); shouldn't this be 'getSnappableDisks' or something? (internal implementation could be getDisksBasedOnImage) Line 421: log.warn("ExportVmCommand::EndMoveVmCommand: Vm is null - not performing full EndAction"); what is the new behaviour now that you don't setSucceeded to true ? Line 427: EndActionOnAllImageGroups(); previously this was run even if vm == null. Is there a situation where we'd have disks to end action on but vm == null? (sounds odd, but need to make sure why code worked as it did). Line 452: this.endDefaultOps(vm); although I have no problem with it, just want to make sure that you realize that this op goes to vdsm and user is waiting in sync mode. .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java Line 904: if (getVm() != null) { brrrr. If what you want is to make sure you reach the db then create a method getVmFromDB which does: setVm(null) return getVm() Right now it looks horrible. Line 907: UpdateVmImSpm(); If you get the chance, please send a patch to change this name to "In" and not "Im" -- To view, visit http://gerrit.ovirt.org/6379 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77565ffd66134b44d15f66cfdfa97422e3c11fb2 Gerrit-PatchSet: 3 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> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches