Maor Lipchuk has posted comments on this change. Change subject: core: Register all the OVF disks to Storage ......................................................................
Patch Set 4: (6 comments) http://gerrit.ovirt.org/#/c/32743/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java: Line 190: } Line 191: } Line 192: } Line 193: Line 194: protected List<OvfEntityData> getEntitiesFromStorageOvfDisk() { > method name no longer correct. done Line 195: List<OvfEntityData> ovfEntitiesFromTar = Collections.emptyList(); Line 196: // Get all unregistered disks. Line 197: List<Disk> unregisteredDisks = getBackend().runInternalQuery(VdcQueryType.GetUnregisteredDisks, Line 198: new GetUnregisteredDisksQueryParameters(getParameters().getStorageDomainId(), Line 247: ovfStoreDiskImage.setShareable(true); Line 248: RegisterDiskParameters registerDiskParams = new RegisterDiskParameters(ovfStoreDiskImage, getParameters().getStorageDomainId()); Line 249: String result = "succeeded"; Line 250: try { Line 251: if (!runInternalAction(VdcActionType.RegisterDisk, registerDiskParams, null).getSucceeded()) { > how does transactivity is handled here and how it aligns with the compensat I will call this method from an existing open trnasaction Line 252: result = "failed"; Line 253: } Line 254: } catch (Exception e) { Line 255: result = "failed"; Line 250: try { Line 251: if (!runInternalAction(VdcActionType.RegisterDisk, registerDiskParams, null).getSucceeded()) { Line 252: result = "failed"; Line 253: } Line 254: } catch (Exception e) { > when there is a sufficent contructor in the class the entire execution shou Sufficient ctor for example happened when we added context in the constructor, but this is not relevant since I already changed it as you asked. I do want to call this log for success also, since I want to know which OVF STORE disks were registered and which don't, this will make reading the log much more easy Line 255: result = "failed"; Line 256: log.error(e); Line 257: } Line 258: log.infoFormat("Register OVF_STORE disk id {0} for storage domain {1} has {2}", Line 254: } catch (Exception e) { Line 255: result = "failed"; Line 256: log.error(e); Line 257: } Line 258: log.infoFormat("Register OVF_STORE disk id {0} for storage domain {1} has {2}", > you'll see that in the log from the logging of the execution of the command This is more verbal and clear to read, it also indicate that this is OVF disk, if you will have parallel operation you can't be sure which disks succeeded and which didn't Line 259: ovfStoreDiskImage.getId(), Line 260: getParameters().getStorageDomainId(), Line 261: result); Line 262: } Line 298: * - A list of OVF_STORE disks Line 299: * @return A Pair which contains the best OVF disk to retrieve data from and its size. Line 300: */ Line 301: private Pair<DiskImage, Long> getLatestOVFDisk(List<DiskImage> ovfStoreDiskImages) { Line 302: Date foundOvfDiskUpdateDate = new Date(); > So the problem here is deeper, No, that is part of the logic, it is OR condition. We first find OVF_STORE disk which is updated, it we found at least one, then the date is changed to something practical. it also does not related to the change here, but making me think about it, I will add another patch that will add a comment of this logic so it will be more clear. Line 303: boolean isFoundOvfDiskUpdated = false; Line 304: Long size = 0L; Line 305: Disk ovfDisk = null; Line 306: for (DiskImage ovfStoreDisk : ovfStoreDiskImages) { Line 309: // Check which disks are of OVF_STORE Line 310: String diskDecription = ovfStoreDisk.getDescription(); Line 311: Map<String, Object> diskDescriptionMap; Line 312: try { Line 313: diskDescriptionMap = JsonHelper.jsonToMap(diskDecription); > registerAllOvfDisks I was thinking about that when implementing this, but I decided that for keeping the code more readable and simple, I prefer to only use the disks and use the json mapper twice. It doesn't reflect much on the performance, and it will save us from handling list of pairs and map which will be much more difficult to maintain Line 314: } catch (IOException e) { Line 315: log.warnFormat("Exception while generating json containing ovf store info. Exception: {0}", e); Line 316: continue; Line 317: } -- To view, visit http://gerrit.ovirt.org/32743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9fc6666a64efa16968935cfceb10e19b8a4e2eef Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Aravot <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
