Sergey Gotliv has posted comments on this change. Change subject: engine: Illegal images should be skipped when validating duplicates ......................................................................
Patch Set 5: (5 comments) .................................................... Commit Message Line 5: CommitDate: 2013-09-20 23:42:53 +0300 Line 6: Line 7: engine: Illegal images should be skipped when validating duplicates Line 8: Line 9: Before starting to import VM from Template Engine validates that You are right! It should be as you says. I'll fix it in the next patch. Line 10: template images don't exist on the storage, if at least one of them Line 11: exists its probably would be better to clone VM instead of import, just Line 12: to prevent potential data loss. Engine assumes that its database Line 13: correctly represents the storage so this validation is done against .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java Line 638: public static List<DiskImage> getDisksFromDB(List<DiskImage> disks) { Line 639: List<DiskImage> disksInDB = new ArrayList<>(); Line 640: for (DiskImage disk : disks) { Line 641: DiskImage diskImage = DbFacade.getInstance().getDiskImageDao().get(disk.getId()); Line 642: if (diskImage != null) { If you search in DB something that doesn't exists there you get null. Please, remember that input disk list is taken from storage, now I am searching if any of them are already exist in DB (it can happen because of previous attempts to import the same VM/Template). Since java can't return 2 values I can't wrap it with "isExist" which will return boolean (status) and DiskImage. Line 643: disksInDB.add(diskImage); Line 644: } Line 645: } Line 646: return disksInDB; .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java Line 337: if (!validateNoDuplicateVm()) { Line 338: return false; Line 339: } Line 340: Line 341: if (!getParameters().isImportAsNewEntity() && !validateNoDuplicateDiskImages(imageList)) { isImportAsNewEntity = true - means user clones VM/TEMPLATE, in this case there is no reason validating dups. if (!getParameters().isImportAsNewEntity() && !validateNoDuplicateDiskImages(imageList)) means "validate dups if this is not a clone" Line 342: return false; Line 343: } Line 344: Line 345: if (!validateDiskInterface(imageList)) { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java Line 366: return true; Line 367: } Line 368: Line 369: protected boolean validateNoDuplicateDiskImages(List<DiskImage> diskImages) { Line 370: List<String> existingLegalDisksAliases = new ArrayList<String>(); Done Line 371: List<DiskImage> disksFromDB = ImagesHandler.getDisksFromDB(diskImages); Line 372: Line 373: if (disksFromDB.isEmpty()) { Line 374: return true; Line 369: protected boolean validateNoDuplicateDiskImages(List<DiskImage> diskImages) { Line 370: List<String> existingLegalDisksAliases = new ArrayList<String>(); Line 371: List<DiskImage> disksFromDB = ImagesHandler.getDisksFromDB(diskImages); Line 372: Line 373: if (disksFromDB.isEmpty()) { This condition checks if images attached to VM/Template in export domain are already exist in our DB, for example after previous attempt to import that VM/Template which is failed. Line 374: return true; Line 375: } Line 376: Line 377: for (DiskImage disk : disksFromDB) { -- To view, visit http://gerrit.ovirt.org/19246 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7b43e4b4e8e04dfb644a33ea9ce2a474987c5e3c Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> 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