Unify the query fields for the storage framework
authorIustin Pop <iustin@google.com>
Tue, 27 Oct 2009 03:56:00 +0000 (12:56 +0900)
committerIustin Pop <iustin@google.com>
Mon, 2 Nov 2009 08:22:22 +0000 (09:22 +0100)
This patch unifies the query fields in the storage framework for all
types. Note that the information is still computed on-demand, so if e.g.
the used disk space is not requested for the ‘file’ type, it won't be
computed on nodes.

Summary of changes:
- improve the LVM storage type to support multiple lvm fields in the
  LIST_FIELDS declaration and constant (not-computed via lvm commands)
  fields
- rename utils.GetFilesystemFreeSpace to utils.GetFilesystemStats
  returning tuple of (total, free)
- add used and free as valid fields for lvm-vg (use being computed as
  vg_size-vg_free)
- make allocatable accepted for all types (ones which are always
  allocatable always return True)
- add a new list field ‘type’ that gives the current selected type; not
  much useful today (except for understanding what the default output
  is) but in the future might help if we want to list multiple types
- add type, size and allocatable to the default output field list
- update the man page with details on how, for file storage, size ≠ used
  + free for non-mountpoint cases

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

lib/cmdlib.py
lib/constants.py
lib/storage.py
lib/utils.py
man/gnt-node.sgml
scripts/gnt-node

index 1b1c37f..8d4a134 100644 (file)
@@ -2618,18 +2618,16 @@ class LUQueryNodeStorage(NoHooksLU):
   """
   _OP_REQP = ["nodes", "storage_type", "output_fields"]
   REQ_BGL = False
-  _FIELDS_STATIC = utils.FieldSet("node")
+  _FIELDS_STATIC = utils.FieldSet(constants.SF_NODE)
 
   def ExpandNames(self):
     storage_type = self.op.storage_type
 
-    if storage_type not in constants.VALID_STORAGE_FIELDS:
+    if storage_type not in constants.VALID_STORAGE_TYPES:
       raise errors.OpPrereqError("Unknown storage type: %s" % storage_type)
 
-    dynamic_fields = constants.VALID_STORAGE_FIELDS[storage_type]
-
     _CheckOutputFields(static=self._FIELDS_STATIC,
-                       dynamic=utils.FieldSet(*dynamic_fields),
+                       dynamic=utils.FieldSet(*constants.VALID_STORAGE_FIELDS),
                        selected=self.op.output_fields)
 
     self.needed_locks = {}
@@ -2661,9 +2659,10 @@ class LUQueryNodeStorage(NoHooksLU):
     else:
       fields = [constants.SF_NAME] + self.op.output_fields
 
-    # Never ask for node as it's only known to the LU
-    while "node" in fields:
-      fields.remove("node")
+    # Never ask for node or type as it's only known to the LU
+    for extra in [constants.SF_NODE, constants.SF_TYPE]:
+      while extra in fields:
+        fields.remove(extra)
 
     field_idx = dict([(name, idx) for (idx, name) in enumerate(fields)])
     name_idx = field_idx[constants.SF_NAME]
@@ -2693,8 +2692,10 @@ class LUQueryNodeStorage(NoHooksLU):
         out = []
 
         for field in self.op.output_fields:
-          if field == "node":
+          if field == constants.SF_NODE:
             val = node
+          elif field == constants.SF_TYPE:
+            val = self.op.storage_type
           elif field in field_idx:
             val = row[field_idx[field]]
           else:
@@ -2722,7 +2723,7 @@ class LUModifyNodeStorage(NoHooksLU):
     self.op.node_name = node_name
 
     storage_type = self.op.storage_type
-    if storage_type not in constants.VALID_STORAGE_FIELDS:
+    if storage_type not in constants.VALID_STORAGE_TYPES:
       raise errors.OpPrereqError("Unknown storage type: %s" % storage_type)
 
   def ExpandNames(self):
index ee995a0..815e666 100644 (file)
@@ -196,6 +196,10 @@ ST_LVM_PV = "lvm-pv"
 ST_LVM_VG = "lvm-vg"
 
 # Storage fields
+# first two are valid in LU context only, not passed to backend
+SF_NODE = "node"
+SF_TYPE = "type"
+# and the rest are valid in backend
 SF_NAME = "name"
 SF_SIZE = "size"
 SF_FREE = "free"
@@ -206,11 +210,10 @@ SF_ALLOCATABLE = "allocatable"
 SO_FIX_CONSISTENCY = "fix-consistency"
 
 # Available fields per storage type
-VALID_STORAGE_FIELDS = {
-  ST_FILE: frozenset([SF_NAME, SF_USED, SF_FREE]),
-  ST_LVM_PV: frozenset([SF_NAME, SF_SIZE, SF_USED, SF_FREE, SF_ALLOCATABLE]),
-  ST_LVM_VG: frozenset([SF_NAME, SF_SIZE]),
-  }
+VALID_STORAGE_FIELDS = frozenset([SF_NAME, SF_TYPE, SF_SIZE,
+                                  SF_USED, SF_FREE, SF_ALLOCATABLE])
+
+VALID_STORAGE_TYPES = frozenset([ST_FILE, ST_LVM_PV, ST_LVM_VG])
 
 MODIFIABLE_STORAGE_FIELDS = {
   ST_LVM_PV: frozenset([SF_ALLOCATABLE]),
index d442108..7f73100 100644 (file)
@@ -125,10 +125,10 @@ class FileStorage(_Base):
     else:
       dirsize = None
 
-    if constants.SF_FREE in fields:
-      fsfree = utils.GetFreeFilesystemSpace(path)
+    if constants.SF_FREE in fields or constants.SF_SIZE in fields:
+      fsstats = utils.GetFilesystemStats(path)
     else:
-      fsfree = None
+      fsstats = None
 
     # Make sure to update constants.VALID_STORAGE_FIELDS when changing fields.
     for field_name in fields:
@@ -139,7 +139,13 @@ class FileStorage(_Base):
         values.append(dirsize)
 
       elif field_name == constants.SF_FREE:
-        values.append(fsfree)
+        values.append(fsstats[1])
+
+      elif field_name == constants.SF_SIZE:
+        values.append(fsstats[0])
+
+      elif field_name == constants.SF_ALLOCATABLE:
+        values.append(True)
 
       else:
         raise errors.StorageError("Unknown field: %r" % field_name)
@@ -150,6 +156,11 @@ class FileStorage(_Base):
 class _LvmBase(_Base):
   """Base class for LVM storage containers.
 
