Change in vdsm[master]: image: Calculate qcow volume size on import.

[Maor Lipchuk]

Maor Lipchuk has uploaded a new change for review.

Change subject: image: Calculate qcow volume size on import.
......................................................................

image: Calculate qcow volume size on import.

Estimating qcow2 file size when converting from raw to
qcow.

The calculation is as follow:
min_allocation = (used_blocks * block_size) +
(virtual_size / cluster_size * 64 bit L2 table entry) +
extra Gib e to avoid instant extend.

Change-Id: Ia87df2b0d5378f93c5cb2cc68a37458fe3b4467b
Bug-Url: https://bugzilla.redhat.com/1358717
Signed-off-by: Maor Lipchuk 
---
M vdsm/storage/image.py
1 file changed, 16 insertions(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/65039/1

diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index ccaf971..274f438 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -45,6 +45,11 @@
 log = logging.getLogger('storage.Image')
 rmanager = rm.ResourceManager.getInstance()

+# Pre-configured QCOW volume size.
+CLUSTER_SIZE = 65536
+BLOCK_SIZE = sc.BLOCK_SIZE
+GIB_BLOCK = 10 ** 9 / BLOCK_SIZE  # GiB
+
 # Disk type
 UNKNOWN_DISK_TYPE = 0
 SYSTEM_DISK_TYPE = 1
@@ -842,7 +847,17 @@
                 if dstVolFormat == sc.RAW_FORMAT:
                     newsize = volParams['size']
                 else:
-                    newsize = volParams['apparentsize']
+                    # Estimating qcow2 file size when converting from raw to
+                    # qcow. The calculation is as follow: min_allocation =
+                    # (used_blocks * block_size) +
+                    # (virtual_size / cluster_size * 64 bit L2 table entry) +
+                    # 1 extra Gib e to avoid instant extend.
+                    newsize = ((volParams['truesize'] * BLOCK_SIZE) +
+                               (volParams['size'] / CLUSTER_SIZE * 8) +
+                               GIB_BLOCK)
+                    self.log.info("qcow volume size to be copied is %i",
+                                  newsize)
+
                 dstVol.extend(newsize)
                 dstPath = dstVol.getVolumePath()
                 # Change destination volume metadata back to the original size.

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 1: Verified+1

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 1:

(7 comments)

https://gerrit.ovirt.org/#/c/65039/1/vdsm/storage/image.py
File vdsm/storage/image.py:

Line 840
Line 841
Line 842
Line 843
Line 844
The new calculation is only for converting raw-sparse to cow-sparse. I don't think you check this here.

Line 45: log = logging.getLogger('storage.Image')
Line 46: rmanager = rm.ResourceManager.getInstance()
Line 47: 
Line 48: # Pre-configured QCOW volume size.
Line 49: CLUSTER_SIZE = 65536
This is QCOW2_CLUSTER_SIZE.

Looks like we need a new qcow2.py module for implemnting this calculation. In this module, we can call the constant CLUSTER_SIZE.
Line 50: BLOCK_SIZE = sc.BLOCK_SIZE
Line 51: GIB_BLOCK = 10 ** 9 / BLOCK_SIZE  # GiB
Line 52: 
Line 53: # Disk type

Line 46: rmanager = rm.ResourceManager.getInstance()
Line 47: 
Line 48: # Pre-configured QCOW volume size.
Line 49: CLUSTER_SIZE = 65536
Line 50: BLOCK_SIZE = sc.BLOCK_SIZE
We don't need our own BLOCK_SIZE copy, use sc.BLOCK_SIZE.
Line 51: GIB_BLOCK = 10 ** 9 / BLOCK_SIZE  # GiB
Line 52: 
Line 53: # Disk type
Line 54: UNKNOWN_DISK_TYPE = 0

Line 47: 
Line 48: # Pre-configured QCOW volume size.
Line 49: CLUSTER_SIZE = 65536
Line 50: BLOCK_SIZE = sc.BLOCK_SIZE
Line 51: GIB_BLOCK = 10 ** 9 / BLOCK_SIZE  # GiB
Not needed.
Line 52: 
Line 53: # Disk type
Line 54: UNKNOWN_DISK_TYPE = 0
Line 55: SYSTEM_DISK_TYPE = 1

Line 852:                     # (used_blocks * block_size) +
Line 853:                     # (virtual_size / cluster_size * 64 bit L2 table entry) +
Line 854:                     # 1 extra Gib e to avoid instant extend.
Line 855:                     newsize = ((volParams['truesize'] * BLOCK_SIZE) +
Line 856:                                (volParams['size'] / CLUSTER_SIZE * 8) +
We need a constant for that magic 8 - qcow2.L2TABLE_ENTRY_SIZE seems like the right name.
Line 857:                                GIB_BLOCK)
Line 858:                     self.log.info("qcow volume size to be copied is %i",
Line 859:                                   newsize)
Line 860:

Line 853:                     # (virtual_size / cluster_size * 64 bit L2 table entry) +
Line 854:                     # 1 extra Gib e to avoid instant extend.
Line 855:                     newsize = ((volParams['truesize'] * BLOCK_SIZE) +
Line 856:                                (volParams['size'] / CLUSTER_SIZE * 8) +
Line 857:                                GIB_BLOCK)
We don't want to add this in this calculation. This should be based on irs:volume_utilization_chunk_mb, so the new disk behave like a regular disk (have about volume_utilization_chunk_mb free space on creation).
Line 858:                     self.log.info("qcow volume size to be copied is %i",
Line 859:                                   newsize)
Line 860: 
Line 861:                 dstVol.extend(newsize)

Line 855:                     newsize = ((volParams['truesize'] * BLOCK_SIZE) +
Line 856:                                (volParams['size'] / CLUSTER_SIZE * 8) +
Line 857:                                GIB_BLOCK)
Line 858:                     self.log.info("qcow volume size to be copied is %i",
Line 859:                                   newsize)
This should be log debug, something like:

Estimated allocation for qcow2 volume: N bytes

I think the best place for this logic is a new qcow2.py module:
lib/vdsm/storage/qcow2.py

We can add a function like:

def estimate_required_size(virtual_size, used_size):
        """
        Explain here what this does and how to use it, mention Kevin Wolf for reference.
        """
        worst_case_overhead = virtual_size / CLUSTER_SIZE * L2TABLE_ENTRY_SIZE
        return use_blocks * BLOCK_SIZE + worst_case_overhead

Note that this should accept and return values in bytes.

We probably need to add qcow2 header size, see
https://github.com/qemu/qemu/blob/master/docs/specs/qcow2.txt

I like to have automated test verifying this calculation, so
if qemu change something our test will break and we can
fix this code.

The test can create a raw file with worst case allocation
(one block written for each l2 table entry) - please check
with Kevin how to do this.

Then we can covert the file to qcow and check the used size
(os.stat(filename).st_blocks * block_size)

We should compare both compat 0.10 and 1.1, this code must
work with all domain versions.

And here we can add the additional volume_utilization_chunk_mb to calculate the disk size and convert the value to blocks for our legacy apis.
Line 860: 
Line 861:                 dstVol.extend(newsize)
Line 862:                 dstPath = dstVol.getVolumePath()
Line 863:                 # Change destination volume metadata back to the original size.

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 1:

(7 comments)

https://gerrit.ovirt.org/#/c/65039/1/vdsm/storage/image.py
File vdsm/storage/image.py:

Line 840
Line 841
Line 842
Line 843
Line 844
> The new calculation is only for converting raw-sparse to cow-sparse. I don'

Done

Line 45: log = logging.getLogger('storage.Image')
Line 46: rmanager = rm.ResourceManager.getInstance()
Line 47: 
Line 48: # Pre-configured QCOW volume size.
Line 49: CLUSTER_SIZE = 65536
> This is QCOW2_CLUSTER_SIZE.

Done
Line 50: BLOCK_SIZE = sc.BLOCK_SIZE
Line 51: GIB_BLOCK = 10 ** 9 / BLOCK_SIZE  # GiB
Line 52: 
Line 53: # Disk type

Line 46: rmanager = rm.ResourceManager.getInstance()
Line 47: 
Line 48: # Pre-configured QCOW volume size.
Line 49: CLUSTER_SIZE = 65536
Line 50: BLOCK_SIZE = sc.BLOCK_SIZE
> We don't need our own BLOCK_SIZE copy, use sc.BLOCK_SIZE.

Done
Line 51: GIB_BLOCK = 10 ** 9 / BLOCK_SIZE  # GiB
Line 52: 
Line 53: # Disk type
Line 54: UNKNOWN_DISK_TYPE = 0

Line 47: 
Line 48: # Pre-configured QCOW volume size.
Line 49: CLUSTER_SIZE = 65536
Line 50: BLOCK_SIZE = sc.BLOCK_SIZE
Line 51: GIB_BLOCK = 10 ** 9 / BLOCK_SIZE  # GiB
> Not needed.

Done
Line 52: 
Line 53: # Disk type
Line 54: UNKNOWN_DISK_TYPE = 0
Line 55: SYSTEM_DISK_TYPE = 1

Line 852:                     # (used_blocks * block_size) +
Line 853:                     # (virtual_size / cluster_size * 64 bit L2 table entry) +
Line 854:                     # 1 extra Gib e to avoid instant extend.
Line 855:                     newsize = ((volParams['truesize'] * BLOCK_SIZE) +
Line 856:                                (volParams['size'] / CLUSTER_SIZE * 8) +
> We need a constant for that magic 8 - qcow2.L2TABLE_ENTRY_SIZE seems like t

Done
Line 857:                                GIB_BLOCK)
Line 858:                     self.log.info("qcow volume size to be copied is %i",
Line 859:                                   newsize)
Line 860:

Line 853:                     # (virtual_size / cluster_size * 64 bit L2 table entry) +
Line 854:                     # 1 extra Gib e to avoid instant extend.
Line 855:                     newsize = ((volParams['truesize'] * BLOCK_SIZE) +
Line 856:                                (volParams['size'] / CLUSTER_SIZE * 8) +
Line 857:                                GIB_BLOCK)
> We don't want to add this in this calculation. This should be based on irs:

Done
Line 858:                     self.log.info("qcow volume size to be copied is %i",
Line 859:                                   newsize)
Line 860: 
Line 861:                 dstVol.extend(newsize)

Line 855:                     newsize = ((volParams['truesize'] * BLOCK_SIZE) +
Line 856:                                (volParams['size'] / CLUSTER_SIZE * 8) +
Line 857:                                GIB_BLOCK)
Line 858:                     self.log.info("qcow volume size to be copied is %i",
Line 859:                                   newsize)
> This should be log debug, something like:

done, without the tests, will handle them in the future uploads.
Added TODO for qcow2 header length
Line 860: 
Line 861:                 dstVol.extend(newsize)
Line 862:                 dstPath = dstVol.getVolumePath()
Line 863:                 # Change destination volume metadata back to the original size.

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 2:

(14 comments)

https://gerrit.ovirt.org/#/c/65039/2/vdsm/storage/image.py
File vdsm/storage/image.py:

Line 842
Line 843
Line 844
Line 845
Line 846
We are adding too much code to existing function which was to long before we touch it.

Lets extract a calculate_required_size function that will be easy to test.

Line 35: from vdsm.storage import misc
Line 36: from vdsm.storage import workarounds
Line 37: from vdsm.storage.threadlocal import vars
Line 38: 
Line 39: import qcow2
This should be in lib/vdsm/storage, so:

from vdsm.storage import qcow2
Line 40: from sdc import sdCache
Line 41: import sd
Line 42: import imageSharing
Line 43: from vdsm.common.exception import ActionStopped

Line 842:                 # For convert to 'raw' we need use the virtual disk size
Line 843:                 # instead of apparent size
Line 844:                 if dstVolFormat == sc.RAW_FORMAT:
Line 845:                     newsize = volParams['size']
Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
I think this checks also that the destination volume is cow. For raw, using apparentsize is fine.
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])

