Arik Hadas has posted comments on this change. Change subject: core: introduce import from external provider ......................................................................
Patch Set 60: (7 comments) https://gerrit.ovirt.org/#/c/33712/60/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromExternalProviderCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromExternalProviderCommand.java: Line 38: import org.ovirt.engine.core.compat.Guid; Line 39: Line 40: @DisableInPrepareMode Line 41: @NonTransactiveCommandAttribute(forceCompensation = true) Line 42: public class ImportVmFromExternalProviderCommand<T extends ImportVmFromExternalProviderParameters> extends ImportVmCommandBase<T> > maybe you wanted to extend ImportVmCommand? no, I don't want to extend ImportVmCommand. it is too complicated command (too many cases). I want to keep it simple. Line 43: implements QuotaStorageDependent { Line 44: Line 45: private static final Pattern VMWARE_DISK_NAME_PATTERN = Pattern.compile("\\[.*?\\] .*/(.*).vmdk"); Line 46: Line 54: Line 55: @Override Line 56: protected void init() { Line 57: super.init(); Line 58: setVm(getParameters().getVm()); > probably not needed as happens already in super Done Line 59: setVdsId(getParameters().getProxyHostId()); Line 60: setVdsGroupId(getParameters().getVdsGroupId()); Line 61: setStorageDomainId(getParameters().getDestDomainId()); Line 62: setStoragePoolId(getVdsGroup() != null ? getVdsGroup().getStoragePoolId() : null); Line 56: protected void init() { Line 57: super.init(); Line 58: setVm(getParameters().getVm()); Line 59: setVdsId(getParameters().getProxyHostId()); Line 60: setVdsGroupId(getParameters().getVdsGroupId()); > also Done Line 61: setStorageDomainId(getParameters().getDestDomainId()); Line 62: setStoragePoolId(getVdsGroup() != null ? getVdsGroup().getStoragePoolId() : null); Line 63: } Line 64: Line 158: Line 159: @Override Line 160: protected void addVmInterfaces() { Line 161: super.addVmInterfaces(); Line 162: for (VmNetworkInterface iface : getVm().getInterfaces()) { > this means the nics are not in the vm.devices list? right Line 163: VmDeviceUtils.addNetworkInterfaceDevice( Line 164: new VmDeviceId(iface.getId(), getVmId()), Line 165: iface.isPlugged(), Line 166: false); Line 237: setSucceeded(true); Line 238: } Line 239: Line 240: private void removeVm() { Line 241: runInternalActionWithTasksContext( > no lock on the vm here? no, the vm is locked and the lock is passed to the remove command Line 242: VdcActionType.RemoveVm, Line 243: new RemoveVmParameters(getVmId(), true)); Line 244: } Line 245: Line 270: @Override Line 271: public AuditLogType getAuditLogTypeValue() { Line 272: switch (getActionState()) { Line 273: case EXECUTE: Line 274: return AuditLogType.IMPORTEXPORT_STARTING_IMPORT_VM; > what about failure? Done Line 275: case END_FAILURE: Line 276: return AuditLogType.IMPORTEXPORT_IMPORT_VM_FAILED; Line 277: case END_SUCCESS: Line 278: default: https://gerrit.ovirt.org/#/c/33712/60/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java: Line 653: ACTION_TYPE_FAILED_LUNS_ALREADY_USED_BY_DISKS(ErrorType.CONFLICT), Line 654: ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST(ErrorType.BAD_PARAMETERS), Line 655: ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST(ErrorType.BAD_PARAMETERS), Line 656: ACTION_TYPE_FAILED_STORAGE_DOMAIN_AND_CLUSTER_IN_DIFFERENT_POOL(ErrorType.BAD_PARAMETERS), Line 657: ACTION_TYPE_FAILED_VDS_NOT_IN_DEST_STORAGE_POOL(ErrorType.BAD_PARAMETERS), > what about messages? Done Line 658: ACTION_TYPE_FAILED_STORAGE_CONNECTION_NOT_EXIST(ErrorType.BAD_PARAMETERS), Line 659: ACTION_TYPE_FAILED_STORAGE_CONNECTION_FOR_DOMAIN_NOT_EXIST(ErrorType.BAD_PARAMETERS), Line 660: ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS(ErrorType.CONFLICT), Line 661: ACTION_TYPE_FAILED_STORAGE_CONNECTION_FOR_DOMAIN_ALREADY_EXISTS(ErrorType.CONFLICT), -- To view, visit https://gerrit.ovirt.org/33712 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9328edf7b8f49aa7975e28f651183b1449030dd6 Gerrit-PatchSet: 60 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
