Liron Ar has posted comments on this change. Change subject: core: Adding import of unregistered VM template ......................................................................
Patch Set 13: (11 comments) http://gerrit.ovirt.org/#/c/27581/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java: Line 104: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_TYPE_ILLEGAL); Line 105: retVal = false; Line 106: } Line 107: Line 108: if (retVal && !isImagesAlreadyOnTarget()) { this is not very clear imo, perhaps atleast add a comment that when the images are on targer we currently import unregistered template. Line 109: // Set the template images from the Export domain and change each image id storage is to the import domain Line 110: GetAllFromExportDomainQueryParameters tempVar = new GetAllFromExportDomainQueryParameters(getParameters() Line 111: .getStoragePoolId(), getParameters().getSourceDomainId()); Line 112: VdcQueryReturnValue qretVal = getBackend().runInternalQuery( http://gerrit.ovirt.org/#/c/27581/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateFromConfigurationCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateFromConfigurationCommand.java: Line 20: import org.ovirt.engine.core.utils.log.LogFactory; Line 21: import org.ovirt.engine.core.utils.ovf.OvfReaderException; Line 22: Line 23: @LockIdNameAttribute Line 24: @NonTransactiveCommandAttribute(forceCompensation = true) compensation isn't being used Line 25: public class ImportVmTemplateFromConfigurationCommand<T extends ImportVmTemplateParameters> extends ImportVmTemplateCommand { Line 26: Line 27: private static final Log log = LogFactory.getLog(ImportVmFromConfigurationCommand.class); Line 28: private OvfEntityData ovfEntityData; Line 37: } Line 38: Line 39: @Override Line 40: public Guid getVmTemplateId() { Line 41: if (isImagesAlreadyOnTarget()) { can you explain the behavior here? why there's a difference? Line 42: return getParameters().getContainerId(); Line 43: } else { Line 44: return super.getVmTemplateId(); Line 45: } Line 46: } Line 47: @Override Line 48: protected boolean canDoAction() { Line 49: init(); Line 50: if (isImagesAlreadyOnTarget()) { can you elaborate how it's relevant for adding from configuration? adding from configuration doesn't include any storage operation. Line 51: if (ovfEntityData == null && !getParameters().isImportAsNewEntity()) { Line 52: // addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_UNSUPPORTED_OVF); Line 53: return false; Line 54: } Line 53: return false; Line 54: } Line 55: Line 56: setStorageDomainId(ovfEntityData.getStorageDomainId()); Line 57: if (!validate(new StorageDomainValidator(getStorageDomain()).isDomainExistAndActive())) { irrelevant Line 58: return false; Line 59: } Line 60: Line 61: if (!getStorageDomain().getStorageDomainType().isDataDomain()) { Line 57: if (!validate(new StorageDomainValidator(getStorageDomain()).isDomainExistAndActive())) { Line 58: return false; Line 59: } Line 60: Line 61: if (!getStorageDomain().getStorageDomainType().isDataDomain()) { irrelevant Line 62: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_TYPE_UNSUPPORTED, Line 63: String.format("$domainId %1$s", getParameters().getStorageDomainId()), Line 64: String.format("$domainType %1$s", getStorageDomain().getStorageDomainType())); Line 65: } Line 64: String.format("$domainType %1$s", getStorageDomain().getStorageDomainType())); Line 65: } Line 66: } Line 67: getParameters().setImages(getVmTemplate().getImages()); Line 68: return super.canDoAction(); see ImportVmFromConfiguration command, you should clear the disks and add the template as template with no disks. Line 69: } Line 70: Line 71: private void init() { Line 72: VmTemplate vmFromConfiguration; Line 117: getUnregisteredOVFDataDao().removeEntity(ovfEntityData.getEntityId(), ovfEntityData.getStorageDomainId()); Line 118: } Line 119: setActionReturnValue(getVmTemplate().getId()); Line 120: } Line 121: since when do we support attaching disks to a template? Line 122: private AuditLogType attemptToAttachDisksToImportedVm(Collection<Disk> disks) { Line 123: List<String> failedDisks = new LinkedList<>(); Line 124: for (Disk disk : disks) { Line 125: AttachDettachVmDiskParameters params = new AttachDettachVmDiskParameters(getVm().getId(), Line 118: } Line 119: setActionReturnValue(getVmTemplate().getId()); Line 120: } Line 121: Line 122: private AuditLogType attemptToAttachDisksToImportedVm(Collection<Disk> disks) { /attemptToAttachDisksToImportedVm/attemptToAttachDisksToImportedTemplate Line 123: List<String> failedDisks = new LinkedList<>(); Line 124: for (Disk disk : disks) { Line 125: AttachDettachVmDiskParameters params = new AttachDettachVmDiskParameters(getVm().getId(), Line 126: disk.getId(), disk.getPlugged(), disk.getReadOnly()); Line 121: Line 122: private AuditLogType attemptToAttachDisksToImportedVm(Collection<Disk> disks) { Line 123: List<String> failedDisks = new LinkedList<>(); Line 124: for (Disk disk : disks) { Line 125: AttachDettachVmDiskParameters params = new AttachDettachVmDiskParameters(getVm().getId(), getVm()? Line 126: disk.getId(), disk.getPlugged(), disk.getReadOnly()); Line 127: VdcReturnValueBase returnVal = getBackend().runInternalAction(VdcActionType.AttachDiskToVm, params); Line 128: if (!returnVal.getSucceeded()) { Line 129: failedDisks.add(disk.getDiskAlias()); http://gerrit.ovirt.org/#/c/27581/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfHelper.java: Line 75: vmTemplate.setInterfaces(interfaces); Line 76: Line 77: // add disk map Line 78: Map<Guid, List<DiskImage>> images = ImagesHandler Line 79: .getImagesLeaf(diskImages); there are no snapshots for template disks, is it relevant? Line 80: for (Map.Entry<Guid, List<DiskImage>> entry : images.entrySet()) { Line 81: List<DiskImage> list = entry.getValue(); Line 82: vmTemplate.getDiskTemplateMap().put(entry.getKey(), list.get(list.size() - 1)); Line 83: } -- To view, visit http://gerrit.ovirt.org/27581 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fc184e687cbeebf4aa732c901f98222d5b11097 Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
