Nir Soffer has posted comments on this change. Change subject: vm: Support replication to different storage type ......................................................................
Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/40185/4/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1036: self.log.debug("Requesting extension for the original drive: %s " Line 1037: "(domainID: %s, volumeID: %s)", Line 1038: vmDrive.name, vmDrive.domainID, vmDrive.volumeID) Line 1039: self.__extendDriveVolume(vmDrive, vmDrive.volumeID, Line 1040: volInfo['newSize']) > Is there a reason why you extend the replica before the original drive? Ha Extending the replica before the original drive is the existing behavior before this patch, I just added a comment in extendDriveVolume to make this more clear. I'm not sure why Federico did this, but since it seems to work for couple of years, I don't want to touch it at this time. We do get duplicate extend request even if all extensions succeeds, because we check the disk every 2 seconds, but the spm poll extend messages every 3 seconds, and extending takes some time. Duplicate extend messages are sent current to lvm, and lvm will fail to extend the volume since the current number of extents is equal to the requested number of extents. I'm not sure that having occasional duplicate extend message is a problem, and it is certainly better then having missing extend request. We probably need to think more about this later. Line 1041: Line 1042: def __extendDriveVolume(self, vmDrive, volumeID, newSize): Line 1043: volInfo = { Line 1044: 'domainID': vmDrive.domainID, Line 3073: drive.name, replica) Line 3074: self.cif.teardownVolumePath(replica) Line 3075: return errCode['replicaErr'] Line 3076: Line 3077: if drive.chunked or drive.replicaChunked: > If migrating from file->block, why would we want to extend drive here? This will not extend the drive - extendDriveVolume will extend the replica, and after the replica is extended, it will skip extension of the drive. This way we have the logic for extending in one place. Basically, I'm keeping the current flow, unless it must change. Line 3078: try: Line 3079: capacity, alloc, physical = self._getExtendInfo(drive) Line 3080: self.extendDriveVolume(drive, drive.volumeID, physical, Line 3081: capacity) -- To view, visit https://gerrit.ovirt.org/40185 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70eb93082af81fe54268ee0133d1252c0d537ca8 Gerrit-PatchSet: 4 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: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
