Change in vdsm[master]: storage: Add inplace virt-sparsify support
smelamud at redhat.com
smelamud at redhat.com
Wed May 11 12:45:03 UTC 2016
Hello Shmuel Melamud,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/57347
to review the following change.
Change subject: storage: Add inplace virt-sparsify support
......................................................................
storage: Add inplace virt-sparsify support
Added support of sparsify operation (removing unused space from a disk
image) that is performed in-place (without copying content of an input
disk to an output disk) using virt-sparsify tool.
Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=734120
Signed-off-by: Shmuel Melamud <smelamud at redhat.com>
---
M client/vdsClient.py
M lib/api/vdsmapi-schema.json
M lib/vdsm/rpc/Bridge.py
M lib/vdsm/rpc/bindingxmlrpc.py
M lib/vdsm/storage/exception.py
M lib/vdsm/virtsparsify.py
M tests/crossImportsTests.py.in
M tests/sparsifyTests.py
M vdsm/API.py
M vdsm/storage/hsm.py
M vdsm/storage/image.py
M vdsm/storage/sp.py
12 files changed, 86 insertions(+), 201 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/47/57347/1
diff --git a/client/vdsClient.py b/client/vdsClient.py
index 6ea0283..e3d2778 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -1370,14 +1370,11 @@
return 0, image['uuid']
def sparsifyImage(self, args):
- (spUUID, tmpSdUUID, tmpImgUUID, tmpVolUUID, dstSdUUID, dstImgUUID,
- dstVolUUID) = args
- status = self.s.sparsifyImage(spUUID, tmpSdUUID, tmpImgUUID,
- tmpVolUUID, dstSdUUID, dstImgUUID,
- dstVolUUID)
+ (imgUUID, spUUID, sdUUID, volUUID) = args
+ status = self.s.sparsifyImage(imgUUID, spUUID, sdUUID, volUUID)
if status['status']['code']:
return status['status']['code'], status['status']['message']
- return 0, status['uuid']
+ return 0
def cloneImageStructure(self, args):
spUUID, sdUUID, imgUUID, dstSdUUID = args
diff --git a/lib/api/vdsmapi-schema.json b/lib/api/vdsmapi-schema.json
index 1da8cc0..7c7e750 100644
--- a/lib/api/vdsmapi-schema.json
+++ b/lib/api/vdsmapi-schema.json
@@ -4847,29 +4847,22 @@
##
# @Image.sparsify:
#
-# Create a copy of a sparse volume to a destination volume within the same
-# Storage Pool with free space removed from the source volume.
+# Perform an in-place sparse (e.g. not creating a new image) on given volume,
+# removing unused space from the volume.
#
-# @imageID: The UUID of the Image
+# @imageID: The UUID of the Volume
#
-# @storagepoolID: The UUID of the Storage Pool associated with the Image
+# @storagepoolID: The UUID of the Storage Pool associated with the Volume
#
-# @storagedomainID: The UUID of the Storage Domain associated with the Image
+# @storagedomainID: The UUID of the Storage Domain associated with the Volume
#
-# @tmpVolUUID: The UUID of the snapshot volume
+# @volumeUUID: The UUID of the Image that contains the Volume
#
-# @dstSdUUID: The UUID of the destination storage domain
-#
-# @dstImgUUID: The UUID of the destination image
-#
-# @dstVolUUID: The UUID of the destination volume
-#
-# Since: 4.17.0
+# Since: 4.18.0
##
{'command': {'class': 'Image', 'name': 'sparsify'},
'data': {'imageID': 'UUID', 'storagepoolID': 'UUID',
- 'storagedomainID': 'UUID', 'tmpVolUUID': 'UUID', 'dstSdUUID': 'UUID',
- 'dstImgUUID': 'UUID', 'dstVolUUID': 'UUID'}}
+ 'storagedomainID': 'UUID', 'volumeUUID': 'UUID'}}
##
# @Image.cloneStructure:
diff --git a/lib/vdsm/rpc/Bridge.py b/lib/vdsm/rpc/Bridge.py
index 6b106dd..636e06a 100644
--- a/lib/vdsm/rpc/Bridge.py
+++ b/lib/vdsm/rpc/Bridge.py
@@ -427,6 +427,7 @@
'Image_mergeSnapshots': {'ret': 'uuid'},
'Image_move': {'ret': 'uuid'},
'Image_reconcileVolumeChain': {'ret': 'volumes'},
+ 'Image_sparsify': {},
'Image_syncData': {'ret': 'uuid'},
'Image_upload': {'ret': 'uuid'},
'Image_prepare': {'ret': Image_prepare_Ret},
diff --git a/lib/vdsm/rpc/bindingxmlrpc.py b/lib/vdsm/rpc/bindingxmlrpc.py
index 9945dc7..b9b6a8b 100644
--- a/lib/vdsm/rpc/bindingxmlrpc.py
+++ b/lib/vdsm/rpc/bindingxmlrpc.py
@@ -746,10 +746,9 @@
image = API.Image(imgUUID, spUUID, srcDomUUID)
return image.move(dstDomUUID, op, postZero, force)
- def imageSparsify(self, spUUID, sdUUID, imgUUID, tmpVolUUID, dstSdUUID,
- dstImgUUID, dstVolUUID):
+ def imageSparsify(self, imgUUID, spUUID, sdUUID, volUUID):
image = API.Image(imgUUID, spUUID, sdUUID)
- return image.sparsify(tmpVolUUID, dstSdUUID, dstImgUUID, dstVolUUID)
+ return image.sparsify(volUUID)
def imageCloneStructure(self, spUUID, sdUUID, imgUUID, dstSdUUID):
image = API.Image(imgUUID, spUUID, sdUUID)
diff --git a/lib/vdsm/storage/exception.py b/lib/vdsm/storage/exception.py
index 23e2628..3924b06 100644
--- a/lib/vdsm/storage/exception.py
+++ b/lib/vdsm/storage/exception.py
@@ -336,11 +336,6 @@
message = "Volume type is not sparse"
-class CannotSparsifyVolume(StorageException):
- code = 234
- message = "Cannot sparsify volume"
-
-
#################################################
# Images Exceptions
#################################################
diff --git a/lib/vdsm/virtsparsify.py b/lib/vdsm/virtsparsify.py
index 44ed778..a8b08ce 100644
--- a/lib/vdsm/virtsparsify.py
+++ b/lib/vdsm/virtsparsify.py
@@ -1,5 +1,5 @@
#
-# Copyright 2014 Red Hat, Inc.
+# Copyright 2016 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -19,39 +19,78 @@
#
from __future__ import absolute_import
+from contextlib import contextmanager
+import logging
import signal
+from uuid import uuid4
-from . import cmdutils
from . import commands
+from . import concurrent
+from . import jobs
from . import utils
+from . import response
# Fedora, EL6
_VIRTSPARSIFY = utils.CommandPath("virt-sparsify",
"/usr/bin/virt-sparsify",)
-def sparsify(src_vol, tmp_vol, dst_vol, src_format=None, dst_format=None):
+def sparsify(imgUUID, sdUUID, spUUID, volUUID, irs):
"""
- Sparsify the 'src_vol' volume (src_format) to 'dst_vol' volume (dst_format)
- using libguestfs virt-sparsify
-
- src_vol: path of base volume
- tmp_vol: path of temporary volume created with src_vol as backing volume
- dst_vol: path of destination volume
- src_format: format of base volume ('raw' or `qcow2')
- src_format: format of destination volume ('raw' or `qcow2')
+ Sparsify the volume in place
+ (without copying from an input disk to an output disk)
"""
- cmd = [_VIRTSPARSIFY.cmd, '--tmp', 'prebuilt:' + tmp_vol]
+ job = SparsifyJob(imgUUID, sdUUID, spUUID, volUUID, irs)
+ job.start()
+ jobs.add(job)
- if src_format:
- cmd.extend(("--format", src_format))
- if dst_format:
- cmd.extend(("--convert", dst_format))
+def delete_sparsify_job(job_id):
+ return jobs.delete(job_id)
- cmd.extend((src_vol, dst_vol))
- rc, out, err = commands.execCmd(cmd, deathSignal=signal.SIGKILL)
+def abort_sparsify_job(job_id):
+ return jobs.abort(job_id)
- if rc != 0:
- raise cmdutils.Error(cmd, rc, out, err)
+
+ at contextmanager
+def volume(imgUUID, sdUUID, spUUID, volUUID, irs):
+ res = irs.prepareImage(sdUUID, spUUID, imgUUID, volUUID)
+ if response.is_error(res):
+ raise Exception('Cannot find volume path for %r' % volUUID)
+ try:
+ yield res['path']
+ finally:
+ try:
+ irs.teardownImage(sdUUID, spUUID, imgUUID)
+ except Exception:
+ logging.exception('Error tearing down image: %r', imgUUID)
+ raise
+
+
+class SparsifyJob(jobs.Job):
+ def __init__(self, imgUUID, sdUUID, spUUID, volUUID, irs):
+ self._imgUUID = imgUUID
+ self._sdUUID = sdUUID
+ self._spUUID = spUUID
+ self._volUUID = volUUID
+ self._irs = irs
+ self._id = uuid4()
+ self._thread = None
+
+ def start(self):
+ self._thread = concurrent.thread(self._run)
+ self._thread.start()
+
+ def _run(self):
+ with volume(self._imgUUID, self._sdUUID, self._spUUID,
+ self._volUUID, self._irs) as path:
+ cmd = [_VIRTSPARSIFY.cmd, '--machine-readable', '--in-place', path]
+ logging.info('>>> cmd: %r', cmd)
+ rc, out, err = commands.execCmd(cmd,
+ deathSignal=signal.SIGTERM,
+ env={'LIBGUESTFS_BACKEND':
+ 'direct'})
+ logging.info('>>> rc: %r', rc)
+ logging.info('>>> out: %r', out)
+ logging.info('>>> err: %r', err)
diff --git a/tests/crossImportsTests.py.in b/tests/crossImportsTests.py.in
index 225f81a..87555e7 100644
--- a/tests/crossImportsTests.py.in
+++ b/tests/crossImportsTests.py.in
@@ -51,7 +51,6 @@
# some random samples. Some names must include 'p', 'y', 'c'
Mod("response.py", "response"),
Mod("cmdutils.py", "cmdutils"),
- Mod("virtsparsify.py", "virtsparsify"),
Mod("pthread.py", "pthread"),
Mod("constants.py", "constants"),
Mod("compat.py", "compat"),
diff --git a/tests/sparsifyTests.py b/tests/sparsifyTests.py
index f99b0c8..81bf477 100644
--- a/tests/sparsifyTests.py
+++ b/tests/sparsifyTests.py
@@ -1,5 +1,5 @@
#
-# Copyright 2015 Red Hat, Inc.
+# Copyright 2016 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -18,26 +18,8 @@
# Refer to the README and COPYING files for full details of the license
#
-from vdsm import cmdutils
-from vdsm import virtsparsify
-
-from monkeypatch import MonkeyPatch
-from testlib import VdsmTestCase as TestCaseBase
-
-
-FAKE_VOLUME = '/we/dont/care/about/this/path'
-
-
-class FakeCommand(object):
- # as per manpage, false ignores any argument
- cmd = '/bin/false'
-
-
-class VirtSparsifyTests(TestCaseBase):
-
- @MonkeyPatch(virtsparsify, '_VIRTSPARSIFY', FakeCommand())
- def test_raise_error_on_failure(self):
-
- self.assertRaises(cmdutils.Error,
- virtsparsify.sparsify,
- FAKE_VOLUME, FAKE_VOLUME, FAKE_VOLUME)
+# from vdsm import cmdutils
+# from vdsm import virtsparsify
+#
+# from monkeypatch import MonkeyPatch
+# from testlib import VdsmTestCase as TestCaseBase
diff --git a/vdsm/API.py b/vdsm/API.py
index e7aac09..019db0c 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -908,10 +908,9 @@
self._UUID, vmUUID, operation, postZero,
force)
- def sparsify(self, tmpVolUUID, dstSdUUID, dstImgUUID, dstVolUUID):
- return self._irs.sparsifyImage(self._spUUID, self._sdUUID, self._UUID,
- tmpVolUUID, dstSdUUID, dstImgUUID,
- dstVolUUID)
+ def sparsify(self, volumeUUID):
+ return self._irs.sparsifyImage(self._UUID, self._spUUID, self._sdUUID,
+ volumeUUID, self._irs)
def cloneStructure(self, dstSdUUID):
return self._irs.cloneImageStructure(self._spUUID, self._sdUUID,
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index bd33b25..13e0305 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -78,6 +78,7 @@
import devicemapper
import dispatcher
import storageServer
+import virtsparsify
import sdm.api.create_volume
@@ -1586,23 +1587,14 @@
misc.parseBool(force))
@public
- def sparsifyImage(self, spUUID, tmpSdUUID, tmpImgUUID, tmpVolUUID,
- dstSdUUID, dstImgUUID, dstVolUUID):
+ def sparsifyImage(self, imgUUID, spUUID, sdUUID, volUUID, irs):
"""
Reduce sparse image size by converting free space on image to free
space on storage domain using virt-sparsify.
"""
- pool = self.getPool(spUUID)
-
- sdUUIDs = sorted(set((tmpSdUUID, dstSdUUID)))
-
- for dom in sdUUIDs:
- vars.task.getSharedLock(STORAGE, dom)
-
- self._spmSchedule(spUUID, "sparsifyImage", pool.sparsifyImage,
- tmpSdUUID, tmpImgUUID, tmpVolUUID, dstSdUUID,
- dstImgUUID, dstVolUUID)
+ virtsparsify.sparsify(imgUUID, spUUID, sdUUID, volUUID, irs)
+ # TODO use self instead of irs?
@public
def cloneImageStructure(self, spUUID, sdUUID, imgUUID, dstSdUUID):
diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 381b2a1..4a52428 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -26,7 +26,6 @@
import volume
from vdsm import qemuimg
-from vdsm import virtsparsify
from vdsm.config import config
from vdsm.storage import exception as se
from vdsm.storage import fileUtils
@@ -582,74 +581,6 @@
self.log.info("%s task on image %s was successfully finished",
OP_TYPES[op], imgUUID)
return True
-
- def _getSparsifyVolume(self, sdUUID, imgUUID, volUUID):
- # FIXME:
- # sdCache.produce.produceVolume gives volumes that return getVolumePath
- # with a colon (:) for NFS servers. So, we're using volClass().
- # https://bugzilla.redhat.com/1128942
- # If, and when the bug gets solved, use
- # sdCache.produce(...).produceVolume(...) to create the volumes.
- volClass = sdCache.produce(sdUUID).getVolumeClass()
- return volClass(self.repoPath, sdUUID, imgUUID, volUUID)
-
- def sparsify(self, tmpSdUUID, tmpImgUUID, tmpVolUUID, dstSdUUID,
- dstImgUUID, dstVolUUID):
- """
- Reduce sparse image size by converting free space on image to free
- space on storage domain using virt-sparsify.
- """
- self.log.info("tmpSdUUID=%s, tmpImgUUID=%s, tmpVolUUID=%s, "
- "dstSdUUID=%s, dstImgUUID=%s, dstVolUUID=%s", tmpSdUUID,
- tmpImgUUID, tmpVolUUID, dstSdUUID, dstImgUUID,
- dstVolUUID)
-
- tmpVolume = self._getSparsifyVolume(tmpSdUUID, tmpImgUUID, tmpVolUUID)
- dstVolume = self._getSparsifyVolume(dstSdUUID, dstImgUUID, dstVolUUID)
-
- if not dstVolume.isSparse():
- raise se.VolumeNotSparse()
-
- srcVolume = self._getSparsifyVolume(tmpSdUUID, tmpImgUUID,
- tmpVolume.getParent())
-
- tmpVolume.prepare()
- try:
- dstVolume.prepare()
- try:
- # By definition "sparsification" is implemented writing a file
- # with zeroes as large as the entire file-system. So at least
- # tmpVolume needs to be as large as the virtual disk size for
- # the worst case.
- # TODO: Some extra space may be needed for QCOW2 headers
- tmpVolume.extend(tmpVolume.getSize())
- # For the dstVolume we may think of an optimization where the
- # extension is as large as the source (and at the end we
- # shrinkToOptimalSize).
- # TODO: Extend the dstVolume only as much as the actual size of
- # srcVolume
- # TODO: Some extra space may be needed for QCOW2 headers
- dstVolume.extend(tmpVolume.getSize())
-
- srcFormat = volume.fmt2str(srcVolume.getFormat())
- dstFormat = volume.fmt2str(dstVolume.getFormat())
-
- virtsparsify.sparsify(srcVolume.getVolumePath(),
- tmpVolume.getVolumePath(),
- dstVolume.getVolumePath(),
- src_format=srcFormat,
- dst_format=dstFormat)
- except Exception:
- self.log.exception('Unexpected error sparsifying %s',
- tmpVolUUID)
- raise se.CannotSparsifyVolume(tmpVolUUID)
- finally:
- dstVolume.teardown(sdUUID=dstSdUUID, volUUID=dstVolUUID)
- finally:
- tmpVolume.teardown(sdUUID=tmpSdUUID, volUUID=tmpVolUUID)
-
- tmpVolume.shrinkToOptimalSize()
- dstVolume.shrinkToOptimalSize()
def cloneStructure(self, sdUUID, imgUUID, dstSdUUID):
self._createTargetImage(sdCache.produce(dstSdUUID), sdUUID, imgUUID)
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index af838de..da4e320 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -1616,48 +1616,6 @@
image.Image(self.poolPath).move(srcDomUUID, dstDomUUID, imgUUID,
vmUUID, op, postZero, force)
- def sparsifyImage(self, tmpSdUUID, tmpImgUUID, tmpVolUUID, dstSdUUID,
- dstImgUUID, dstVolUUID):
- """
- Reduce sparse image size by converting free space on image to free
- space on host using virt-sparsify.
-
- :param tmpSdUUID: The UUID of the storage domain where the temporary
- snapshot of source volume exists.
- :type tmpSdUUID: UUUID
- :param tmpImgUUID: The UUID of the temporary snapshot image.
- :type tmpImgUUID: UUID
- :param tmpVolUUID: The UUID of the temporary snapshot volume that needs
- to be sparsified.
- :type tmpVolUUID: UUID
- :param dstSdUUID: The UUID of the storage domain where the destination
- image exists.
- :type dstSdUUID: UUUID
- :param dstImgUUID: The UUID of the destination image to which the
- destination volume belongs.
- :type dstImgUUID: UUID
- :param dstVolUUID: The UUID of the destination volume for the
- sparsified volume.
- :type dstVolUUID: UUID
- """
- srcNamespace = sd.getNamespace(tmpSdUUID, IMAGE_NAMESPACE)
- dstNamespace = sd.getNamespace(dstSdUUID, IMAGE_NAMESPACE)
-
- # virt-sparsify writes to temporary volume when using --tmp:prebuilt,
- # so we acquire exclusive lock for the temporary image.
- # Destination image is where the sparsified volume gets written to, so
- # we acquire exclusive lock for the destination image too.
- # Since source volume is only a parent of temporary volume, we don't
- # need to acquire any lock for it.
- with nested(
- rmanager.acquireResource(srcNamespace, tmpImgUUID,
- rm.LockType.exclusive),
- rmanager.acquireResource(dstNamespace, dstImgUUID,
- rm.LockType.exclusive)):
- image.Image(self.poolPath).sparsify(
- tmpSdUUID, tmpImgUUID, tmpVolUUID, dstSdUUID, dstImgUUID,
- dstVolUUID)
-
def cloneImageStructure(self, sdUUID, imgUUID, dstSdUUID):
"""
Clone an image structure from a source domain to a destination domain
--
To view, visit https://gerrit.ovirt.org/57347
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud <smelamud at redhat.com>
Gerrit-Reviewer: Shmuel Melamud <smelamud at redhat.com>
More information about the vdsm-patches
mailing list