Liron Aravot has posted comments on this change.
Change subject: core: export/import of diskless VM/VM with no snappable
disks(#838937)
......................................................................
Patch Set 3: (6 inline comments)
replied to abaron comments
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java
Line 183: getDisksBasedOnImage();
getDisksBasedOnImage() is an existing method that calls the method
ImagesHandler.filterImageDisks() and initiates a private member list of
DiskImages.I don't think that we need to change it at the moment because the
whole code refers to it's return value as list of DiskImages based on the
private member list.
Line 421: log.warn("ExportVmCommand::EndMoveVmCommand: Vm is null -
not performing full EndAction");
if the VM doesn't exist any more (so getVM won't load the VM from DB) we mark
to command as ended successfully - so basically the user might think that the
VM was exported and that the process is finished (will get a message that vm X
was exported) although we didn't perform any of the EndSuccessfully()
operations, the question is whether this behaviour is correct or not..returning
the setSucceded(true) for now, but i'm not sure that it should be there in case
of a non existing VM as the first place, what do you think?
Line 427: EndActionOnAllImageGroups();
will return it, until verification of why was it done even without existing
VM..because as you wrote, it was a bit odd to me as well.
Line 452: this.endDefaultOps(vm);
thanks, i'm aware of that - my thought was that it's relatively a short and
simple operation unlike copy disks it's fine to implement it synchronously. if
it's problematic in any way i'll change it.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 904: if (getVm() != null) {
agreed, this implementation isn't mine and it's just a result of the separation
of the original EndSuccefully method..will send another patch which will
replace it with your suggestion.
Line 907: UpdateVmImSpm();
will do
--
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
