Federico Simoncelli has posted comments on this change. Change subject: [wip] providers: add the openstack image support ......................................................................
Patch Set 4: (10 inline comments) .................................................... 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); Postponed. To be discussed. 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 24: Line 25: Line 26: public class OpenstackImageProviderProxy implements ProviderProxy { Line 27: Line 28: enum GlanceImageFormat { Because these are Glance specific formats that happen to have the same name. See: http://docs.openstack.org/developer/glance/formats.html I implemented only the ones I currently need/support. Line 29: RAW("raw"), Line 30: ISO("iso"), Line 31: COW("qcow2"); Line 32: Line 41: } Line 42: } Line 43: Line 44: enum GlanceImageContainer { Line 45: BARE("bare"); Same concept, this comes from: http://docs.openstack.org/developer/glance/formats.html I implemented only the one I'm able to deal with at the moment. Line 46: Line 47: private String value; Line 48: Line 49: GlanceImageContainer(String value) { Line 68: } Line 69: Line 70: @Override Line 71: public void testConnection() { Line 72: } Done Line 73: Line 74: @Override Line 75: public List<? extends Certificate> getCertificateChain() { Line 76: return null; Line 80: return DbFacade.getInstance(); Line 81: } Line 82: Line 83: @Override Line 84: public void onAddition() { 1. ui that I know of, rest too probably, 2. yes Line 85: // Storage domain static Line 86: StorageDomainStatic domainStaticEntry = new StorageDomainStatic(); Line 87: domainStaticEntry.setId(Guid.newGuid()); Line 88: domainStaticEntry.setStorage(provider.getId().toString()); 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); Because glance doesn't let you discover the available space, and we don't want to monitor it anyway. What do you suggest? keeping it null? I remember I had problems leaving it null (failing code around), so I set it to 0 and in the UI it is displayed as "NA". 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: } It is, the transaction is in the onAddition/onRemoval caller. The scope we have here is too limited, the transaction involves other actions. 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 { Because we might need additional properties along the way. It is also confusing seeing generics using TenantProviderProperties when dealing with OpenstackImageProvider. Introducing this in the future would be slightly harder (we'd need to go over the TenantProviderProperties usages and understand where to change it to OpenstackImageProviderProperties). Line 4: Line 5: private static final long serialVersionUID = -3887979451360188295L; Line 6: Line 7: @Override .................................................... File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/provider/ProviderDaoDbFacadeImpl.java Line 39: tenantName = networkProperties.getTenantName(); Line 40: pluginType = networkProperties.getPluginType().name(); Line 41: break; Line 42: case OPENSTACK_IMAGE: Line 43: OpenstackImageProviderProperties imageProperties = The base class exists and they both inherit from that (in this patch). Please elaborate what you are suggesting here. Line 44: (OpenstackImageProviderProperties) entity.getAdditionalProperties(); Line 45: tenantName = imageProperties.getTenantName(); Line 46: break; Line 47: default: .................................................... File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAO.java Line 24: * @param storage Line 25: * the storage value Line 26: * @return the domain Line 27: */ Line 28: List<StorageDomainStatic> getAllByStorage(String storage); Please check the sql query (that is provided in this patch as well). Line 29: Line 30: /** Line 31: * Retrieves all domains of the specified type for the specified pool. Line 32: * -- 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