On Thu, Dec 5, 2013 at 11:29 AM, Thomas Thrainer <[email protected]> wrote:
> Why can't we add ST_SHARED_FILE in STS_REPORT? It does do (dummy) reporting,
> right? In any case, I think having a special case here is not so great, so
> either go for and stick to the dummy reporting, or don't claim that it's
> supported at all (which also defeats the purpose of the
> _DeclineToProvideSharedFileStorageInfo function, right?).
I found out that the culprit was my implementation of
getDefaultStorageKey. If my understanding is correct, gnt-node list
causes a node storage query to be sent to luxid; luxid uses
getDefaultStorageKey, ignoring stsReport, and the query reached
_DeclineToProvideSharedFileStorageInfo;
_DeclineToProvideSharedFileStorageInfo refused to provide information,
which confused luxid and in turn gnt-node list. Of course, this
happens entirely at runtime.
Once I do that, the only thing stopping me from adding ST_SHARED_FILE
to STS_REPORT is that if I do so, I _must_ have some
_DeclineToProvideSharedFileStorageInfo function, even if it takes the
non-implemented behavior (as specified in
_ComputeStorageDataFromSpaceInfoByTemplate) and returns:
{"type": constants.ST_SHARED_FILE, "name": path,
"storage_size": 0, "storage_free": 0}
...given that this seems to be the default value all iAllocators get,
my previous worries about those zeroes having a special meaning in
_all_ iAllocators are essentially vanished.
So, now I have two choices:
* Add a new list, STS_REPORT_NODE_STORAGE, that only gets used in the
functions I "whitelist" (so far, mainly LUNodeQueryStorage); see the
interdiff below.
* Add Gluster to STS_REPORT anyway and hope nothing I didn't check breaks.
My local QA works either way, but I might've disabled important bits
and pieces in order to let it run in the first place in a manageable
amount of time on an existing cluster. I consider the former approach
safer, but if the latter works without breaking ours and third party
stuff, that makes the code simpler.
diff --git a/lib/backend.py b/lib/backend.py
index 51b76d0..5f04438 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -755,14 +755,6 @@ def GetNodeInfo(storage_units, hv_specs):
return (bootid, storage_info, hv_info)
-def _DeclineToProvideSharedFileStorageInfo(path, _):
- """Returns no storage data for the Shared File disk template.
-
- """
- return {"type": constants.ST_SHARED_FILE,
- "name": path}
-
-
def _GetFileStorageSpaceInfo(path, params):
"""Wrapper around filestorage.GetSpaceInfo.
@@ -786,7 +778,7 @@ _STORAGE_TYPE_INFO_FN = {
constants.ST_FILE: _GetFileStorageSpaceInfo,
constants.ST_LVM_PV: _GetLvmPvSpaceInfo,
constants.ST_LVM_VG: _GetLvmVgSpaceInfo,
- constants.ST_SHARED_FILE: _DeclineToProvideSharedFileStorageInfo,
+ constants.ST_SHARED_FILE: _GetSharedFileDummySpaceInfo,
constants.ST_RADOS: None,
}
diff --git a/lib/bootstrap.py b/lib/bootstrap.py
index a0185ff..0c0b6d1 100644
--- a/lib/bootstrap.py
+++ b/lib/bootstrap.py
@@ -393,9 +393,10 @@ def _PrepareFileBasedStorage(
@returns: the name of the actual file storage directory
"""
- assert (file_disk_template in
- utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE,
-
constants.ST_SHARED_FILE))
+ assert (file_disk_template in utils.storage.GetDiskTemplatesOfStorageTypes(
+ constants.ST_FILE, constants.ST_SHARED_FILE
+ ))
+
if file_storage_dir is None:
file_storage_dir = default_dir
if not acceptance_fn:
diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
index a16d45a..5534f94 100644
--- a/lib/cmdlib/cluster.py
+++ b/lib/cmdlib/cluster.py
@@ -654,9 +654,9 @@ def CheckFileBasedStoragePathVsEnabledDiskTemplates(
path should be checked
"""
- assert (file_disk_template in
- utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE,
-
constants.ST_SHARED_FILE))
+ assert (file_disk_template in utils.storage.GetDiskTemplatesOfStorageTypes(
+ constants.ST_FILE, constants.ST_SHARED_FILE
+ ))
file_storage_enabled = file_disk_template in enabled_disk_templates
if file_storage_dir is not None:
if file_storage_dir == "":
@@ -2576,10 +2576,10 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors):
in case something goes wrong in this verification step
"""
- assert (file_disk_template in
- utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE,
-
constants.ST_SHARED_FILE
- ))
+ assert (file_disk_template in utils.storage.GetDiskTemplatesOfStorageTypes(
+ constants.ST_FILE, constants.ST_SHARED_FILE
+ ))
+
cluster = self.cfg.GetClusterInfo()
if cluster.IsDiskTemplateEnabled(file_disk_template):
self._ErrorIf(
diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py
index 8dc09d1..77b6f20 100644
--- a/lib/cmdlib/common.py
+++ b/lib/cmdlib/common.py
@@ -1104,7 +1104,7 @@ def CheckStorageTypeEnabled(cluster, storage_type):
CheckStorageTypeEnabled(cluster, constants.ST_LVM_VG)
else:
possible_disk_templates = \
- utils.storage.GetDiskTemplatesOfStorageType(storage_type)
+ utils.storage.GetDiskTemplatesOfStorageTypes(storage_type)
for disk_template in possible_disk_templates:
if disk_template in cluster.enabled_disk_templates:
return
@@ -1174,7 +1174,7 @@ def CheckDiskAccessModeConsistency(parameters,
cfg, group=None):
access = parameters[disk_template].get(constants.LDP_ACCESS,
constants.DISK_KERNELSPACE)
- if dt not in constants.DTS_HAVE_ACCESS
+ if disk_template not in constants.DTS_HAVE_ACCESS:
continue
#Check the combination of instance hypervisor, disk template and access
diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
index a95fddc..e5e53bf 100644
--- a/lib/cmdlib/node.py
+++ b/lib/cmdlib/node.py
@@ -1310,8 +1310,7 @@ class LUNodeQueryStorage(NoHooksLU):
self.storage_type = self.op.storage_type
else:
self.storage_type = self._DetermineStorageType()
- supported_storage_types = set(constants.STS_REPORT)
- supported_storage_types.add(constants.ST_SHARED_FILE)
+ supported_storage_types = constants.STS_REPORT_NODE_STORAGE
if self.storage_type not in supported_storage_types:
raise errors.OpPrereqError(
"Storage reporting for storage type '%s' is not supported. Please"
diff --git a/lib/utils/storage.py b/lib/utils/storage.py
index e3b6cb9..e0328ff 100644
--- a/lib/utils/storage.py
+++ b/lib/utils/storage.py
@@ -27,7 +27,7 @@ import logging
from ganeti import constants
-def GetDiskTemplatesOfStorageType(*storage_types):
+def GetDiskTemplatesOfStorageTypes(*storage_types):
"""Given the storage type, returns a list of disk templates based on that
storage type."""
return [dt for dt in constants.DISK_TEMPLATES
diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
index ef1d40b..af792a0 100644
--- a/qa/qa_cluster.py
+++ b/qa/qa_cluster.py
@@ -498,8 +498,8 @@ def TestClusterModifyFileBasedStorageDir(
# Get some non-file-based disk template to disable file storage
other_disk_template = _GetOtherEnabledDiskTemplate(
- utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE,
- constants.ST_SHARED_FILE),
+ utils.storage.GetDiskTemplatesOfStorageTypes(constants.ST_FILE,
+ constants.ST_SHARED_FILE),
enabled_disk_templates
)
diff --git a/qa/qa_config.py b/qa/qa_config.py
index 29d17c5..bc650ce 100644
--- a/qa/qa_config.py
+++ b/qa/qa_config.py
@@ -480,9 +480,9 @@ class _QaConfig(object):
"""
enabled_disk_templates = self.GetEnabledDiskTemplates()
if storage_type == constants.ST_LVM_PV:
- disk_templates = utils.GetDiskTemplatesOfStorageType(constants.ST_LVM_VG)
+ disk_templates =
utils.GetDiskTemplatesOfStorageTypes(constants.ST_LVM_VG)
else:
- disk_templates = utils.GetDiskTemplatesOfStorageType(storage_type)
+ disk_templates = utils.GetDiskTemplatesOfStorageTypes(storage_type)
return bool(set(enabled_disk_templates).intersection(set(disk_templates)))
def AreSpindlesSupported(self):
diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
index 5a2ce25..a98ac49 100644
--- a/src/Ganeti/Constants.hs
+++ b/src/Ganeti/Constants.hs
@@ -674,13 +674,16 @@ stRados = Types.storageTypeToRaw StorageRados
storageTypes :: FrozenSet String
storageTypes = ConstantUtils.mkSet $ map Types.storageTypeToRaw [minBound..]
--- | The set of storage types for which storage reporting is available
---
--- FIXME: Remove this, once storage reporting is available for all
--- types.
+-- | The set of storage types for which full storage reporting is available
stsReport :: FrozenSet String
stsReport = ConstantUtils.mkSet [stFile, stLvmPv, stLvmVg]
+-- | The set of storage types for which node storage reporting is available
+-- | (as used by LUQueryNodeStorage)
+stsReportNodeStorage :: FrozenSet String
+stsReportNodeStorage = ConstantUtils.union stsReport $
+ ConstantUtils.mkSet [stSharedFile]
+
-- * Storage fields
-- ** First two are valid in LU context only, not passed to backend
diff --git a/test/py/ganeti.backend_unittest.py
b/test/py/ganeti.backend_unittest.py
index 6651cd9..39fca14 100755
--- a/test/py/ganeti.backend_unittest.py
+++ b/test/py/ganeti.backend_unittest.py
@@ -846,9 +846,7 @@ class TestSpaceReportingConstants(unittest.TestCase):
"""
- REPORTING = set(constants.STS_REPORT)
- REPORTING.add(constants.ST_SHARED_FILE)
-
+ REPORTING = set(constants.STS_REPORT_NODE_STORAGE)
NOT_REPORTING = set(constants.STORAGE_TYPES) - REPORTING
def testAllReportingTypesHaveAReportingFunction(self):
diff --git a/test/py/ganeti.utils.storage_unittest.py
b/test/py/ganeti.utils.storage_unittest.py
index c48cd0c..ac607ed 100755
--- a/test/py/ganeti.utils.storage_unittest.py
+++ b/test/py/ganeti.utils.storage_unittest.py
@@ -81,7 +81,7 @@ class TestGetStorageUnits(unittest.TestCase):
def testGetStorageUnits(self):
disk_templates = constants.DTS_FILEBASED - frozenset(
- storage.GetDiskTemplatesOfStorageType(constants.ST_SHARED_FILE)
+ storage.GetDiskTemplatesOfStorageTypes(constants.ST_SHARED_FILE)
)
storage_units = storage.GetStorageUnits(self._cfg, disk_templates)
self.assertEqual(len(storage_units), len(disk_templates))
--
Raffa Santi
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
