Ayal Baron has posted comments on this change. Change subject: BZ#836161 - Rewrited deleteImage(). ......................................................................
Patch Set 2: I would prefer that you didn't submit this (7 inline comments) .................................................... Commit Message Line 3: AuthorDate: 2012-10-04 10:55:51 +0200 Line 4: Commit: Eduardo Warszawski <[email protected]> Line 5: CommitDate: 2012-10-21 23:59:27 +0200 Line 6: Line 7: BZ#836161 - Rewrited deleteImage(). s/Rewrited/Rewrite of/ Line 8: Line 9: Volume operations should be done at the SD level to avoid Line 10: redundant metadata operations of information that is Line 11: already present in the SD layout. Line 6: Line 7: BZ#836161 - Rewrited deleteImage(). Line 8: Line 9: Volume operations should be done at the SD level to avoid Line 10: redundant metadata operations of information that is to avoid retrieving static data multiple times from disk. Line 11: already present in the SD layout. Line 12: Line 13: Change-Id: I304ff5cd70186ffc9789cd1ac9337efa6c5ff695 .................................................... File vdsm/storage/blockSD.py Line 182: """ Line 183: # TODO: Change for zero 128 M chuncks and log. Line 184: # 128 M is the vdsm extent size default Line 185: cmd = tuple(constants.EXT_DD, "if=/dev/zero", Line 186: "of=%s" % os.path.join("/dev", sdUUID, volUUID)) This isn't running with the correct priority! Line 187: p = misc.execCmd(cmd, sudo=False, sync=False) Line 188: return p Line 189: Line 190: Line 215: except se.StorageException as e: Line 216: log.debug("SD %s, Image %s pre zeroing ops failed", sdUUID, imgUUID, Line 217: volUUIDs) Line 218: try: Line 219: lvm.changelv(sdUUID, volUUIDs, ("--permission", "rw")) need a comment with lvm bug number which explains why you did this separation once it's fixed, we'd know we can get rid of it. Line 220: except se.StorageException as e: Line 221: # Hope this only means that some volumes were already writable. Line 222: log.debug("IGNORED: %s", e) Line 223: # blank the volumes. Line 218: try: Line 219: lvm.changelv(sdUUID, volUUIDs, ("--permission", "rw")) Line 220: except se.StorageException as e: Line 221: # Hope this only means that some volumes were already writable. Line 222: log.debug("IGNORED: %s", e) this is still really bad Line 223: # blank the volumes. Line 224: zeroPs = {} Line 225: for volUUID in volUUIDs: Line 226: zeroPs[volUUID] = _zeroVolume(sdUUID, volUUID) Line 227: Line 228: # Yes, this is a potentially infinite loop. Kill the vdsm task. Line 229: last = time.time() Line 230: while len(zeroPs) > 0: Line 231: time.sleep(ZERO_SLEEP) need to change to poll Line 232: toDelete = [] Line 233: for volUUID, rc in _getZerosFinished(zeroPs): Line 234: if rc != 0: Line 235: log.warning("zero leftover: %s/%s rc = %s", sdUUID, volUUID, rc) Line 234: if rc != 0: Line 235: log.warning("zero leftover: %s/%s rc = %s", sdUUID, volUUID, rc) Line 236: else: Line 237: toDelete.append(volUUID) Line 238: log.debug("zeroed %s/%s will be deleted", sdUUID, volUUID) s/.*/%s/%s was zeroed and will be deleted / Line 239: if toDelete: Line 240: deleteVolumes(sdUUID, toDelete) Line 241: Line 242: now = time.time() -- To view, visit http://gerrit.ovirt.org/8506 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I304ff5cd70186ffc9789cd1ac9337efa6c5ff695 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Shu Ming <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
