Change in vdsm[master]: storage: Initial size for thin provisioning disk

nsoffer at redhat.com nsoffer at redhat.com
Tue Oct 13 07:05:51 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: storage: Initial size for thin provisioning disk
......................................................................


Patch Set 8: Code-Review+1

(1 comment)

Looks good, but I want to discuss the ugly bytes/sectors interface first before we continue with this.

https://gerrit.ovirt.org/#/c/46417/8/vdsm/storage/blockVolume.py
File vdsm/storage/blockVolume.py:

Line 177: 
Line 178:         if initial_size and preallocate == volume.PREALLOCATED_VOL:
Line 179:             log.error("Initial size is not supported for preallocated volumes")
Line 180:             raise se.InvalidParameterException("initial size",
Line 181:                                                initial_size)
This pattern of logging an error and raising is bad, leading to logging the same error twice.

The first log give context, but does not show the value. The exception has no context but show the value.

We should have (in a future patch), code like this:

    raise se.InvalidParameterException("Initial size is not supported for preallocated volumes", initial_size=initial_size)

Logging this exception should show:

    InvalidParameterException: Initial size is not supported for preallocated volumes (initial_size=2048)
Line 182: 
Line 183:         if preallocate == volume.SPARSE_VOL:
Line 184:             if initial_size:
Line 185:                 initial_size = int(initial_size * QCOW_OVERHEAD_FACTOR)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf5f142541bf0b311a77a0544a1c4ffb689a9fde
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list