+  @cvar LIST_FIELDS: list of tuples consisting of three elements: SF_*
+      constants, lvm command output fields (list), and conversion
+      function or static value (for static value, the lvm output field
+      can be an empty list)
+
   """
   LIST_SEP = "|"
   LIST_COMMAND = None
@@ -200,9 +211,9 @@ class _LvmBase(_Base):
       except IndexError:
         raise errors.StorageError("Unknown field: %r" % field_name)
 
-      (_, lvm_name, _) = fields_def[idx]
+      (_, lvm_names, _) = fields_def[idx]
 
-      lvm_fields.append(lvm_name)
+      lvm_fields.extend(lvm_names)
 
     return utils.UniqueSequence(lvm_fields)
 
@@ -231,14 +242,24 @@ class _LvmBase(_Base):
       row = []
 
       for field_name in wanted_field_names:
-        (_, lvm_name, convert_fn) = fields_def[field_to_idx[field_name]]
+        (_, lvm_names, mapper) = fields_def[field_to_idx[field_name]]
 
-        value = raw_data[lvm_name_to_idx[lvm_name]]
+        values = [raw_data[lvm_name_to_idx[i]] for i in lvm_names]
 
-        if convert_fn:
-          value = convert_fn(value)
+        if callable(mapper):
+          # we got a function, call it with all the declared fields
+          val = mapper(*values)
+        elif len(values) == 1:
+          # we don't have a function, but we had a single field
+          # declared, pass it unchanged
+          val = values[0]
+        else:
+          # let's make sure there are no fields declared (cannot map >
+          # 1 field without a function)
+          assert not values, "LVM storage has multi-fields without a function"
+          val = mapper
 
-        row.append(value)
+        row.append(val)
 
       data.append(row)
 
@@ -297,6 +318,7 @@ class _LvmBase(_Base):
       fields = line.strip().split(sep)
 
       if len(fields) != fieldcount:
+        logging.warning("Invalid line returned from lvm command: %s", line)
         continue
 
       yield fields
@@ -318,11 +340,11 @@ class LvmPvStorage(_LvmBase):
   # Make sure to update constants.VALID_STORAGE_FIELDS when changing field
   # definitions.
   LIST_FIELDS = [
-    (constants.SF_NAME, "pv_name", None),
-    (constants.SF_SIZE, "pv_size", _ParseSize),
-    (constants.SF_USED, "pv_used", _ParseSize),
-    (constants.SF_FREE, "pv_free", _ParseSize),
-    (constants.SF_ALLOCATABLE, "pv_attr", _GetAllocatable),
+    (constants.SF_NAME, ["pv_name"], None),
+    (constants.SF_SIZE, ["pv_size"], _ParseSize),
+    (constants.SF_USED, ["pv_used"], _ParseSize),
+    (constants.SF_FREE, ["pv_free"], _ParseSize),
+    (constants.SF_ALLOCATABLE, ["pv_attr"], _GetAllocatable),
     ]
 
   def _SetAllocatable(self, name, allocatable):
@@ -372,8 +394,12 @@ class LvmVgStorage(_LvmBase):
   # Make sure to update constants.VALID_STORAGE_FIELDS when changing field
   # definitions.
   LIST_FIELDS = [
-    (constants.SF_NAME, "vg_name", None),
-    (constants.SF_SIZE, "vg_size", _ParseSize),
+    (constants.SF_NAME, ["vg_name"], None),
+    (constants.SF_SIZE, ["vg_size"], _ParseSize),
+    (constants.SF_FREE, ["vg_free"], _ParseSize),
+    (constants.SF_USED, ["vg_size", "vg_free"],
+     lambda x, y: _ParseSize(x) - _ParseSize(y)),
+    (constants.SF_ALLOCATABLE, [], True),
     ]
 
   def _RemoveMissing(self, name):
index 56d0108..2466a87 100644 (file)
@@ -1890,18 +1890,20 @@ def CalculateDirectorySize(path):
   return BytesToMebibyte(size)
 
 
-def GetFreeFilesystemSpace(path):
-  """Returns the free space on a filesystem.
+def GetFilesystemStats(path):
+  """Returns the total and free space on a filesystem.
 
   @type path: string
   @param path: Path on filesystem to be examined
   @rtype: int
-  @return: Free space in mebibytes
+  @return: tuple of (Total space, Free space) in mebibytes
 
   """
   st = os.statvfs(path)
 
