Change in vdsm[master]: constants: unify the BLANK_UUID definition

[Federico Simoncelli]

Federico Simoncelli has uploaded a new change for review.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

constants: unify the BLANK_UUID definition

Change-Id: Ib9260d74ae1da1382394a375843b2edff112e6f7
Signed-off-by: Federico Simoncelli 
---
M client/vdsClient.py
M lib/vdsm/constants.py.in
M vdsm/API.py
M vdsm/storage/misc.py
M vdsm/storage/sd.py
M vdsm/storage/volume.py
6 files changed, 11 insertions(+), 9 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/15442/1

diff --git a/client/vdsClient.py b/client/vdsClient.py
index 332b438..0aabd5b 100644
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -26,13 +26,13 @@
 import pprint as pp

from vdsm import vdscli
+from vdsm.constants import BLANK_UUID
+
 try:
     import vdsClientGluster as ge
     _glusterEnabled = True
 except ImportError:
     _glusterEnabled = False
-
-BLANK_UUID = '00000000-0000-0000-0000-000000000000'

STATUS_ERROR = {'status': {'code': 100, 'message': "ERROR"}}

diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index 63771f6..c750462 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -53,6 +53,7 @@
 SUPPORTED_DOMAIN_VERSIONS = DOMAIN_VERSIONS

UUID_GLOB_PATTERN = '*-*-*-*-*'
+BLANK_UUID = '00000000-0000-0000-0000-000000000000'

MEGAB = 2 ** 20  # = 1024 ** 2 = 1 MiB

diff --git a/vdsm/API.py b/vdsm/API.py
index e04e894..0eb23b4 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -104,6 +104,8 @@
 class VM(APIBase):
     ctorArgs = ['vmID']

+    BLANK_UUID = constants.BLANK_UUID
+
     def __init__(self, UUID):
         APIBase.__init__(self)
         self._UUID = UUID
@@ -676,7 +678,7 @@
         SHARED = storage.volume.SHARED_VOL
         LEAF = storage.volume.LEAF_VOL

-    BLANK_UUID = storage.volume.BLANK_UUID
+    BLANK_UUID = constants.BLANK_UUID

def __init__(self, UUID, spUUID, sdUUID, imgUUID):
         APIBase.__init__(self)
@@ -747,7 +749,7 @@
 class Image(APIBase):
     ctorArgs = ['imageID', 'storagepoolID', 'storagedomainID']

-    BLANK_UUID = storage.volume.BLANK_UUID
+    BLANK_UUID = constants.BLANK_UUID

class DiskTypes:
         UNKNOWN = storage.image.UNKNOWN_DISK_TYPE
@@ -855,7 +857,7 @@
         ISO = storage.sd.ISO_DOMAIN
         BACKUP = storage.sd.BACKUP_DOMAIN

-    BLANK_UUID = storage.sd.BLANK_UUID
+    BLANK_UUID = constants.BLANK_UUID

def __init__(self, UUID):
         APIBase.__init__(self)
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index e5ba760..a107fb6 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -433,7 +433,6 @@

UUID_REGEX = re.compile("^[a-f0-9]{8}-(?:[a-f0-9]{4}-){3}[a-f0-9]{12}$")
-UUID_BLANK = "00000000-0000-0000-0000-000000000000"

def validateUUID(uuid, name="uuid", blank=True):
@@ -455,7 +454,7 @@
     if m is None:
         raise se.InvalidParameterException(name, uuid)

-    if not blank and uuid == UUID_BLANK:
+    if not blank and uuid == constants.BLANK_UUID:
         raise se.InvalidParameterException(name, uuid)

diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py
index 8c55c09..0f71b3d 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -133,7 +133,7 @@

ImgsPar = namedtuple("ImgsPar", "imgs,parent")
 ISO_IMAGE_UUID = '11111111-1111-1111-1111-111111111111'
-BLANK_UUID = '00000000-0000-0000-0000-000000000000'
+BLANK_UUID = constants.BLANK_UUID
 REMOVED_IMAGE_PREFIX = "_remove_me_"
 ZEROED_IMAGE_PREFIX = REMOVED_IMAGE_PREFIX + "ZERO_"

diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index 36e8d30..737f24e 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -68,7 +68,7 @@
                 SHARED_VOL: 'SHARED', INTERNAL_VOL: 'INTERNAL',
                 LEAF_VOL: 'LEAF'}

-BLANK_UUID = misc.UUID_BLANK
+BLANK_UUID = constants.BLANK_UUID

# Volume meta data fields
 SIZE = "SIZE"

--
To view, visit http://gerrit.ovirt.org/15442
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib9260d74ae1da1382394a375843b2edff112e6f7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 1: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/15442
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9260d74ae1da1382394a375843b2edff112e6f7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2686/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1874/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2760/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/15442
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9260d74ae1da1382394a375843b2edff112e6f7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2785/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1973/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2859/ : SUCCESS

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 2: Looks good to me, but someone else must approve

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 2: Looks good to me, but someone else must approve

humble devassy has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 2: Looks good to me, but someone else must approve

Alissa Bonas has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 2: (1 inline comment)

How about changing the blank id in BindingXMLRPC to point to the main constant ?
Currently there are many references there to other places that mention blank id (such as Volume.BLANK_UUID, Image.BLANK_UUID) - isn't it better to directly work with the one in constants?

....................................................
File vdsm/API.py
Line 103: 
Line 104: class VM(APIBase):
Line 105:     ctorArgs = ['vmID']
Line 106: 
Line 107:     BLANK_UUID = constants.BLANK_UUID
Why is there a need to add it here if it wasn't previously declared?
Line 108: 
Line 109:     def __init__(self, UUID):
Line 110:         APIBase.__init__(self)
Line 111:         self._UUID = UUID

Saggi Mizrahi has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 2: Code-Review-1

(1 comment)

I agree that code duplication is an issue but constants.py is not the answer.

Having a file names constants.py which is *system wide* no less is an encapsulation nightmare and points out to a deeper problem.

The solution is putting BLANK_UUID = str(uuid.UUID(int=0))
at various places. If you use it only once you don't even need to declare it.

This means that you always use a correct 0 UUID

....................................................
File lib/vdsm/constants.py.in
Line 52: # future we might slice it (eg. tuple(DOMAIN_VERSION[1:]))
Line 53: SUPPORTED_DOMAIN_VERSIONS = DOMAIN_VERSIONS
Line 54: 
Line 55: UUID_GLOB_PATTERN = '*-*-*-*-*'
Line 56: BLANK_UUID = '00000000-0000-0000-0000-000000000000'
We already moved it out of here.
constants.py is a mistake and we are (slowly) moving stuff *out* of it. *Don't* add anything in.
Line 57: 
Line 58: MEGAB = 2 ** 20  # = 1024 ** 2 = 1 MiB
Line 59: 
Line 60: #

Itamar Heim has posted comments on this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Patch Set 2:

ping

Itamar Heim has abandoned this change.

Change subject: constants: unify the BLANK_UUID definition
......................................................................

Abandoned

abandoning per no reply. please restore if still relevant.