Line 843:                 # instead of apparent size
Line 844:                 if dstVolFormat == sc.RAW_FORMAT:
Line 845:                     newsize = volParams['size']
Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
Put this todo in qcow2.estimate_required_size
Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",

Line 844:                 if dstVolFormat == sc.RAW_FORMAT:
Line 845:                     newsize = volParams['size']
Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
Line 848:                     newsize = qcow2.estimate_required_size(
Using newsize for this is not a good idea, better keep two separate variables - estimated_size and chunk_size.
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")

Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
qcow2.estimate_required_size must accept sizes in bytes, you are sending sizes in sectors.
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size

Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
I would call it chunk_size, and initialize it in bytes:

chunk_size = config.getint("irs", "volume_utilization_chunk_mb") * constants.MEGAB

You will probably need to fight with pep8 tool on this.
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size
Line 855:                     self.log.debug("Estimated allocation for qcow2 volume:"
Line 856:                                    "%i bytes", newsize)

Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
We have a constant for MB.
Line 854:                     newsize += extend_size
Line 855:                     self.log.debug("Estimated allocation for qcow2 volume:"
Line 856:                                    "%i bytes", newsize)
Line 857:                 else:

Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size
I would do:

newsize = (estimated_size + chunk_size) / BLOCK_SIZE

This make our intent very clear.
Line 855:                     self.log.debug("Estimated allocation for qcow2 volume:"
Line 856:                                    "%i bytes", newsize)
Line 857:                 else:
Line 858:                     newsize = volParams['apparentsize']

Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size
Line 855:                     self.log.debug("Estimated allocation for qcow2 volume:"
Line 856:                                    "%i bytes", newsize)
%i may work but we always use %d, lets keep the tradition.
Line 857:                 else:
Line 858:                     newsize = volParams['apparentsize']
Line 859: 
Line 860:                 dstVol.extend(newsize)

https://gerrit.ovirt.org/#/c/65039/2/vdsm/storage/qcow2.py
File vdsm/storage/qcow2.py:

Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: 
Please add:

from __future__ import absolute_import

And move this module to lib/vdsm/storage. We don't create new modules in vdsm, we are trying to move everything to lib/vdsm.
Line 21: from vdsm.storage import constants as sc
Line 22: 
Line 23: # Pre-configured QCOW volume size.
Line 24: CLUSTER_SIZE = 65536

Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: 
Line 21: from vdsm.storage import constants as sc
We don't need this import, since we can require used_size in bytes.
Line 22: 
Line 23: # Pre-configured QCOW volume size.
Line 24: CLUSTER_SIZE = 65536
Line 25: L2TABLE_ENTRY_SIZE = 8

Line 24: CLUSTER_SIZE = 65536
Line 25: L2TABLE_ENTRY_SIZE = 8
Line 26: 
Line 27: 
Line 28: def estimate_required_size(virtual_size, used_size):
Both virtual_size and used_size must be in bytes. Please document the arguments types and semantics to make this easy to use without errors.

virtual_size: size of the volume in bytes as seen by the guest.
    For raw-sparse file volume, the file size (st_size).
    For raw-preallocated block volume, the device size.

used_size: number of bytes used in this volume. For raw-sparse file
    volume, numbers of blocks (st_blocks) multiplied by the block size.
    for raw-preallocated block volume the used size can be obtained
    using qeme-img map.

See Ala patch for adding qemuimg.map():
https://gerrit.ovirt.org/#/c/65112/.
Line 29:     """
Line 30:     Estimating qcow2 file size in bytes when converting from raw to
Line 31:     qcow as advised by [e-mail].
Line 32:     The calculation for minimum allocation is as follow:

Line 27: 
Line 28: def estimate_required_size(virtual_size, used_size):
Line 29:     """
Line 30:     Estimating qcow2 file size in bytes when converting from raw to
Line 31:     qcow as advised by [e-mail].
I think Kevin Wolf is better then his email, in case he don't want his email to be on github, and is also more friendly.
Line 32:     The calculation for minimum allocation is as follow:
Line 33:     (used_blocks * block_size) +
Line 34:     (virtual_size / cluster_size * 64 bit L2 table entry)
Line 35:     """

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/65039/2/vdsm/storage/qcow2.py
File vdsm/storage/qcow2.py:

Line 33:     (used_blocks * block_size) +
Line 34:     (virtual_size / cluster_size * 64 bit L2 table entry)
Line 35:     """
Line 36:     worst_case_overhead = virtual_size / CLUSTER_SIZE * L2TABLE_ENTRY_SIZE
Line 37:     return used_size * sc.BLOCK_SIZE + worst_case_overhead
used_size must be in bytes, we don't need to multiple with bloc size.

And of course we need to write couple of tests that verify our assumptions:
- convert empty file to qcow2
- convert file with some used blocks to qcow2
- convert file with worst case allocation to qcow2 - mybe one byte for each 64K? ask Kevin how to create such file.
- covert file full with data (not zeros) to qcow2

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/65039/2/vdsm/storage/image.py
File vdsm/storage/image.py:

Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
> qcow2.estimate_required_size must accept sizes in bytes, you are sending si

truesize is not correct for block volume, this is always the same as the virtual size since the volume is preallocated. In this case we need to get the value using qemu-img map.

We can simply not handle this now by limiting the new estimation logic to conversion from raw-sparse (always a file) to cow-sparse.
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 2:

Missing changes:
- Update lib/vdsm/storage/Makefile.am
- vdsm.spec.in

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 2: Verified+1

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 2: -Verified

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: image: Calculate qcow volume size on import.
......................................................................

Patch Set 2:

(15 comments)

https://gerrit.ovirt.org/#/c/65039/2/vdsm/storage/image.py
File vdsm/storage/image.py:

Line 842
Line 843
Line 844
Line 845
Line 846
> We are adding too much code to existing function which was to long before w

Done

Line 35: from vdsm.storage import misc
Line 36: from vdsm.storage import workarounds
Line 37: from vdsm.storage.threadlocal import vars
Line 38: 
Line 39: import qcow2
> This should be in lib/vdsm/storage, so:

Done
Line 40: from sdc import sdCache
Line 41: import sd
Line 42: import imageSharing
Line 43: from vdsm.common.exception import ActionStopped

Line 842:                 # For convert to 'raw' we need use the virtual disk size
Line 843:                 # instead of apparent size
Line 844:                 if dstVolFormat == sc.RAW_FORMAT:
Line 845:                     newsize = volParams['size']
Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
> I think this checks also that the destination volume is cow. For raw, using

I'm not sure I understood this comment, can u please try to rephrase
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])

