Ayal Baron has posted comments on this change.

Change subject: BZ#836161 - Rewrite of deleteImage().
......................................................................


Patch Set 5: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/blockSD.py
Line 184:     size: size to be blanked in bytes.
Line 185:     """
Line 186:     # TODO: Change for zero 128 M chuncks and log.
Line 187:     # 128 M is the vdsm extent size default
Line 188:     BS = 512
issuing 512b writes will flood the storage with IOPS.
At a minimum this should be in 1M blocks.

ddWatchCopy already does all these things, in the least you should reuse 
similar logic.

Also, I'm not sure whether this should be ODIRECT (when possible) or flood the 
pagecache with zeros (seems redundant, but faster as kernel can aggregate IOs)
Line 189:     count = size / BS
Line 190:     cmd = tuple(constants.CMD_LOWPRIO)
Line 191:     cmd += tuple(constants.EXT_DD, "if=/dev/zero",
Line 192:                     "of=%s" % os.path.join("/dev", sdUUID, volUUID),


--
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: 5
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

Reply via email to