On 10:20 Wed 07 Jul , Iustin Pop wrote:
> On Tue, Jul 06, 2010 at 03:03:27PM +0300, Apollon Oikonomopoulos wrote:
> > Hi,
> >
> > Following patch 06, I changed the way bdev_list is called during disk
> > adoption.
> > Also, gone are all DRBD-related methods (GetSyncStatus(), AttachNet(),
> > DisconnectNet()).
> >
> > The revised patch follows.
>
> Looks good, just a few small comments:
>
> > diff --git a/lib/bdev.py b/lib/bdev.py
> > index c321508..2734af6 100644
> > --- a/lib/bdev.py
> > +++ b/lib/bdev.py
> > @@ -1,4 +1,4 @@
> > -#
> > +
> > #
>
> I think this is an extra change…
Yes, random keystrokes, thanks.
> > + """
> > + def __init__(self, unique_id, children, size):
> > + """Attaches to a static block device.
> > +
> > + The unique_id is a path under /dev.
> > +
> > + """
> > + super(PersistentBlockDevice, self).__init__(unique_id, children, size)
> > + if not isinstance(unique_id, (tuple, list)):
> > + raise ValueError("Invalid configuration data %s" % str(unique_id))
> > + self.dev_path = unique_id[1]
>
> Hmm, why unique_id[1]? What does unique_id[0] contain?
The reasoning is that unique_id is a tuple of the form (storage_driver,
persistent_path). I'm not sure this is the best option, perhaps the
storage driver could be set instance-wide, although a per-disk storage
driver would permit more flexibility. I'll post a follow-up message about
storage driver design, as soon as I get something basic running.
> > + if not os.path.realpath(self.dev_path).startswith('/dev/'):
> > + raise ValueError("Full path '%s' lies outside /dev" %
> > + os.path.realpath(self.dev_path))
>
> This ('/dev/') needs to be abstracted into a constant, that should be
> also used for the comments about the path verification in patch 6/14 (in
> backend.py).
Done, actually I had already introduced constants.BLOCKDEV_ROOT but not
used it here.
>
> > diff --git a/lib/objects.py b/lib/objects.py
> > index d6cf527..6d26bd7 100644
> > --- a/lib/objects.py
> > +++ b/lib/objects.py
> > @@ -396,11 +396,13 @@ class Disk(ConfigObject):
> >
> > def CreateOnSecondary(self):
> > """Test if this device needs to be created on a secondary node."""
> > - return self.dev_type in (constants.LD_DRBD8, constants.LD_LV)
> > + return self.dev_type in (constants.LD_DRBD8, constants.LD_LV,
> > + constants.LD_BLOCK)
> >
> > def AssembleOnSecondary(self):
> > """Test if this device needs to be assembled on a secondary node."""
> > - return self.dev_type in (constants.LD_DRBD8, constants.LD_LV)
> > + return self.dev_type in (constants.LD_DRBD8, constants.LD_LV,
> > + constants.LD_BLOCK)
>
> Not sure about these two (CreateOnSecondary and AssembleOnSecondary), as
> LD_BLOCK do not have a secondary at all :)
>
Yes, these were redundant as well. I removed them and double-checked
that everything works as expected.
> >
> > def OpenOnSecondary(self):
> > """Test if this device needs to be opened on a secondary node."""
> > @@ -419,6 +421,8 @@ class Disk(ConfigObject):
> > """
> > if self.dev_type == constants.LD_LV:
> > return "/dev/%s/%s" % (self.logical_id[0], self.logical_id[1])
> > + elif self.dev_type == constants.LD_BLOCK:
> > + return self.logical_id[1]
>
> Not sure about this, per the first comment.
>
> iustin
Regards,
Apollon
---
This patch introduces basic shared block storage support.
It introduces a new storage backend, bdev.PersistentBlockDevice, to use as a
backend for shared block storage.
A new disk template, DT_BLOCK is introduced as well and added to DTS_EXT_MIRROR
and DTS_MAY_ADOPT.
This is very basic support and includes no storage manipulation (provisioning,
resizing, renaming) which will have to be implemented through a "driver"
framework.
Signed-off-by: Apollon Oikonomopoulos <[email protected]>
---
lib/bdev.py | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
lib/cmdlib.py | 102 ++++++++++++++++++++++++++++++++-------------------
lib/constants.py | 14 ++++---
lib/objects.py | 5 ++-
4 files changed, 184 insertions(+), 45 deletions(-)
diff --git a/lib/bdev.py b/lib/bdev.py
index c321508..bae1c38 100644
--- a/lib/bdev.py
+++ b/lib/bdev.py
@@ -26,6 +26,7 @@ import time
import errno
import pyparsing as pyp
import os
+import stat
import logging
from ganeti import utils
@@ -1954,10 +1955,117 @@ class FileStorage(BlockDev):
return FileStorage(unique_id, children, size)
+class PersistentBlockDevice(BlockDev):
+ """A block device with persistent node
+
+ May be either directly attached, or exposed through DM (e.g. dm-multipath).
+ udev helpers are probably required to give persistent, human-friendly
+ names.
+
+ For the time being, pathnames are required to lie under
+ constants.BLOCKDEV_ROOT.
+
+ """
+ def __init__(self, unique_id, children, size):
+ """Attaches to a static block device.
+
+ The unique_id is a path under /dev.
+
+ """
+ super(PersistentBlockDevice, self).__init__(unique_id, children, size)
+ if not isinstance(unique_id, (tuple, list)):
+ raise ValueError("Invalid configuration data %s" % str(unique_id))
+ self.dev_path = unique_id[1]
+
+ blockdev_root = os.path.normpath(constants.BLOCKDEV_ROOT + '/')
+ if not os.path.abspath(self.dev_path).startswith(blockdev_root):
+ raise ValueError("Full path '%s' lies outside %s" %
+ (os.path.realpath(self.dev_path),
+ constants.BLOCKDEV_ROOT))
+
+ self.major = self.minor = None
+ self.Attach()
+
+ @classmethod
+ def Create(cls, unique_id, children, size):
+ """Create a new device
+
+ This is a noop, we only return a PersistentBlockDevice instance
+
+ """
+ return PersistentBlockDevice(unique_id, children, size=0)
+
+ def Remove(self):
+ """Remove a device
+
+ This is a noop
+
+ """
+ pass
+
+ def Rename(self, new_id):
+ """Rename this device.
+
+ """
+ raise NotImplementedError
+
+ def Attach(self):
+ """Attach to an existing block device.
+
+
+ """
+ self.attached = False
+ try:
+ st = os.stat(self.dev_path)
+ except OSError, err:
+ logging.error("Error stat()'ing %s: %s" % (self.dev_path, str(err)))
+ return False
+
+ if not stat.S_ISBLK(st[stat.ST_MODE]):
+ logging.error("%s is not a block device", self.dev_path)
+ return False
+
+ self.major = os.major(st.st_rdev)
+ self.minor = os.minor(st.st_rdev)
+ self.attached = True
+
+ return True
+
+ def Assemble(self):
+ """Assemble the device.
+
+ """
+ pass
+
+ def Shutdown(self):
+ """Shutdown the device.
+
+ """
+ pass
+
+ def Open(self, force=False):
+ """Make the device ready for I/O.
+
+ """
+ pass
+
+ def Close(self):
+ """Notifies that the device will no longer be used for I/O.
+
+ """
+ pass
+
+ def Grow(self, amount):
+ """Grow the logical volume.
+
+ """
+ raise NotImplementedError
+
DEV_MAP = {
constants.LD_LV: LogicalVolume,
constants.LD_DRBD8: DRBD8,
+ constants.LD_BLOCK: PersistentBlockDevice,
}
if constants.ENABLE_FILE_STORAGE or constants.ENABLE_SHARED_FILE_STORAGE:
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index f6d530d..153cfb5 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -6200,6 +6200,18 @@ def _GenerateDiskTemplate(lu, template_name,
disk_index)),
mode=disk["mode"])
disks.append(disk_dev)
+ elif template_name == constants.DT_BLOCK:
+ if len(secondary_nodes) != 0:
+ raise errors.ProgrammerError("Wrong template configuration")
+
+ for idx, disk in enumerate(disk_info):
+ disk_index = idx + base_index
+ disk_dev = objects.Disk(dev_type=constants.LD_BLOCK, size=disk["size"],
+ logical_id=('dummy', disk["adopt"]),
+ iv_name="disk/%d" % disk_index,
+ mode=disk["mode"])
+ disks.append(disk_dev)
+
else:
raise errors.ProgrammerError("Invalid disk template '%s'" % template_name)
return disks
@@ -6318,6 +6330,7 @@ def _ComputeDiskSize(disk_template, disks):
constants.DT_DRBD8: sum(d["size"] + 128 for d in disks),
constants.DT_FILE: None,
constants.DT_SHARED_FILE: 0,
+ constants.DT_BLOCK: 0,
}
if disk_template not in req_size_dict:
@@ -7062,34 +7075,46 @@ class LUCreateInstance(LogicalUnit):
_CheckNodesFreeDisk(self, nodenames, req_size)
if self.adopt_disks: # instead, we must check the adoption data
- all_lvs = set([i["adopt"] for i in self.disks])
- if len(all_lvs) != len(self.disks):
- raise errors.OpPrereqError("Duplicate volume names given for adoption",
+ all_disks = set([i["adopt"] for i in self.disks])
+ if len(all_disks) != len(self.disks):
+ raise errors.OpPrereqError("Duplicate disk names given for adoption",
errors.ECODE_INVAL)
- for lv_name in all_lvs:
- try:
- self.cfg.ReserveLV(lv_name, self.proc.GetECId())
- except errors.ReservationError:
- raise errors.OpPrereqError("LV named %s used by another instance" %
- lv_name, errors.ECODE_NOTUNIQUE)
-
- node_lvs = self.rpc.call_lv_list([pnode.name],
- self.cfg.GetVGName())[pnode.name]
- node_lvs.Raise("Cannot get LV information from node %s" % pnode.name)
- node_lvs = node_lvs.payload
- delta = all_lvs.difference(node_lvs.keys())
- if delta:
- raise errors.OpPrereqError("Missing logical volume(s): %s" %
- utils.CommaJoin(delta),
- errors.ECODE_INVAL)
- online_lvs = [lv for lv in all_lvs if node_lvs[lv][2]]
- if online_lvs:
- raise errors.OpPrereqError("Online logical volumes found, cannot"
- " adopt: %s" % utils.CommaJoin(online_lvs),
- errors.ECODE_STATE)
- # update the size of disk based on what is found
- for dsk in self.disks:
- dsk["size"] = int(float(node_lvs[dsk["adopt"]][0]))
+ if self.op.disk_template == constants.DT_PLAIN:
+ for lv_name in all_disks:
+ try:
+ self.cfg.ReserveLV(lv_name, self.proc.GetECId())
+ except errors.ReservationError:
+ raise errors.OpPrereqError("LV named %s used by another instance" %
+ lv_name, errors.ECODE_NOTUNIQUE)
+
+ node_lvs = self.rpc.call_lv_list([pnode.name],
+ self.cfg.GetVGName())[pnode.name]
+ node_lvs.Raise("Cannot get LV information from node %s" % pnode.name)
+ node_lvs = node_lvs.payload
+ delta = all_disks.difference(node_lvs.keys())
+ if delta:
+ raise errors.OpPrereqError("Missing logical volume(s): %s" %
+ utils.CommaJoin(delta),
+ errors.ECODE_INVAL)
+ online_lvs = [lv for lv in all_disks if node_lvs[lv][2]]
+ if online_lvs:
+ raise errors.OpPrereqError("Online logical volumes found, cannot"
+ " adopt: %s" %
utils.CommaJoin(online_lvs),
+ errors.ECODE_STATE)
+ # update the size of disk based on what is found
+ for dsk in self.disks:
+ dsk["size"] = int(float(node_lvs[dsk["adopt"]][0]))
+ elif self.op.disk_template == constants.DT_BLOCK:
+ node_disks = self.rpc.call_bdev_list([pnode.name],
+ list(all_disks))[pnode.name]
+ node_disks = node_disks.payload
+ delta = all_disks.difference(node_disks.keys())
+ if delta:
+ raise errors.OpPrereqError("Missing block device(s): %s" %
+ utils.CommaJoin(delta),
+ errors.ECODE_INVAL)
+ for dsk in self.disks:
+ dsk["size"] = int(float(node_disks[dsk["adopt"]]))
_CheckHVParams(self, nodenames, self.op.hypervisor, self.op.hvparams)
@@ -7161,17 +7186,18 @@ class LUCreateInstance(LogicalUnit):
)
if self.adopt_disks:
- # rename LVs to the newly-generated names; we need to construct
- # 'fake' LV disks with the old data, plus the new unique_id
- tmp_disks = [objects.Disk.FromDict(v.ToDict()) for v in disks]
- rename_to = []
- for t_dsk, a_dsk in zip (tmp_disks, self.disks):
- rename_to.append(t_dsk.logical_id)
- t_dsk.logical_id = (t_dsk.logical_id[0], a_dsk["adopt"])
- self.cfg.SetDiskID(t_dsk, pnode_name)
- result = self.rpc.call_blockdev_rename(pnode_name,
- zip(tmp_disks, rename_to))
- result.Raise("Failed to rename adoped LVs")
+ if self.op.disk_template == constants.DT_PLAIN:
+ # rename LVs to the newly-generated names; we need to construct
+ # 'fake' LV disks with the old data, plus the new unique_id
+ tmp_disks = [objects.Disk.FromDict(v.ToDict()) for v in disks]
+ rename_to = []
+ for t_dsk, a_dsk in zip (tmp_disks, self.disks):
+ rename_to.append(t_dsk.logical_id)
+ t_dsk.logical_id = (t_dsk.logical_id[0], a_dsk["adopt"])
+ self.cfg.SetDiskID(t_dsk, pnode_name)
+ result = self.rpc.call_blockdev_rename(pnode_name,
+ zip(tmp_disks, rename_to))
+ result.Raise("Failed to rename adoped LVs")
else:
feedback_fn("* creating instance disks...")
try:
diff --git a/lib/constants.py b/lib/constants.py
index 189354f..f7d0a53 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -319,15 +319,16 @@ DT_PLAIN = "plain"
DT_DRBD8 = "drbd"
DT_FILE = "file"
DT_SHARED_FILE = "sharedfile"
+DT_BLOCK = "blockdev"
# the set of network-mirrored disk templates
DTS_NET_MIRROR = frozenset([DT_DRBD8])
-# the set of externally mirrored disk templates
-DTS_EXT_MIRROR = frozenset([DT_SHARED_FILE])
+# the set of externally-mirrored disk templates (e.g. SAN, NAS)
+DTS_EXT_MIRROR = frozenset([DT_SHARED_FILE, DT_BLOCK])
# the set of non-lvm-based disk templates
-DTS_NOT_LVM = frozenset([DT_DISKLESS, DT_FILE, DT_SHARED_FILE])
+DTS_NOT_LVM = frozenset([DT_DISKLESS, DT_FILE, DT_SHARED_FILE, DT_BLOCK])
# the set of disk templates which can be grown
DTS_GROWABLE = frozenset([DT_PLAIN, DT_DRBD8, DT_FILE, DT_SHARED_FILE])
@@ -339,13 +340,14 @@ DTS_MAY_ADOPT = frozenset([DT_PLAIN])
DTS_MIRRORED = frozenset.union(DTS_NET_MIRROR, DTS_EXT_MIRROR)
# the set of disk templates that allow adoption
-DTS_MAY_ADOPT = frozenset([DT_PLAIN])
+DTS_MAY_ADOPT = frozenset([DT_PLAIN, DT_BLOCK])
# logical disk types
LD_LV = "lvm"
LD_DRBD8 = "drbd8"
LD_FILE = "file"
-LDS_BLOCK = frozenset([LD_LV, LD_DRBD8])
+LD_BLOCK = "blockdev"
+LDS_BLOCK = frozenset([LD_LV, LD_DRBD8, LD_BLOCK])
# drbd constants
DRBD_HMAC_ALG = "md5"
@@ -409,7 +411,7 @@ RIE_CERT_VALIDITY = 24 * 60 * 60
RIE_CONNECT_TIMEOUT = 60
DISK_TEMPLATES = frozenset([DT_DISKLESS, DT_PLAIN, DT_DRBD8,
- DT_FILE, DT_SHARED_FILE])
+ DT_FILE, DT_SHARED_FILE, DT_BLOCK])
FILE_DRIVER = frozenset([FD_LOOP, FD_BLKTAP])
diff --git a/lib/objects.py b/lib/objects.py
index d6cf527..7101785 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -419,6 +419,8 @@ class Disk(ConfigObject):
"""
if self.dev_type == constants.LD_LV:
return "/dev/%s/%s" % (self.logical_id[0], self.logical_id[1])
+ elif self.dev_type == constants.LD_BLOCK:
+ return self.logical_id[1]
return None
def ChildrenNeeded(self):
@@ -446,7 +448,8 @@ class Disk(ConfigObject):
devices needs to (or can) be assembled.
"""
- if self.dev_type in [constants.LD_LV, constants.LD_FILE]
+ if self.dev_type in [constants.LD_LV, constants.LD_FILE,
+ constants.LD_BLOCK]:
result = [node]
elif self.dev_type in constants.LDS_DRBD:
result = [self.logical_id[0], self.logical_id[1]]
--
1.7.1