-  return BytesToMebibyte(st.f_bavail * st.f_frsize)
+  fsize = BytesToMebibyte(st.f_bavail * st.f_frsize)
+  tsize = BytesToMebibyte(st.f_blocks * st.f_frsize)
+  return (tsize, fsize)
 
 
 def LockedMethod(fn):
index 44ab6c2..a62edd0 100644 (file)
@@ -794,8 +794,7 @@ node1.example.com /dev/hdc1 xenvg instance1.example.com-sda_11001.data 256  inst
       <para>
         The <option>--storage-type</option> option can be used to choose a
         storage unit type. Possible choices are <literal>lvm-pv</literal>,
-        <literal>lvm-vg</literal> or <literal>file</literal>. Depending on the
-        storage type, the available output fields change.
+        <literal>lvm-vg</literal> or <literal>file</literal>.
       </para>
 
       <para>
@@ -809,17 +808,24 @@ node1.example.com /dev/hdc1 xenvg instance1.example.com-sda_11001.data 256  inst
             </listitem>
           </varlistentry>
           <varlistentry>
+            <term>type</term>
+            <listitem>
+              <simpara>the type of the storage unit (currently just
+              what is passed in via
+              <option>--storage-type</option>)</simpara>
+            </listitem>
+          </varlistentry>
+          <varlistentry>
             <term>name</term>
             <listitem>
-              <simpara>the physical drive name</simpara>
+              <simpara>the path/identifier of the storage unit</simpara>
             </listitem>
           </varlistentry>
           <varlistentry>
             <term>size</term>
             <listitem>
               <simpara>
-                the physical drive size
-                (<literal>lvm-pv</literal> and <literal>lvm-vg</literal> only)
+                total size of the unit; for the file type see a note below
               </simpara>
             </listitem>
           </varlistentry>
@@ -827,8 +833,7 @@ node1.example.com /dev/hdc1 xenvg instance1.example.com-sda_11001.data 256  inst
             <term>used</term>
             <listitem>
               <simpara>
-                used disk space
-                (<literal>lvm-pv</literal> and <literal>file</literal> only)
+                used space in the unit; for the file type see a note below
               </simpara>
             </listitem>
           </varlistentry>
@@ -837,7 +842,6 @@ node1.example.com /dev/hdc1 xenvg instance1.example.com-sda_11001.data 256  inst
             <listitem>
               <simpara>
                 available disk space
-                (<literal>lvm-pv</literal> and <literal>file</literal> only)
               </simpara>
             </listitem>
           </varlistentry>
@@ -845,8 +849,9 @@ node1.example.com /dev/hdc1 xenvg instance1.example.com-sda_11001.data 256  inst
             <term>allocatable</term>
             <listitem>
               <simpara>
-                whether physical volume is allocatable
-                (<literal>lvm-pv</literal> only)
+                whether we the unit is available for allocation
+                (only <literal>lvm-pv</literal> can change this
+                setting, the other types always report true)
               </simpara>
             </listitem>
           </varlistentry>
@@ -854,12 +859,25 @@ node1.example.com /dev/hdc1 xenvg instance1.example.com-sda_11001.data 256  inst
       </para>
 
       <para>
