Allon Mureinik has posted comments on this change.
Change subject: [wip] providers: add the openstack image support
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(5 inline comments)
@Alissa - please take a look at getAllByStorage(String) - didn't we already add
something similar?
@Alon - does anything need to be done to the spec file?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/IsoDomainListSyncronizer.java
Line 214: List<RepoImage> problematicRepoFileList, final
ImageFileType imageType) {
Line 215: final RepoFileMetaDataDAO repoFileMetaDataDao =
repoStorageDom;
Line 216:
Line 217: Provider provider = providerDao.get(new
Guid(storageDomain.getStorage()));
Line 218: final OpenstackImageProviderProxy client =
ProviderProxyFactory.getInstance().create(provider);
wouldn't we want to cache this somewhere instead of creating it each time anew?
Line 219:
Line 220: Lock syncObject = getSyncObject(storageDomain.getId(),
imageType);
Line 221: try {
Line 222: syncObject.lock();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/OpenstackImageProviderProxy.java
Line 68: }
Line 69:
Line 70: @Override
Line 71: public void testConnection() {
Line 72: }
can't we do something a bit better here?
Line 73:
Line 74: @Override
Line 75: public List<? extends Certificate> getCertificateChain() {
Line 76: return null;
Line 95: // Storage domain dynamic
Line 96: StorageDomainDynamic domainDynamicEntry = new
StorageDomainDynamic();
Line 97: domainDynamicEntry.setId(domainStaticEntry.getId());
Line 98: domainDynamicEntry.setAvailableDiskSize(0);
Line 99: domainDynamicEntry.setUsedDiskSize(0);
why zeroes?
Line 100:
getDbFacade().getStorageDomainDynamicDao().save(domainDynamicEntry);
Line 101: }
Line 102:
Line 103: @Override
Line 108: // removing the static and dynamic storage domain entries
Line 109: for (StorageDomainStatic domainStaticEntry :
domainStaticEntries) {
Line 110:
getDbFacade().getStorageDomainDynamicDao().remove(domainStaticEntry.getId());
Line 111:
getDbFacade().getStorageDomainStaticDao().remove(domainStaticEntry.getId());
Line 112: }
this should be done in a transaction, IMHO
Line 113: }
Line 114:
Line 115: private Provider getProvider() {
Line 116: return provider;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/OpenstackImageProviderProperties.java
Line 1: package org.ovirt.engine.core.common.businessentities;
Line 2:
Line 3: public class OpenstackImageProviderProperties extends
TenantProviderProperties {
why do we need a new class for this?
why not just reuse TenantProviderProperties ?
Line 4:
Line 5: private static final long serialVersionUID = -3887979451360188295L;
Line 6:
Line 7: @Override
--
To view, visit http://gerrit.ovirt.org/15899
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I424998de6f6514d65938484fffa5405e35857e43
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
