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.


-- 
To view, visit https://gerrit.ovirt.org/65039
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia87df2b0d5378f93c5cb2cc68a37458fe3b4467b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Reply via email to