On Mon, May 20, 2013 at 5:11 PM, Bernardo Dal Seno <[email protected]>wrote:
> The option is parsed but ignored, for the moment. > > Signed-off-by: Bernardo Dal Seno <[email protected]> > --- > lib/cli.py | 3 +++ > lib/cmdlib/instance_storage.py | 1 + > lib/constants.py | 2 ++ > man/gnt-instance.rst | 13 +++++++++---- > 4 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/lib/cli.py b/lib/cli.py > index 32ef069..0bddf43 100644 > --- a/lib/cli.py > +++ b/lib/cli.py > @@ -2647,6 +2647,9 @@ def GenericInstanceCreate(mode, opts, args): > raise errors.OpPrereqError("Invalid disk size for disk %d: %s" % > (didx, err), errors.ECODE_INVAL) > elif constants.IDISK_ADOPT in ddict: > + if constants.IDISK_SPINDLES in ddict: > + raise errors.OpPrereqError("spindles is not a valid option when" > + " adopting a disk", > errors.ECODE_INVAL) > That a bit strange to read, as the if goes like if IDISK_SIZE if ADOPT error ... do stuff ... elif ADOPT if SPINDLES error ... do stuff ... wouldn't it be more readable to do if ADOPT if IDISK_SIZE error if SPINDLES error ... do stuff ... elif IDISK_SIZE ... do stuff ... It's just that the same condition (IDISK_ADOPT) is checked twice, in two different levels, which makes it a bit hard to follow. > if mode == constants.INSTANCE_IMPORT: > raise errors.OpPrereqError("Disk adoption not allowed for > instance" > " import", errors.ECODE_INVAL) > diff --git a/lib/cmdlib/instance_storage.py > b/lib/cmdlib/instance_storage.py > index f6561e3..76fc529 100644 > --- a/lib/cmdlib/instance_storage.py > +++ b/lib/cmdlib/instance_storage.py > @@ -526,6 +526,7 @@ class LUInstanceRecreateDisks(LogicalUnit): > _MODIFYABLE = compat.UniqueFrozenset([ > constants.IDISK_SIZE, > constants.IDISK_MODE, > + constants.IDISK_SPINDLES, > ]) > > # New or changed disk parameters may have different semantics > diff --git a/lib/constants.py b/lib/constants.py > index d4022b2..c17261a 100644 > --- a/lib/constants.py > +++ b/lib/constants.py > @@ -1326,6 +1326,7 @@ NICS_PARAMETERS = > frozenset(NICS_PARAMETER_TYPES.keys()) > > # IDISK_* constants are used in opcodes, to create/change disks > IDISK_SIZE = "size" > +IDISK_SPINDLES = "spindles" > IDISK_MODE = "mode" > IDISK_ADOPT = "adopt" > IDISK_VG = "vg" > @@ -1334,6 +1335,7 @@ IDISK_PROVIDER = "provider" > IDISK_NAME = "name" > IDISK_PARAMS_TYPES = { > IDISK_SIZE: VTYPE_SIZE, > + IDISK_SPINDLES: VTYPE_INT, > IDISK_MODE: VTYPE_STRING, > IDISK_ADOPT: VTYPE_STRING, > IDISK_VG: VTYPE_STRING, > diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst > index 793b664..5e7cb25 100644 > --- a/man/gnt-instance.rst > +++ b/man/gnt-instance.rst > @@ -28,7 +28,7 @@ ADD > > | **add** > | {-t|\--disk-template {diskless \| file \| plain \| drbd \| rbd}} > -| {\--disk=*N*: {size=*VAL* \| adopt=*LV*}[,options...] > +| {\--disk=*N*: {size=*VAL*[,spindles=*VAL*] \| adopt=*LV*}[,options...] > | \| {size=*VAL*,provider=*PROVIDER*}[,param=*value*... ][,options...] > | \| {-s|\--os-size} *SIZE*} > | [\--no-ip-check] [\--no-name-check] [\--no-conflicts-check] > @@ -57,6 +57,9 @@ given) in mebibytes. You can also use one of the > suffixes *m*, *g* or > mebibytes, gibibytes and tebibytes. Each disk can also take these > parameters (all optional): > > +spindles > + How many spindles (physical disks on the node) the disk should span. > + > mode > The access mode. Either ``ro`` (read-only) or the default ``rw`` > (read-write). > @@ -1103,7 +1106,8 @@ by ballooning it up or down to the new value. > The ``--disk add:size=*SIZE*,[options..]`` option adds a disk to the > instance, and ``--disk *N*:add:size=*SIZE*,[options..]`` will add a disk > to the the instance at a specific index. The available options are the > -same as in the **add** command(``mode``, ``name``, ``vg``, ``metavg``). > +same as in the **add** command(``spindles``, ``mode``, ``name``, ``vg``, > +``metavg``). > When adding an ExtStorage disk the ``provider=*PROVIDER*`` option is > also mandatory and specifies the ExtStorage provider. Also, for > ExtStorage disks arbitrary parameters can be passed as additional comma > @@ -1616,7 +1620,7 @@ RECREATE-DISKS > > | **recreate-disks** [\--submit] > | [{-n node1:[node2] \| {-I\|\--iallocator *name*}}] > -| [\--disk=*N*[:[size=*VAL*][,mode=*ro\|rw*]]] {*instance*} > +| [\--disk=*N*[:[size=*VAL*][,spindles=*VAL*][,mode=*ro\|rw*]]] > {*instance*} > Recreates all or a subset of disks of the given instance. > > @@ -1627,7 +1631,8 @@ normal operation and as such the impact of this is > low. > > If only a subset should be recreated, any number of ``disk`` options can > be specified. It expects a disk index and an optional list of disk > -parameters to change. Only ``size`` and ``mode`` can be changed while > +parameters to change. Only ``size``, ``spindles``, and ``mode`` can be > +changed while > Why not fill the line? > recreating disks. To recreate all disks while changing parameters on > a subset only, a ``--disk`` option must be given for every disk of the > instance. > -- > 1.8.2.1 > > Rest LGTM, Thanks. -- 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
