From 562ac865c7e2d68a4ae96a6acf046ad48e1ce9f7 Mon Sep 17 00:00:00 2001
In-Reply-To: <92c2af7fd7fa73cce11a6b0a51aaa3c01caf7a17.1492699617.git.phrdina@redhat.com>
References: <92c2af7fd7fa73cce11a6b0a51aaa3c01caf7a17.1492699617.git.phrdina@redhat.com>
From: Pavel Hrdina <phrdina@redhat.com>
Date: Fri, 17 Mar 2017 12:00:03 -0400
Subject: [virt-manager PATCH 2/6] storage: Move alloc/cap validation to
 validate()

From: Cole Robinson <crobinso@redhat.com>

Doing this at property set time is overly noisy. Follow the same
pattern of VirtualDisk and only do this in the validate() function.

https://bugzilla.redhat.com/show_bug.cgi?id=1433239
(cherry picked from commit 9c8ffe51dacee208af4d5d7cc3e439ae7197fc09)
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 virtinst/diskbackend.py |  9 +++++----
 virtinst/storage.py     | 43 +++++++++++++------------------------------
 2 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/virtinst/diskbackend.py b/virtinst/diskbackend.py
index f70f5b21..5198057c 100644
--- a/virtinst/diskbackend.py
+++ b/virtinst/diskbackend.py
@@ -345,10 +345,11 @@ class _StorageCreator(_StorageBase):
 
         if self._vol_install:
             self._vol_install.validate()
-        else:
-            if self._size is None:
-                raise ValueError(_("size is required for non-existent disk "
-                                   "'%s'" % self.get_path()))
+            return
+
+        if self._size is None:
+            raise ValueError(_("size is required for non-existent disk "
+                               "'%s'" % self.get_path()))
 
         err, msg = self.is_size_conflict()
         if err:
diff --git a/virtinst/storage.py b/virtinst/storage.py
index cd74467a..808891f3 100644
--- a/virtinst/storage.py
+++ b/virtinst/storage.py
@@ -683,22 +683,6 @@ class StorageVolume(_StorageObject):
             raise ValueError(_("Name '%s' already in use by another volume." %
                                 name))
 
-    def _validate_allocation(self, val):
-        ret = self.is_size_conflict(allocation=val)
-        if ret[0]:
-            raise ValueError(ret[1])
-        elif ret[1]:
-            logging.warn(ret[1])
-        return val
-
-    def _validate_capacity(self, val):
-        ret = self.is_size_conflict(capacity=val)
-        if ret[0]:
-            raise ValueError(ret[1])
-        elif ret[1]:
-            logging.warn(ret[1])
-        return val
-
     def _default_format(self):
         if self.file_type == self.TYPE_FILE:
             return "raw"
@@ -728,10 +712,8 @@ class StorageVolume(_StorageObject):
 
     type = XMLProperty("./@type")
     key = XMLProperty("./key")
-    capacity = XMLProperty("./capacity", is_int=True,
-                           validate_cb=_validate_capacity)
-    allocation = XMLProperty("./allocation", is_int=True,
-                             validate_cb=_validate_allocation)
+    capacity = XMLProperty("./capacity", is_int=True)
+    allocation = XMLProperty("./allocation", is_int=True)
     format = XMLProperty("./target/format/@type", default_cb=_default_format)
     target_path = XMLProperty("./target/path")
     backing_store = XMLProperty("./backingStore/path")
@@ -809,6 +791,12 @@ class StorageVolume(_StorageObject):
                                "setting allocation equal to capacity"))
                 self.allocation = self.capacity
 
+        isfatal, errmsg = self.is_size_conflict()
+        if isfatal:
+            raise ValueError(errmsg)
+        if errmsg:
+            logging.warn(errmsg)
+
     def install(self, meter=None):
         """
         Build and install storage volume from xml
@@ -891,7 +879,7 @@ class StorageVolume(_StorageObject):
             time.sleep(1)
 
 
-    def is_size_conflict(self, capacity=None, allocation=None):
+    def is_size_conflict(self):
         """
         Report if requested size exceeds its pool's available amount
 
@@ -900,27 +888,22 @@ class StorageVolume(_StorageObject):
             2. String message if some collision was encountered.
         @rtype: 2 element C{tuple}: (C{bool}, C{str})
         """
-        if capacity is None:
-            capacity = self.capacity
-        if allocation is None:
-            allocation = self.allocation
-
         if not self.pool:
             return (False, "")
 
         # pool info is [pool state, capacity, allocation, available]
         avail = self.pool.info()[3]
-        if allocation > avail:
+        if self.allocation > avail:
             return (True, _("There is not enough free space on the storage "
                             "pool to create the volume. "
                             "(%d M requested allocation > %d M available)") %
-                            ((allocation / (1024 * 1024)),
+                            ((self.allocation / (1024 * 1024)),
                              (avail / (1024 * 1024))))
-        elif capacity > avail:
+        elif self.capacity > avail:
             return (False, _("The requested volume capacity will exceed the "
                              "available pool space when the volume is fully "
                              "allocated. "
                              "(%d M requested capacity > %d M available)") %
-                             ((capacity / (1024 * 1024)),
+                             ((self.capacity / (1024 * 1024)),
                               (avail / (1024 * 1024))))
         return (False, "")
-- 
2.12.2