Liron Ar has posted comments on this change.
Change subject: core: OvfManager - package and functionality
......................................................................
Patch Set 4: (2 inline comments)
in second thought moving this makes us loose this important tool.
your choice is to pass an object which will contain your bll logic wrap it in a
way it would be usefull.
Allon Mureinik Patch Set 4: Roy - can you clarify your suggestion? I, for one,
am not … 6:07 PM
@Roy -
1.
can you please elaborate on your suggestion?
2.I don't agree that we are we missing here an important tool,
i think that the readers/writers and the "manager" should be separated -
1. readers/writers - "dummy" classes responsible for reading/writing the ovf
file and return the read data.
2. manager - should delegate calls to the readers/writers and encapsulate any
further needed bll logic for the operation - like deciding how to organize the
disks in the vm's diskmap/disk list/images list..etc.
The manager currently only delegates bll calls to the reader/writer and contain
bll logic - therefore i see it as the correct thing to move it out of the utils
package regardless of this change.
@Arik
guys, can you please explain what is the "bll logic" in OvfManager you're
referring to? is it anything else besides setting the VM id for each interface?
As you can see, in this patch i also wanted export the logic of "assembling"
the "rest" of the vm to the ovf manager.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfManager.java
Line 89:
Line 90: // add disk map
Line 91: Map<Guid, List<DiskImage>> images = ImagesHandler
Line 92: .getImagesLeaf(diskImages);
Line 93: for (Guid id : images.keySet()) {
actually it was changed by the automatic refactoring, i'll return it.. :)
Line 94: List<DiskImage> list = images.get(id);
Line 95: toReturn.getDiskMap().put(id, list.get(list.size() - 1));
Line 96: }
Line 97:
Line 91: Map<Guid, List<DiskImage>> images = ImagesHandler
Line 92: .getImagesLeaf(diskImages);
Line 93: for (Guid id : images.keySet()) {
Line 94: List<DiskImage> list = images.get(id);
Line 95: toReturn.getDiskMap().put(id, list.get(list.size() - 1));
the list shouldn't be empty as it should contain atleast one "image" per disk
Line 96: }
Line 97:
Line 98: return toReturn;
Line 99: }
--
To view, visit http://gerrit.ovirt.org/15626
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3bfc4094178d90e7b0583db4e427fc2b195a367d
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
