Nir Soffer has posted comments on this change. Change subject: vm: Add required information to replica dict ......................................................................
Patch Set 9: (4 comments) https://gerrit.ovirt.org/#/c/40024/9/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2990: """ Line 2991: Update the persisted copy of drive replica. Line 2992: """ Line 2993: if not srcDrive.isDiskReplicationInProgress(): Line 2994: raise RuntimeError("Disk '%s' does not have an ongoing " > Is this a common check worth unifying? (Does it deserve a proper exception? We have 2 calls - one check that you are not replicating, and one check that you are, so cannot unify. We can have specific error, but it is not very useful since we cannot handle it. Line 2995: "replication" % srcDrive.name) Line 2996: Line 2997: for device in self.conf["devices"]: Line 2998: if (device['type'] == hwclass.DISK Line 2993: if not srcDrive.isDiskReplicationInProgress(): Line 2994: raise RuntimeError("Disk '%s' does not have an ongoing " Line 2995: "replication" % srcDrive.name) Line 2996: Line 2997: for device in self.conf["devices"]: > I vaguely remember methods like: findDiskByName (or device/drive?) Cleaned up in https://gerrit.ovirt.org/40217 Line 2998: if (device['type'] == hwclass.DISK Line 2999: and device.get("name") == srcDrive.name): Line 3000: with self._confLock: Line 3001: device['diskReplicate'] = srcDrive.diskReplicate Line 3038: Line 3039: replica['device'] = 'disk' Line 3040: replica['format'] = 'cow' Line 3041: replica.setdefault('cache', drive.cache) Line 3042: replica.setdefault('propagateErrors', drive.propagateErrors) > Where are these coming from? (Engine iirc). Wouldn't this change the origin These come from engine or defaults when you create a drive. How can this change the original drive when I take the values from the original drive? If you the drive was created without propagateErrors, it would use some default. The replica copy the current value so the <driver> element of the replica will use the same values. When finishing the replication, we do not use the replica to update parameters, so it cannot effect the original drive. Line 3043: Line 3044: # First mark the disk as replicated, so if we crash after the volume is Line 3045: # prepared, we clean up properly in diskReplicateFinish. Line 3046: try: Line 3054: replica['path'] = self.cif.prepareVolumePath(replica) Line 3055: try: Line 3056: # Add information required during replication, and persist it Line 3057: # so migration can continue after vdsm crash. Line 3058: if utils.isBlockDevice(replica['path']): > I thought we had this logic somewhere else already (Drive class?). But the replica may use different storage, so we cannot use the drive object. I can move this to a function in the storage module in another patch if you like. Line 3059: replica['diskType'] = DISK_TYPE.BLOCK Line 3060: else: Line 3061: replica['diskType'] = DISK_TYPE.FILE Line 3062: self._updateDiskReplica(drive) -- To view, visit https://gerrit.ovirt.org/40024 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If28c9e6d084fd27857ff9da2c024d8798f375cbd Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
