On Mon, May 20, 2013 at 5:11 PM, Bernardo Dal Seno <[email protected]>wrote:
> The requested number of spindles is used to allocate PVs when creating new > LVs. > > Signed-off-by: Bernardo Dal Seno <[email protected]> > --- > lib/storage/base.py | 7 +++++-- > lib/storage/bdev.py | 24 ++++++++++++++++-------- > lib/storage/drbd.py | 2 +- > 3 files changed, 22 insertions(+), 11 deletions(-) > > diff --git a/lib/storage/base.py b/lib/storage/base.py > index 385a78a..e38839d 100644 > --- a/lib/storage/base.py > +++ b/lib/storage/base.py > @@ -109,7 +109,7 @@ class BlockDev(object): > raise NotImplementedError > > @classmethod > - def Create(cls, unique_id, children, size, params, excl_stor): > + def Create(cls, unique_id, children, size, spindles, params, excl_stor): > """Create the device. > > If the device cannot be created, it will return None > @@ -120,11 +120,14 @@ class BlockDev(object): > enough for both creation and assembly (later). > > @type unique_id: 2-element tuple or list > - @param unique_id: unique identifier; the details depend on the actual > device type > + @param unique_id: unique identifier; the details depend on the actual > device > + type > Should go in previous patch. > @type children: list of L{BlockDev} > @param children: for hierarchical devices, the child devices > @type size: float > @param size: size in MiB > + @type spindles: int > + @param spindles: number of physical disk to dedicate to the device > As far as I've understood, the spindles parameter can only ever be used by LogicalVolume and is ignored by all the other sub-classes of BlockDev. The documentation of BlockDev.Create's params parameter says "...params: device-specific options/parameters". So why do we introduce a new (required) parameter here instead of putting the spindles option in params, where it could be LogicalVolume specific? This comment goes beyond this patch, as previous ones have already added "spindles" to the Disk class. But the reasoning is the same: should we add a specific field, which is only valid for a specific disk type, to the Disk class, which IMHO should only hold generic disk information? > @type params: dict > @param params: device-specific options/parameters > @type excl_stor: bool > diff --git a/lib/storage/bdev.py b/lib/storage/bdev.py > index 404ec9c..e3ab383 100644 > --- a/lib/storage/bdev.py > +++ b/lib/storage/bdev.py > @@ -230,7 +230,7 @@ class LogicalVolume(base.BlockDev): > return map((lambda pv: pv.name), empty_pvs) > > @classmethod > - def Create(cls, unique_id, children, size, params, excl_stor): > + def Create(cls, unique_id, children, size, spindles, params, excl_stor): > """Create a new logical volume. > > """ > @@ -265,11 +265,19 @@ class LogicalVolume(base.BlockDev): > for m in err_msgs: > logging.warning(m) > req_pvs = cls._ComputeNumPvs(size, pvs_info) > + if spindles: > + if spindles < req_pvs: > + base.ThrowError("Requested number of spindles (%s) is not > enough for" > + " a disk of %d MB (at least %d spindles > needed)", > + spindles, size, req_pvs) > + else: > + req_pvs = spindles > pvlist = cls._GetEmptyPvNames(pvs_info, req_pvs) > current_pvs = len(pvlist) > if current_pvs < req_pvs: > - base.ThrowError("Not enough empty PVs to create a disk of %d MB:" > - " %d available, %d needed", size, current_pvs, > req_pvs) > + base.ThrowError("Not enough empty PVs (spindles) to create a disk > of %d" > + " MB: %d available, %d needed", > + size, current_pvs, req_pvs) > assert current_pvs == len(pvlist) > if stripes > current_pvs: > # No warning issued for this, as it's no surprise > @@ -836,7 +844,7 @@ class FileStorage(base.BlockDev): > base.ThrowError("Can't stat %s: %s", self.dev_path, err) > > @classmethod > - def Create(cls, unique_id, children, size, params, excl_stor): > + def Create(cls, unique_id, children, size, spindles, params, excl_stor): > """Create a new file. > > @param size: the size of file in MiB > @@ -904,7 +912,7 @@ class PersistentBlockDevice(base.BlockDev): > self.Attach() > > @classmethod > - def Create(cls, unique_id, children, size, params, excl_stor): > + def Create(cls, unique_id, children, size, spindles, params, excl_stor): > """Create a new device > > This is a noop, we only return a PersistentBlockDevice instance > @@ -1004,7 +1012,7 @@ class RADOSBlockDevice(base.BlockDev): > self.Attach() > > @classmethod > - def Create(cls, unique_id, children, size, params, excl_stor): > + def Create(cls, unique_id, children, size, spindles, params, excl_stor): > """Create a new rbd device. > > Provision a new rbd volume inside a RADOS pool. > @@ -1358,7 +1366,7 @@ class ExtStorageDevice(base.BlockDev): > self.Attach() > > @classmethod > - def Create(cls, unique_id, children, size, params, excl_stor): > + def Create(cls, unique_id, children, size, spindles, params, excl_stor): > """Create a new extstorage device. > > Provision a new volume using an extstorage provider, which will > @@ -1803,5 +1811,5 @@ def Create(disk, children, excl_stor): > _VerifyDiskType(disk.dev_type) > _VerifyDiskParams(disk) > device = DEV_MAP[disk.dev_type].Create(disk.physical_id, children, > disk.size, > - disk.params, excl_stor) > + disk.spindles, disk.params, > excl_stor) > return device > diff --git a/lib/storage/drbd.py b/lib/storage/drbd.py > index c50a217..a74a170 100644 > --- a/lib/storage/drbd.py > +++ b/lib/storage/drbd.py > @@ -1001,7 +1001,7 @@ class DRBD8Dev(base.BlockDev): > base.ThrowError("Can't initialize meta device: %s", result.output) > > @classmethod > - def Create(cls, unique_id, children, size, params, excl_stor): > + def Create(cls, unique_id, children, size, spindles, params, excl_stor): > """Create a new DRBD8 device. > > Since DRBD devices are not created per se, just assembled, this > -- > 1.8.2.1 > > -- Thomas Thrainer | Software Engineer | [email protected] | Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Katherine Stephens