+        Note that for the <quote>file</quote> type, the total disk
+        space might not equal to the sum of used and free, due to the
+        method Ganeti uses to compute each of them. The total and free
+        values are computed as the total and free space values for the
+        filesystem to which the directory belongs, but the used space
+        is computed from the used space under that directory
+        <emphasis>only</emphasis>, which might not be necessarily the
+        root of the filesystem, and as such there could be files
+        outside the file storage directory using disk space and
+        causing a mismatch in the values.
+      </para>
+
+      <para>
         Example:
         <screen>
-# gnt-node list-storage node5.example.com
-Node              Name        Size Used   Free
-node5.example.com /dev/sda7 673.8G   0M 673.8G
-node5.example.com /dev/sdb1 698.6G 1.3G 697.4G
+node1# gnt-node list-storage node2
+Node  Type   Name        Size Used   Free Allocatable
+node2 lvm-pv /dev/sda7 673.8G 1.5G 672.3G Y
+node2 lvm-pv /dev/sdb1 698.6G   0M 698.6G Y
         </screen>
       </para>
     </refsect2>
index 9febd42..6b54778 100755 (executable)
@@ -41,6 +41,19 @@ _LIST_DEF_FIELDS = [
   "pinst_cnt", "sinst_cnt",
   ]
 
+
+#: default list of field for L{ListStorage}
+_LIST_STOR_DEF_FIELDS = [
+  constants.SF_NODE,
+  constants.SF_TYPE,
+  constants.SF_NAME,
+  constants.SF_SIZE,
+  constants.SF_USED,
+  constants.SF_FREE,
+  constants.SF_ALLOCATABLE,
+  ]
+
+
 #: headers (and full field list for L{ListNodes}
 _LIST_HEADERS = {
   "name": "Node", "pinst_cnt": "Pinst", "sinst_cnt": "Sinst",
@@ -59,6 +72,19 @@ _LIST_HEADERS = {
   "ctime": "CTime", "mtime": "MTime", "uuid": "UUID"
   }
 
+
+#: headers (and full field list for L{ListStorage}
+_LIST_STOR_HEADERS = {
+  constants.SF_NODE: "Node",
+  constants.SF_TYPE: "Type",
+  constants.SF_NAME: "Name",
+  constants.SF_SIZE: "Size",
+  constants.SF_USED: "Used",
+  constants.SF_FREE: "Free",
+  constants.SF_ALLOCATABLE: "Allocatable",
+  }
+
+
 #: User-facing storage unit types
 _USER_STORAGE_TYPE = {
   constants.ST_FILE: "file",
@@ -476,29 +502,10 @@ def ListStorage(opts, args):
 
   storage_type = ConvertStorageType(opts.user_storage_type)
 
-  default_fields = {
-    constants.ST_FILE: [
-      constants.SF_NAME,
-      constants.SF_USED,
-      constants.SF_FREE,
-      ],
-    constants.ST_LVM_PV: [
-      constants.SF_NAME,
-      constants.SF_SIZE,
-      constants.SF_USED,
-      constants.SF_FREE,
-      ],
-    constants.ST_LVM_VG: [
-      constants.SF_NAME,
-      constants.SF_SIZE,
-      ],
-  }
-
-  def_fields = ["node"] + default_fields[storage_type]
   if opts.output is None:
-    selected_fields = def_fields
+    selected_fields = _LIST_STOR_DEF_FIELDS
   elif opts.output.startswith("+"):
-    selected_fields = def_fields + opts.output[1:].split(",")
+    selected_fields = _LIST_STOR_DEF_FIELDS + opts.output[1:].split(",")
   else:
     selected_fields = opts.output.split(",")
 
@@ -509,7 +516,8 @@ def ListStorage(opts, args):
 
   if not opts.no_headers:
     headers = {
-      "node": "Node",
+      constants.SF_NODE: "Node",
+      constants.SF_TYPE: "Type",
       constants.SF_NAME: "Name",
       constants.SF_SIZE: "Size",
       constants.SF_USED: "Used",
@@ -568,6 +576,8 @@ def ModifyStorage(opts, args):
                                      name=volume_name,
                                      changes=changes)
     SubmitOpCode(op)
+  else:
+    ToStderr("No changes to perform, exiting.")
 
 
 def RepairStorage(opts, args):
@@ -683,7 +693,9 @@ commands = {
   'list-storage': (
     ListStorage, ARGS_MANY_NODES,
     [NOHDR_OPT, SEP_OPT, USEUNITS_OPT, FIELDS_OPT, _STORAGE_TYPE_OPT],
-    "[<node_name>...]", "List physical volumes on node(s)"),
+    "[<node_name>...]", "List physical volumes on node(s). The available"
+    " fields are (see the man page for details): %s." %
+    (", ".join(_LIST_STOR_HEADERS))),
   'modify-storage': (
     ModifyStorage,
     [ArgNode(min=1, max=1),