liron aravot has posted comments on this change.
Change subject: core: introducing OvfAutoUpdate
......................................................................
Patch Set 18: (6 inline comments)
....................................................
File backend/manager/dbscripts/vms_sp.sql
Line 720: AS $procedure$
Line 721: BEGIN
Line 722: RETURN QUERY SELECT vm.*
Line 723: FROM vms vm
Line 724: WHERE vm.vm_guid in (SELECT ID from
fnSplitterUuid(v_vms_ids));
all over the code,
select X forces us to return only one column from the splitter method, why do
we need to have such contract between the implementation and the usage? i
prefer to leave it like that for uniformity.
Line 725: END; $procedure$
Line 726: LANGUAGE plpgsql;
Line 727:
Line 728:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfDataUpdater.java
Line 277: protected void buildMetadataDictionaryForTemplate(VmTemplate
template , Map<Guid, KeyValuePairCompat<String, List<Guid>>> metaDictionary){
Line 278: List<DiskImage> allTemplateImages = template.getDiskList();
Line 279: String templateMeta = generateVmTemplateMetadata(template,
allTemplateImages);
Line 280: metaDictionary.put(template.getId(), new
KeyValuePairCompat<String, List<Guid>>(
Line 281: templateMeta, LinqUtils.foreach(allTemplateImages,
new Function<DiskImage, Guid>() {
We need to create a list of guids only - so having it with a for will make this
code longer. that code was only moved here, i prefer to leave it like that in
this patch.
Line 282: @Override
Line 283: public Guid eval(DiskImage diskImage) {
Line 284: return diskImage.getId().getValue();
Line 285: }
Line 301: * loads additional need vm data for it's ovf
Line 302: * @param vm
Line 303: */
Line 304: protected void loadVmData(VM vm) {
Line 305: if (vm.getInterfaces().isEmpty()) {
That code was only moved here, i prefer to leave it like that in this patch as
it's called also from export command - it can be handled in a further patch, i
want to limit the scope of this patch to the addon of the ovf updater, not to
handle the exist ovf build logic.
Line 306:
vm.setInterfaces(getVmNetworkInterfaceDao().getAllForVm(vm.getId()));
Line 307: }
Line 308: if (StringUtils.isEmpty(vm.getVmtName())) {
Line 309: if (!Guid.Empty.equals(vm.getVmtGuid())) {
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/OvfDataUpdaterTest.java
Line 264: StoragePoolStatus.Maintanance.getValue());
Line 265: pools.add(pool1);
Line 266: pools.add(pool2);
Line 267: pools.add(pool3);
Line 268: return pools;
I prefer to have it like that as i want to have different pool statuses, what
will be added is verification that update methods weren't called with the pool
id which is in maintanance.
Line 269: }
Line 270:
Line 271: private VM createVm(Guid id, VMStatus status) {
Line 272: VM vm = new VM();
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/UpdateVMVDSCommandParameters.java
Line 4:
Line 5: import java.util.List;
Line 6: import java.util.HashMap;
Line 7: import java.util.Map;
Line 8:
ctor now gets a Map instead of HashMap, therefore privateInfoDictionary is now
declared as a Map
Line 9: public class UpdateVMVDSCommandParameters extends
StorageDomainIdParametersBase {
Line 10: public UpdateVMVDSCommandParameters(Guid storagePoolId,
Line 11: Map<Guid, KeyValuePairCompat<String, List<Guid>>>
infoDictionary) {
Line 12: super(storagePoolId);
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java
Line 192: return entity;
Line 193: }
Line 194: };
Line 195:
Line 196: return getCallsHandler().executeReadList("GetAllByStatus",
mapper, parameterSource);
I prefer to not have this patch dependent in other patch, i decided to add a
mapper for this method and will make use of it in other methods as well in a
further patch.
Line 197: }
Line 198:
Line 199: @Override
Line 200: public List<storage_pool> getAll(Guid userID, boolean isFiltered)
{
--
To view, visit http://gerrit.ovirt.org/9328
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b5132300fb1f1fd94f771cab15efe5246dbeca8
Gerrit-PatchSet: 18
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: liron aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches