Federico Simoncelli has uploaded a new change for review.

Change subject: block: simplify zeroImgVolumes using threads ......................................................................

block: simplify zeroImgVolumes using threads

Change-Id: I665d863085842de8ff93c9aacf7c26277dfb031d Signed-off-by: Federico Simoncelli fsimonce@redhat.com --- M vdsm/storage/blockSD.py 1 file changed, 26 insertions(+), 72 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/35630/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 36078d0..daaa94f 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -22,7 +22,6 @@ import threading import logging import signal -import select import errno import re from StringIO import StringIO @@ -50,6 +49,7 @@ import resourceManager as rm import mount import supervdsm as svdsm +from threadLocal import vars import volume

STORAGE_DOMAIN_TAG = "RHAT_storage_domain" @@ -200,80 +200,34 @@ lvm.removeLVs(sdUUID, vols)

-def _zeroVolume(sdUUID, volUUID): - """Fill a block volume. - - This function requires an active LV. - """ - dm = lvm.lvDmDev(sdUUID, volUUID) - size = multipath.getDeviceSize(dm) # Bytes - # TODO: Change for zero 128 M chuncks and log. - # 128 M is the vdsm extent size default - BS = constants.MEGAB # 1024 ** 2 = 1 MiB - count = size / BS - cmd = [constants.EXT_DD, "oflag=%s" % misc.DIRECTFLAG, "if=/dev/zero", - "of=%s" % lvm.lvPath(sdUUID, volUUID), "bs=%s" % BS, - "count=%s" % count] - p = misc.execCmd(cmd, sync=False, nice=utils.NICENESS.HIGH, - ioclass=utils.IOCLASS.IDLE, deathSignal=signal.SIGKILL) - return p - - def zeroImgVolumes(sdUUID, imgUUID, volUUIDs): - ProcVol = namedtuple("ProcVol", "proc, vol") - # Put a sensible value for dd zeroing a 128 M or 1 G chunk and lvremove - # spent time. - ZEROING_TIMEOUT = 60000 # [miliseconds] - log.debug("sd: %s, LVs: %s, img: %s", sdUUID, volUUIDs, imgUUID) - # Following call to changelv is separate since setting rw permission on an - # LV fails if the LV is already set to the same value, hence we would not - # be able to differentiate between a real failure of deltag/addtag and one - # we would like to ignore (permission is the same) - try: - lvm.changelv(sdUUID, volUUIDs, ("--permission", "rw")) - except se.StorageException as e: - # Hope this only means that some volumes were already writable. - log.debug("Ignoring failed permission change: %s", e) - # blank the volumes. - zerofds = {} - poller = select.poll() - for volUUID in volUUIDs: - proc = _zeroVolume(sdUUID, volUUID) - fd = proc.stdout.fileno() - zerofds[fd] = ProcVol(proc, volUUID) - poller.register(fd, select.EPOLLHUP) + taskid = vars.task.id + aborting = vars.task.aborting

- # Wait until all the asyncs procs return - # Yes, this is a potentially infinite loop. Kill the vdsm task. - while zerofds: - fdevents = poller.poll(ZEROING_TIMEOUT) # [(fd, event)] - toDelete = [] - for fd, event in fdevents: - proc, vol = zerofds[fd] - if not proc.wait(0): - continue - else: - poller.unregister(fd) - zerofds.pop(fd) - if proc.returncode != 0: - log.error("zeroing %s/%s failed. Zero and remove this " - "volume manually. rc=%s %s", sdUUID, vol, - proc.returncode, proc.stderr.read(1000)) - else: - log.debug("%s/%s was zeroed and will be deleted", - sdUUID, volUUID) - toDelete.append(vol) - if toDelete: - try: - deleteVolumes(sdUUID, toDelete) - except se.CannotRemoveLogicalVolume: - # TODO: Add the list of removed fail volumes to the exception. - log.error("Remove failed for some of VG: %s zeroed volumes: " - "%s", sdUUID, toDelete, exc_info=True) + def zeroVolume(volUUID): + log.debug('starting to zero volume %s on domain %s for ' + 'task %s', volUUID, sdUUID, taskid) + try: + lvm.changelv(sdUUID, volUUIDs, ("--permission", "rw")) + except se.StorageException as e: + # We ignore the failure hoping that the volumes were + # already writable. + log.debug("Ignoring failed permission change: %s", e)

- log.debug("finished with VG:%s LVs: %s, img: %s", sdUUID, volUUIDs, - imgUUID) - return + path = lvm.lvPath(sdUUID, volUUID) + size = multipath.getDeviceSize(lvm.lvDmDev(sdUUID, volUUID)) + + try: + misc.ddWatchCopy("/dev/zero", path, aborting, size) + except Exception as e: + log.exception('zeroing operation failed') + raise se.VolumesZeroingError(path) + + deleteVolumes(sdUUID, volUUID) + log.debug('zeroing of volume %s on domain %s for task %s ' + 'completed successfully', volUUID, sdUUID, taskid) + + misc.tmap(zeroVolume, volUUIDs)

class VGTagMetadataRW(object):