Line 843:                 # instead of apparent size
Line 844:                 if dstVolFormat == sc.RAW_FORMAT:
Line 845:                     newsize = volParams['size']
Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
> Put this todo in qcow2.estimate_required_size

Done
Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",

Line 844:                 if dstVolFormat == sc.RAW_FORMAT:
Line 845:                     newsize = volParams['size']
Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
Line 848:                     newsize = qcow2.estimate_required_size(
> Using newsize for this is not a good idea, better keep two separate variabl

Done
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")

Line 846:                 elif volParams['volFormat'] == sc.RAW_FORMAT:
Line 847:                     # TODO: Some extra space may be needed for QCOW2 headers
Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
> truesize is not correct for block volume, this is always the same as the vi

If we can't handle this now what do you suggest?
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size

Line 848:                     newsize = qcow2.estimate_required_size(
Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
> I would call it chunk_size, and initialize it in bytes:

Done
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size
Line 855:                     self.log.debug("Estimated allocation for qcow2 volume:"
Line 856:                                    "%i bytes", newsize)

Line 849:                         volParams['size'],
Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
> We have a constant for MB.

Done
Line 854:                     newsize += extend_size
Line 855:                     self.log.debug("Estimated allocation for qcow2 volume:"
Line 856:                                    "%i bytes", newsize)
Line 857:                 else:

Line 850:                         volParams['truesize'])
Line 851:                     extend_size = config.getint("irs",
Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size
> I would do:

Done
Line 855:                     self.log.debug("Estimated allocation for qcow2 volume:"
Line 856:                                    "%i bytes", newsize)
Line 857:                 else:
Line 858:                     newsize = volParams['apparentsize']

Line 852:                                                 "volume_utilization_chunk_mb")
Line 853:                     extend_size *= (10 ** 6)
Line 854:                     newsize += extend_size
Line 855:                     self.log.debug("Estimated allocation for qcow2 volume:"
Line 856:                                    "%i bytes", newsize)
> %i may work but we always use %d, lets keep the tradition.

Done
Line 857:                 else:
Line 858:                     newsize = volParams['apparentsize']
Line 859: 
Line 860:                 dstVol.extend(newsize)

https://gerrit.ovirt.org/#/c/65039/2/vdsm/storage/qcow2.py
File vdsm/storage/qcow2.py:

Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20:
> Please add:

Done
Line 21: from vdsm.storage import constants as sc
Line 22: 
Line 23: # Pre-configured QCOW volume size.
Line 24: CLUSTER_SIZE = 65536

Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: 
Line 21: from vdsm.storage import constants as sc
> We don't need this import, since we can require used_size in bytes.

Done
Line 22: 
Line 23: # Pre-configured QCOW volume size.
Line 24: CLUSTER_SIZE = 65536
Line 25: L2TABLE_ENTRY_SIZE = 8

Line 24: CLUSTER_SIZE = 65536
Line 25: L2TABLE_ENTRY_SIZE = 8
Line 26: 
Line 27: 
Line 28: def estimate_required_size(virtual_size, used_size):
> Both virtual_size and used_size must be in bytes. Please document the argum

What do you suggest to do with qemiimg.map()?
Line 29:     """
Line 30:     Estimating qcow2 file size in bytes when converting from raw to
Line 31:     qcow as advised by [e-mail].
Line 32:     The calculation for minimum allocation is as follow:

Line 27: 
Line 28: def estimate_required_size(virtual_size, used_size):
Line 29:     """
Line 30:     Estimating qcow2 file size in bytes when converting from raw to
Line 31:     qcow as advised by [e-mail].
> I think Kevin Wolf is better then his email, in case he don't want his emai

I think it is better to use Red Hat email since it is more proffesional that way, but I won't mind changing this.
Done
Line 32:     The calculation for minimum allocation is as follow:
Line 33:     (used_blocks * block_size) +
Line 34:     (virtual_size / cluster_size * 64 bit L2 table entry)
Line 35:     """

Line 33:     (used_blocks * block_size) +
Line 34:     (virtual_size / cluster_size * 64 bit L2 table entry)
Line 35:     """
Line 36:     worst_case_overhead = virtual_size / CLUSTER_SIZE * L2TABLE_ENTRY_SIZE
Line 37:     return used_size * sc.BLOCK_SIZE + worst_case_overhead
> used_size must be in bytes, we don't need to multiple with bloc size.

Done, working on the tests