Gilad Chaplik has posted comments on this change.
Change subject: webadmin: Import VM dialog redesign (#845947)
......................................................................
Patch Set 2: Verified
(31 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 981: jobProperties = super.getJobMessageProperties();
Line 982: jobProperties.put(VdcObjectType.VM.name().toLowerCase(),
Line 983: (getVmName() == null) ? "" : getVmName());
Line 984:
jobProperties.put(VdcObjectType.VdsGroups.name().toLowerCase(),
getVdsGroupName());
Line 985: jobProperties.put(VdcObjectType.StoragePool.name(),
getStoragePoolName());
Done
Line 986: }
Line 987: return jobProperties;
Line 988: }
Line 989:
Line 1007: }
Line 1008:
Line 1009: private List<StorageQuotaValidationParameter>
getStorageQuotaListParameters() {
Line 1010: List<StorageQuotaValidationParameter> list = new
ArrayList<StorageQuotaValidationParameter>();
Line 1011: for (Disk disk :
getParameters().getVm().getDiskMap().values()) {
Done
Line 1012: if(disk instanceof DiskImage){
Line 1013: DiskImage diskImage = (DiskImage)disk;
Line 1014: list.add(new
StorageQuotaValidationParameter(diskImage.getQuotaId(),
Line 1015:
imageToDestinationDomainMap.get(diskImage.getId()),
....................................................
File
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 473: IMPORTEXPORT_EXPORT_VM=Vm ${VmName} was exported successfully to
${StorageDomainName}
Line 474: IMPORTEXPORT_EXPORT_VM_FAILED=Failed to export Vm ${VmName} to
${StorageDomainName}
Line 475: IMPORTEXPORT_STARTING_IMPORT_VM=Starting to import Vm ${VmName} to
${StoragePoolName}
Line 476: IMPORTEXPORT_IMPORT_VM=Vm ${VmName} was imported successfully to
${StoragePoolName}
Line 477: IMPORTEXPORT_IMPORT_VM_FAILED=Failed to import Vm ${VmName} to
${StoragePoolName}
who said?
Line 478: IMPORTEXPORT_STARTING_EXPORT_TEMPLATE=Starting to export Template
${VmTemplateName} to ${StorageDomainName}
Line 479: IMPORTEXPORT_EXPORT_TEMPLATE=Template ${VmTemplateName} was exported
successfully to ${StorageDomainName}
Line 480: IMPORTEXPORT_EXPORT_TEMPLATE_FAILED=Failed to export Template
${VmTemplateName} to ${StorageDomainName}
Line 481: IMPORTEXPORT_STARTING_IMPORT_TEMPLATE=Starting to import Template
${VmTemplateName} to ${StoragePoolName}
....................................................
Commit Message
Line 3: AuthorDate: 2012-08-15 09:08:46 +0300
Line 4: Commit: Gilad Chaplik <[email protected]>
Line 5: CommitDate: 2012-08-19 12:58:09 +0300
Line 6:
Line 7: webadmin: Import VM dialog redesign (#845947)
Done, removed backend stuff from this patch.
Line 8:
Line 9: https://bugzilla.redhat.com/845947
Line 10: https://bugzilla.redhat.com/827097
Line 11: https://bugzilla.redhat.com/848933
Line 19: - includes a dialog for conflicting vm (vm that already exist in
Line 20: the system)
Line 21: - refactoring the way the dialog works (fetching all the data when
dialog loads)
Line 22: - fixing paper cuts bug in the dialog (mainly wrong messages)
Line 23: - adding quota according to multiple storage domain (per disk)
not sure I can do it.
Line 24:
Line 25: Change-Id: I1b6551b235c66e6a32db0f8842cd8e410d6e0318
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java
Line 55:
Line 56: @DefaultStringValue("Storage")
Line 57: String singleDestinationStorage();
Line 58:
Line 59: @DefaultStringValue("Default Storage")
didn't quite get you
Line 60: String defaultStorage();
Line 61:
Line 62: @DefaultStringValue(" and Quota")
Line 63: String singleQuota();
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java
Line 165: getEntity().getId());
Line 166: }
Line 167:
Line 168: @Override
Line 169: protected void Restore() {
I don't see any relation between method name to the name displayed in UI.
can be in clean-up.
Line 170: super.Restore();
Line 171:
Line 172: if (getWindow() != null) {
Line 173: return;
Line 210: }
Line 211: cloneVmMap = new HashMap<Guid, VM>();
Line 212:
Line 213: vmsInSetupMap = new HashMap<Guid, VM>();
Line 214: if (importModel.hasVmInSystem()) {
Done, removed 'if'
Line 215: for (VM vm : (ArrayList<VM>) importModel.getItems()) {
Line 216: if (importModel.isObjectInSetup(vm)) {
Line 217: vmsInSetupMap.put(vm.getId(), vm);
Line 218: }
Line 220: }
Line 221: excuteImportClone();
Line 222: }
Line 223:
Line 224: private void excuteImportClone() {
Done
Line 225: if (vmsInSetupMap.size() == 0) {
Line 226: excuteImport();
Line 227: return;
Line 228: }
Line 223:
Line 224: private void excuteImportClone() {
Line 225: if (vmsInSetupMap.size() == 0) {
Line 226: excuteImport();
Line 227: return;
no, there are 2 flows for this code:
1) onRestore (click ok in the import dialog)
1) onClone (click ok in the conflict dialog)
therefore it should be here.
Line 228: }
Line 229: ImportCloneModel entity = new ImportCloneModel();
Line 230: VM vm = (VM) vmsInSetupMap.values().toArray()[0];
Line 231: entity.setEntity(vm);
Line 225: if (vmsInSetupMap.size() == 0) {
Line 226: excuteImport();
Line 227: return;
Line 228: }
Line 229: ImportCloneModel entity = new ImportCloneModel();
I've created some-kind of a loop (on vmsInSetupMap.size()) once we deal with a
vm in the map, we remove if from the map, and continue handling till the map is
empty.
Line 230: VM vm = (VM) vmsInSetupMap.values().toArray()[0];
Line 231: entity.setEntity(vm);
Line 232: entity.setTitle("Import Virtual Machine Conflict");
//$NON-NLS-1$
Line 233: entity.setHashName("import_virtual_machine_conflict");
//$NON-NLS-1$
Line 248: ImportCloneModel cloneModel = (ImportCloneModel)
getConfirmWindow();
Line 249: if ((Boolean) cloneModel.getApplyToAll().getEntity()) {
Line 250: if (!(Boolean) cloneModel.getNoClone().getEntity()) {
Line 251: String suffix = (String)
cloneModel.getSuffix().getEntity();
Line 252: if (!validateSuffix(suffix, cloneModel.getSuffix())) {
future?
Line 253: return;
Line 254: }
Line 255: for (VM vm : vmsInSetupMap.values()) {
Line 256: vm.setvm_name(vm.getvm_name() + suffix);
Line 261: } else {
Line 262: VM vm = (VM) cloneModel.getEntity();
Line 263: if (!(Boolean) cloneModel.getNoClone().getEntity()) {
Line 264: String vmName = (String)
cloneModel.getName().getEntity();
Line 265: if (!validateVmName(vmName, vm.getos(),
cloneModel.getName())) {
iiuc, it's already supported
Line 266: return;
Line 267: }
Line 268:
Line 269: vm.setvm_name(vmName);
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportData.java
Line 42: this.selectedQuota = selectedQuota;
Line 43: }
Line 44:
Line 45: public VolumeType getSelectedVolumeType() {
Line 46: if (!(Boolean) collapseSnapshots.getEntity() ||
selectedVolumeType == null) {
this is the get method, and not the set, if it returns 'null' we return the
default
Line 47: return getVolumeType();
Line 48: }
Line 49: return selectedVolumeType;
Line 50: }
Line 64: this.storageDomains = storageDomains;
Line 65: }
Line 66:
Line 67: public storage_domains getSelectedStorageDomain() {
Line 68: if (selectedStorageDomain == null &&
!storageDomains.isEmpty()) {
it can empty no sure if null... (having a null check is always better than no
check at all).
Line 69: selectedStorageDomain = storageDomains.get(0);
Line 70: }
Line 71:
Line 72: return selectedStorageDomain;
Line 76: this.selectedStorageDomain = selectedStorageDomain;
Line 77: }
Line 78:
Line 79: public void setSelectedStorageDomainString(String value) {
Line 80: for (storage_domains storageDomain : getStorageDomains()) {
afaik, it can't be null
Line 81: if (storageDomain.getstorage_name().equals(value)) {
Line 82: setSelectedStorageDomain(storageDomain);
Line 83: break;
Line 84: }
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmModel.java
Line 148: super.setSelectedItem(value);
Line 149: OnEntityChanged();
Line 150: }
Line 151:
Line 152: public boolean hasVmInSystem() {
done
Line 153: if (alreadyInSystemVmMap == null ||
alreadyInSystemVmMap.size() == 0) {
Line 154: return false;
Line 155: }
Line 156: return true;
Line 173: @Override
Line 174: public void OnSuccess(Object model, Object returnValue) {
Line 175: ArrayList<storage_pool> pools =
(ArrayList<storage_pool>) returnValue;
Line 176: storage_pool dataCenter = null;
Line 177: if (pools != null && pools.size() > 0) {
Done
Line 178: dataCenter = pools.get(0);
Line 179: } else {
Line 180: //TODO internal client ERROR
Line 181: }
Line 213: }
Line 214: }
Line 215:
Line 216:
getStorage().setItems(filteredStorageDomain);
Line 217: if (hasQuota) {
Done
Line 218: ArrayList<VdcQueryType>
queryTypeList = new ArrayList<VdcQueryType>();
Line 219:
ArrayList<VdcQueryParametersBase> queryParamsList =
Line 220: new
ArrayList<VdcQueryParametersBase>();
Line 221: for (storage_domains
storage : filteredStorageDomain) {
Line 270: setMessage(isDefaultStorageApplicableForAllDisks ? "" :
Line 271:
ConstantsManager.getInstance().getConstants().importNotApplicableForDefaultStorage());
Line 272: }
Line 273:
Line 274: private void checkForProblematicItems() {
Done
Line 275: for (Object item : getItems()) {
Line 276: VM vm = (VM) item;
Line 277: if (vm.getDiskMap() != null) {
Line 278: for (Map.Entry<Guid, Disk> pair :
vm.getDiskMap().entrySet()) {
Line 282: && getDiskImportData(disk.getId()) != null
Line 283: &&
(getDiskImportData(disk.getId()).getSelectedStorageDomain()
Line 284: .getstorage_type() ==
StorageType.ISCSI ||
Line 285:
getDiskImportData(disk.getId()).getSelectedStorageDomain()
Line 286: .getstorage_type() ==
StorageType.FCP)) {
Done
Line 287: getProblematicItems().add(vm);
Line 288: break;
Line 289: }
Line 290: }
Line 283: &&
(getDiskImportData(disk.getId()).getSelectedStorageDomain()
Line 284: .getstorage_type() ==
StorageType.ISCSI ||
Line 285:
getDiskImportData(disk.getId()).getSelectedStorageDomain()
Line 286: .getstorage_type() ==
StorageType.FCP)) {
Line 287: getProblematicItems().add(vm);
?
Line 288: break;
Line 289: }
Line 290: }
Line 291: }
Line 290: }
Line 291: }
Line 292: }
Line 293: if (getProblematicItems().size() > 0) {
Line 294: if (getProblematicItems().size() ==
Linq.Count(getItems())) {
Done
Line 295: // All items are problematic.
Line 296: getCollapseSnapshots().setEntity(true);
Line 297: getCollapseSnapshots().setIsChangable(false);
Line 298:
Line 311:
Line 312: private void initDisksStorageDomainsList() {
Line 313: for (Object item : getItems()) {
Line 314: VM vm = (VM) item;
Line 315: if (!NGuid.Empty.equals(vm.getvmt_guid())) {
future? or derez (I'm not going to change that)
Line 316: templateDiskMap.put(vm.getvmt_guid(), new
ArrayList<Disk>(vm.getDiskMap().values()));
Line 317: } else {
Line 318: for (Disk disk : vm.getDiskMap().values()) {
Line 319: DiskImage diskImage = (DiskImage) disk;
Line 312: private void initDisksStorageDomainsList() {
Line 313: for (Object item : getItems()) {
Line 314: VM vm = (VM) item;
Line 315: if (!NGuid.Empty.equals(vm.getvmt_guid())) {
Line 316: templateDiskMap.put(vm.getvmt_guid(), new
ArrayList<Disk>(vm.getDiskMap().values()));
shouldn't be null, if so that's a bug.
Line 317: } else {
Line 318: for (Disk disk : vm.getDiskMap().values()) {
Line 319: DiskImage diskImage = (DiskImage) disk;
Line 320: setDiskImportData(diskImage.getId(),
Line 317: } else {
Line 318: for (Disk disk : vm.getDiskMap().values()) {
Line 319: DiskImage diskImage = (DiskImage) disk;
Line 320: setDiskImportData(diskImage.getId(),
Line 321: filteredStorageDomain,
diskImage.getvolume_type());
Done
Line 322: }
Line 323: }
Line 324: }
Line 325: if (!templateDiskMap.isEmpty()) {
Line 490: List<VM> vmList =
Line 491: (List<VM>) ((VdcQueryReturnValue)
returnValue).getReturnValue();
Line 492: alreadyInSystemVmMap = new HashMap<Guid,
VM>();
Line 493: for (VM vm : vmList) {
Line 494: alreadyInSystemVmMap.put(vm.getId(), vm);
we can open a bug for it (discuss with PM)
I've changed my mind.
Line 495: }
Line 496:
Line 497: setSuperItems(value);
Line 498: }
Line 518: public boolean isObjectInSetup(Object vm) {
Line 519: if (alreadyInSystemVmMap == null) {
Line 520: return false;
Line 521: }
Line 522: return alreadyInSystemVmMap.containsKey(((VM) vm).getId());
same.
Line 523: }
Line 524:
Line 525: public boolean isQuotaEnabled() {
Line 526: return getStoragePool() != null
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmImportDiskListModel.java
Line 51:
Line 52: public void setDiskStorageMap(HashMap<Guid, ArrayList<Guid>> value)
Line 53: {
Line 54: diskStorageMap = value;
Line 55: raiseDiskEvent();
Done
Line 56: }
Line 57:
Line 58: public void raiseDiskEvent() {
Line 59: OnPropertyChanged(new
PropertyChangedEventArgs("DiskStorageMap")); //$NON-NLS-1$
....................................................
File
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java
Line 1481:
Line 1482: @DefaultStringValue("Unassigned Logical Networks panel")
Line 1483: String unassignedLogicalNetworksPanel();
Line 1484:
Line 1485: @DefaultStringValue("Some Templates' disks which are missing.
Suggested solutions: "
future
Line 1486: + "1. Preserving Template-Based structure - "
Line 1487: + "Make sure the relevant Templates exist on the
relevant Data-Center and Import the VMs again."
Line 1488: + "2. Using non-Template-Based structure (less optimal
storage-wise) - "
Line 1489: + "Export the VMs again using the Collapse Snapshots
option.")
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/backup/ImportVmPopupView.java
Line 177: @Override
Line 178: public void onSelection(SelectionEvent<Integer>
event) {
Line 179: if (object != null) {
Line 180:
object.setActiveDetailModel(object.getDetailModels().get(event.getSelectedItem()));
Line 181: if (event.getSelectedItem() == 2) {
Done
Line 182: generalView.setMainTabSelectedItem((VM)
object.getSelectedItem());
Line 183: }
Line 184: }
Line 185: }
--
To view, visit http://gerrit.ovirt.org/7313
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b6551b235c66e6a32db0f8842cd8e410d6e0318
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Haim Ateya <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
