storagemanager-devel — System Storage Manager mailing list

[ssm-devel] [PATCH 2/4] multipath: disallow multipath as default backend

[Jan T. <jt...@re...>]

As multipath is a read-only, device-only backend, it makes no sense
to use it as a default backend (-b multipath). Do the same thing we
already do for md backend and remove it from valid options to prevent
misunderstandings and errors.

Resolves: rhbz#1685019

Signed-off-by: Jan Tulak <jt...@re...>
---
 ssmlib/main.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ssmlib/main.py b/ssmlib/main.py
index b117652..abb39e2 100644
--- a/ssmlib/main.py
+++ b/ssmlib/main.py
@@ -38,7 +38,7 @@ except ImportError:
 
 EXTN = ['ext2', 'ext3', 'ext4']
 SUPPORTED_FS = ['xfs', 'btrfs'] + EXTN
-SUPPORTED_BACKENDS = ['lvm', 'btrfs', 'crypt', 'multipath']
+SUPPORTED_BACKENDS = ['lvm', 'btrfs', 'crypt']
 SUPPORTED_RAID = ['0', '1', '10']
 os.environ['LC_ALL'] = "C"
 
-- 
2.21.0

[ssm-devel] [PATCH 3/4] tests: do not use info command in multipath test

[Jan T. <jt...@re...>]

There is no reason for using it ATM.

Signed-off-by: Jan Tulak <jt...@re...>
---
 tests/bashtests/016-multipath.sh | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tests/bashtests/016-multipath.sh b/tests/bashtests/016-multipath.sh
index 4187af3..4180e6c 100755
--- a/tests/bashtests/016-multipath.sh
+++ b/tests/bashtests/016-multipath.sh
@@ -70,9 +70,6 @@ not ssm remove $MPATH
 
 
 
-ssm info $MPATH
-
-
 
 
 
-- 
2.21.0

[ssm-devel] [PATCH 1/4] multipath: allow any string as mp dev id

[Jan T. <jt...@re...>]

The regexes for multipath devices allowed only for hex strings, but in
some cases, any string can appear there. This patch changes the regexes
to detect such devices too.

Signed-off-by: Jan Tulak <jt...@re...>
---
 ssmlib/backends/multipath.py      | 4 ++--
 tests/unittests/test_multipath.py | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/ssmlib/backends/multipath.py b/ssmlib/backends/multipath.py
index d34730a..66d9e80 100644
--- a/ssmlib/backends/multipath.py
+++ b/ssmlib/backends/multipath.py
@@ -73,7 +73,7 @@ class Multipath(template.Backend):
         command = [MP, '-ll']
         try:
             output = misc.run(command, stderr=False, can_fail=True)[1].split("\n")
-            pattern = re.compile(r"^([a-z0-9]+) \([0-9a-f]+\)")
+            pattern = re.compile(r"^([a-z0-9]+) \([^)]+\)")
         except (problem.CommandFailed, OSError):
             # probably multipath not installed
             output = []
@@ -97,7 +97,7 @@ class Multipath(template.Backend):
             output = []
 
         if len(output) > 0:
-            match = re.search(r"\(([0-9a-f]+)\)",output[0])
+            match = re.search(r"\(([^)]+)\)",output[0])
             data['wwid'] = match.group(1)
             data['dev_size'] = misc.get_device_size(data['dev_name'])
             data['nodes'] = []
diff --git a/tests/unittests/test_multipath.py b/tests/unittests/test_multipath.py
index 39802e1..7239c2a 100644
--- a/tests/unittests/test_multipath.py
+++ b/tests/unittests/test_multipath.py
@@ -100,7 +100,8 @@ class MultipathFunctionCheck(MockSystemDataSource):
                 counter += 1
                 if mp_vol != None and mp_vol != mp_name:
                     continue
-                output += "{0} (360000000000000000e00000000010001) {1} QEMU    ,QEMU HARDDISK \n".format(mp_name, basename(v_data['real_dev']))
+                mp_id="XX360000000000000000e0000000"+chr(ord('a')+counter)
+                output += "{name} ({id}) {dev} QEMU    ,QEMU HARDDISK \n".format(name=mp_name, id=mp_id, dev=basename(v_data['real_dev']))
                 output += "size={0} features='0' hwhandler='0' wp=rw\n".format(size_converted)
 
                 devs = []
-- 
2.21.0

[ssm-devel] [PATCH 2/3] ssmlib: fix sizes in xfs_get_info for mounted devices

[Jan T. <jt...@re...>]

ssm usually works with sizes in kb, but statvfs reports in bytes,
so the values we got for mounted xfs were off by an order. A simple
division by 1024 brings that into order.

Signed-off-by: Jan Tulak <jt...@re...>
---
 ssmlib/main.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ssmlib/main.py b/ssmlib/main.py
index 80e39e7..b117652 100644
--- a/ssmlib/main.py
+++ b/ssmlib/main.py
@@ -353,9 +353,9 @@ class FsInfo(object):
         mount_point = misc.get_mounts(dev).get(realdev, {}).get('mp', None)
         if mount_point:
             stat = os.statvfs(mount_point)
-            total = stat.f_blocks*stat.f_bsize
-            free = stat.f_bfree*stat.f_bsize
-            used = (stat.f_blocks-stat.f_bfree)*stat.f_bsize
+            total = stat.f_blocks*stat.f_bsize/1024
+            free = stat.f_bfree*stat.f_bsize/1024
+            used = (stat.f_blocks-stat.f_bfree)*stat.f_bsize/1024
             self.data['fs_size'] = total
             self.data['fs_free'] = free
             self.data['fs_used'] = used
-- 
2.21.0

[ssm-devel] [PATCH 3/3] bashtests: test for delay in detecting xfs resize

[Jan T. <jt...@re...>]

A test for issue rbhz#1664652, where ssm had a delay in detecting
changed sizes for xfs partitions.

Signed-off-by: Jan Tulak <jt...@re...>
---
 tests/bashtests/015-xfs-resize.sh | 75 +++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100755 tests/bashtests/015-xfs-resize.sh

diff --git a/tests/bashtests/015-xfs-resize.sh b/tests/bashtests/015-xfs-resize.sh
new file mode 100755
index 0000000..1b91d5e
--- /dev/null
+++ b/tests/bashtests/015-xfs-resize.sh
@@ -0,0 +1,75 @@
+#!/bin/bash
+#
+# (C)2019 Red Hat, Inc., Jan Tulak <jt...@re...>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+export test_name='014-xfs-resize'
+test_description='Ensure that resize is reflected immediately in filesystem list'
+
+. lib/test
+
+DEV_COUNT=1
+DEV_SIZE=400
+# The real size of the device which lvm will use is smaller
+TEST_MAX_SIZE=$(($DEV_COUNT*($DEV_SIZE-4)))
+aux prepare_devs $DEV_COUNT $DEV_SIZE
+aux prepare_mnts 1
+TEST_DEVS=$(cat DEVICES)
+export SSM_DEFAULT_BACKEND='lvm'
+export SSM_LVM_DEFAULT_POOL=$vg1
+export LVOL_PREFIX="lvol"
+export SSM_NONINTERACTIVE='1'
+lvol1=${LVOL_PREFIX}001
+
+DEFAULT_VOLUME=${SSM_LVM_DEFAULT_POOL}/$lvol1
+TEST_MNT=$TESTDIR/mnt
+
+smallsize=$(($DEV_SIZE/2))
+step=$(($smallsize/4))
+steps=3
+fullpath="$TESTDIR/dev//$SSM_LVM_DEFAULT_POOL/$lvol1"
+
+function check_sizes() {
+    pool="$1"
+    vol="$2"
+    fullpath="$TESTDIR/dev/$pool/$vol"
+
+    output=$(ssm list fs)
+    ssm list fs
+    dfout=$(df $fullpath | tail -n1)
+    fssize=$(echo "$dfout" | awk '{printf "%0.2f", $2/1024}')
+    fsfree=$(echo "$dfout" | awk '{printf "%0.2f", $4/1024}')
+    check list_table "$output" $pool/$vol none none none ${fssize}MB ${fsfree}MB
+
+}
+
+ssm create -s ${smallsize}M --fstype xfs $dev1 $mnt1
+check lv_field $SSM_LVM_DEFAULT_POOL/$lvol1 lv_size ${smallsize}.00m
+# before resize
+check_sizes $SSM_LVM_DEFAULT_POOL $lvol1
+
+# try it in a few steps, because we might be lucky with timing even when
+# the issue is present
+for i in $(seq 1 $steps); do
+    echo "iteration $i"
+    # After resize, the size should match what the rest of the system sees
+    # immediately.
+    ssm resize -s +${step}M $SSM_LVM_DEFAULT_POOL/$lvol1
+    check_sizes $SSM_LVM_DEFAULT_POOL $lvol1
+done
+umount $mnt1
+ssm -f remove -a
+
+exit 0
\ No newline at end of file
-- 
2.21.0

[ssm-devel] [PATCH 1/3] ssmlib: correctly use real devname in xfs_get_info

[Jan T. <jt...@re...>]

The dict from get_mounts() is indexed by real device name
(get_real_device() in get_mountinfo/get_mounts_old), but we were using
arbitrary name in xfs_get_info().  Use the real dev name instead.

RHBZ 1664652

Signed-off-by: Jan Tulak <jt...@re...>
---
 ssmlib/main.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ssmlib/main.py b/ssmlib/main.py
index 6cc3219..80e39e7 100644
--- a/ssmlib/main.py
+++ b/ssmlib/main.py
@@ -349,7 +349,8 @@ class FsInfo(object):
     def xfs_get_info(self, dev):
         # Never use xfs_db for a mounted filesystem - such use is unsupported
         # by XFS and almost guaranteed to report stale data.
-        mount_point = misc.get_mounts(dev).get(dev, {}).get('mp', None)
+        realdev = misc.get_real_device(dev)
+        mount_point = misc.get_mounts(dev).get(realdev, {}).get('mp', None)
         if mount_point:
             stat = os.statvfs(mount_point)
             total = stat.f_blocks*stat.f_bsize
-- 
2.21.0

Re: [ssm-devel] [PATCH 3/3] Method to resize PV.

[Nitin Y. <ny...@re...>]

On Tue, Mar 19, 2019 at 7:04 PM Jan Tulak <jt...@re...> wrote:

> On Tue, Mar 19, 2019 at 12:16 PM Lukas Czerner <lcz...@re...>
> wrote:
>
> [snip]
>
> > > > > +
> > > > > +ssm resize $dev3
> > > > > +check pv_field $dev2 pv_size 96.00m
> > > >
> > > > Does it make sense to you ? You resize the pv to the size of the
> device
> > > > (which is I assume what no size specified means) and it's smaller
> than
> > > > it was before ?? Why ?
> > >
> > > The shrinking actually makes a sense, it's the size of the underlying
> > > device. But what I see is that in the previous step the PV is resized
> > > to be larger than the physical size available. Which is something I
> > > don't think we should allow even with ssm -f and needs to be
> > > addressed.
> >
> > yes, that's what I was referring to. I am not sure what and how it's
> > happening but I am surprised that pvresize would allow the resize past
> > device size without complaining. Especially since the standard way is to
> > enlarge the disk/partition first and then resize the pv
> >
>
> It's not quite quiet.
>
> # ./bin/ssm.local -f resize -s 4G /dev/sdd1  <-- after creating LVM PV
> signature
>   WARNING: /dev/sdd1: Overriding real size 1.00 GiB. You could lose data.
>   WARNING: /dev/sdd1: Pretending size is 8388608 not 2097152 sectors.
>   Physical volume "/dev/sdd1" changed
>   1 physical volume(s) resized / 0 physical volume(s) not resized
>
> I'm not sure how old versions of pvresize behaved, but with the
> current one, we have to either force everything or nothing (and it
> wants a confirmation anytime the target size != device size). But no
> matter what pvresize does or doesn't, SSM should not allow for pv size
> > dev size. The only usecases for such resizes I know about are of the
> black magic type. And SSM is too young and innocent to go anywhere
> near of it. :-D
>
> So, Nitin, please, change the "resize to 4G on a 200MB dev" test (and
> similar) to expect an error instead of a pass.
>

ack Jan.

Thank you.



> Cheers,
> Jan
>
>
> --
> Jan Tulak
>


-- 
Regards,
Nitin Yewale

Re: [ssm-devel] [PATCH 3/3] Method to resize PV.

[Jan T. <jt...@re...>]

On Tue, Mar 19, 2019 at 12:16 PM Lukas Czerner <lcz...@re...> wrote:

[snip]

> > > > +
> > > > +ssm resize $dev3
> > > > +check pv_field $dev2 pv_size 96.00m
> > >
> > > Does it make sense to you ? You resize the pv to the size of the device
> > > (which is I assume what no size specified means) and it's smaller than
> > > it was before ?? Why ?
> >
> > The shrinking actually makes a sense, it's the size of the underlying
> > device. But what I see is that in the previous step the PV is resized
> > to be larger than the physical size available. Which is something I
> > don't think we should allow even with ssm -f and needs to be
> > addressed.
>
> yes, that's what I was referring to. I am not sure what and how it's
> happening but I am surprised that pvresize would allow the resize past
> device size without complaining. Especially since the standard way is to
> enlarge the disk/partition first and then resize the pv
>

It's not quite quiet.

# ./bin/ssm.local -f resize -s 4G /dev/sdd1  <-- after creating LVM PV signature
  WARNING: /dev/sdd1: Overriding real size 1.00 GiB. You could lose data.
  WARNING: /dev/sdd1: Pretending size is 8388608 not 2097152 sectors.
  Physical volume "/dev/sdd1" changed
  1 physical volume(s) resized / 0 physical volume(s) not resized

I'm not sure how old versions of pvresize behaved, but with the
current one, we have to either force everything or nothing (and it
wants a confirmation anytime the target size != device size). But no
matter what pvresize does or doesn't, SSM should not allow for pv size
> dev size. The only usecases for such resizes I know about are of the
black magic type. And SSM is too young and innocent to go anywhere
near of it. :-D

So, Nitin, please, change the "resize to 4G on a 200MB dev" test (and
similar) to expect an error instead of a pass.

Cheers,
Jan


--
Jan Tulak

Re: [ssm-devel] [PATCH 3/3] Method to resize PV.

[Lukas C. <lcz...@re...>]

On Mon, Mar 18, 2019 at 02:34:14PM +0100, Jan Tulak wrote:
> Hi guys
> 
> Welcome back Nitin, I hope you had a good time on holiday. :-)
> 
> 
> On Thu, Mar 7, 2019 at 2:24 PM Lukas Czerner <lcz...@re...> wrote:
> >
> > On Tue, Mar 05, 2019 at 06:36:03PM +0530, Nitin U. Yewale wrote:
> > > ---
> > > From f02d6d142ae9ba2d24bf15ed56bdef59b3a42f1a Mon Sep 17 00:00:00 2001
> > > From: "Nitin U. Yewale" <ny...@re...>
> > > Date: Thu, 21 Feb 2019 19:23:38 +0530
> > >
> > >       Method to resize PV.
> > >
> > > Signed-off-by: Jan Tulak <jt...@re...>
> > > Signed-off-by: Nitin U. Yewale <ny...@re...>
> > >
> > >  ssmlib/main.py                    | 36 ++++++++++++++++++++++++-------
> > >  tests/bashtests/004-lvm-resize.sh | 10 ++++++++-
> > >  2 files changed, 37 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/ssmlib/main.py b/ssmlib/main.py
> > > index 2c44caf..8913360 100644
> > > --- a/ssmlib/main.py
> > > +++ b/ssmlib/main.py
> > > @@ -1744,14 +1744,17 @@ class StorageHandle(object):
> > >
> > >          return have_size, devices
> > >
> > > -    def resize(self, args):
> > > -        """
> > > -        Resize the volume to the given size. If more devices are provided as
> > > -        arguments, it will be added into the pool prior to the volume resize
> > > -        only if the space in the pool is not sufficient. That said, only the
> > > -        number of devices are added into the pool to be able to cover the
> > > -        resize.
> > > -        """
> > > +    def resize_pv(self, args):
> >
> > You're working in a generic part of the ssm so it would be better to
> > avoid backend specific names such as pv and lv.
> >
> > resize_dev
> > resize_vol
> >
> > may be better ?
> 
> Yeah, this sounds reasonable - just a rename.
> 
> >
> > > +        args.dev = args.volume.name
> > > +        if args.size is None:
> > > +            new_size = misc.get_file_size(args.dev)
> > > +        else:
> > > +            new_size = calculate_pvresize_size(args.size, args.dev)
> > > +            if new_size <= 0:
> > > +                PR.error("New volume size \'{0} KB\' is too small".format(new_size))
> >
> > Why not to do all of this in calculate_pvresize_size ? Also why to use
> > misc.get_file_size() since we already have the physical device size
> > information ?
> >
> > > +        args.volume.resize(new_size)
> > > +
> > > +    def resize_lv(self, args):
> > >          args.pool = self.pool[args.volume['pool_name']]
> > >          vol_size = float(args.volume['vol_size'])
> > >
> > > @@ -1813,6 +1816,20 @@ class StorageHandle(object):
> > >          if new_size != vol_size:
> > >              args.volume.resize(new_size, fs)
> > >
> > > +    def resize(self, args):
> > > +        """
> > > +        Resize the PV or volume to the given size. If more devices are provided as
> >
> > Again do not use backend specific terminology here. There coul dbe other
> > backends supporting device resize.
> >
> > > +        arguments, it will be added into the pool prior to the volume resize
> > > +        only if the space in the pool is not sufficient. That said, only the
> > > +        number of devices are added into the pool to be able to cover the
> > > +        resize.
> > > +        """
> > > +        device_type = args.volume.type
> > > +        if device_type == 'device':
> > > +            self.resize_pv(args)
> > > +        else:
> > > +            self.resize_lv(args)
> > > +
> > >      def get_check_passphrase(self):
> > >          """
> > >          Ask for a passphrase and check its quality.
> > > @@ -2334,6 +2351,9 @@ class StorageHandle(object):
> > >          vol = self.vol[string]
> > >          if vol:
> > >              return vol
> > > +        device = self.dev[string]
> > > +        if device:
> > > +            return device
> > >          err = "'{0}' is not a valid volume to resize".format(string)
> > >          raise argparse.ArgumentTypeError(err)
> > >
> > > diff --git a/tests/bashtests/004-lvm-resize.sh b/tests/bashtests/004-lvm-resize.sh
> > > index ed6c4cf..9282837 100755
> > > --- a/tests/bashtests/004-lvm-resize.sh
> > > +++ b/tests/bashtests/004-lvm-resize.sh
> > > @@ -365,12 +365,20 @@ check lv_field $SSM_LVM_DEFAULT_POOL/$tpool1 lv_size ${size}.00m
> > >  not ssm resize --size 500% $SSM_LVM_DEFAULT_POOL/$tpool1
> > >  ssm  -f remove --all
> > >
> > > +#Resize PV
> > > +ssm create $TEST_DEVS
> > > +ssm -f resize -s 2048M  $dev3
> > > +check pv_field $dev3 pv_size "<2.00g"
> >
> > Hmmm, this is new to me - I've never seen '<' I am not sure what that
> > means and I do not think we're using this in the tests.
> >
> > Jan is it potential problem for us ? Maybe something that changed
> > recently ?
> 
> From man pvs
>        --units r|R|h|H|b|B|s|S|k|K|m|M|g|G|t|T|p|P|e|E
>               All sizes are output in these units: human-(r)eadable with '<'
>               rounding indicator, (h)uman-readable, (b)ytes, (s)ectors, [snip]
> 
> We do not use it anywhere else in tests. And I didn't see it cause any
> issues within ssm tests (or at least none that I would remember). For
> safety, I think we can add '--units h' to check.sh, which AFAIK omits
> the '<' sign and a test check seems results in [PASSED] for
> everything. SSM itself uses '--units k' all the time, so it is only
> about what values we want to use for 'check pv_field' and similar.

Right, it makes sense to work with specified units in the entire test
suite.

> 
> >
> > > +
> > > +ssm resize $dev3
> > > +check pv_field $dev2 pv_size 96.00m
> >
> > Does it make sense to you ? You resize the pv to the size of the device
> > (which is I assume what no size specified means) and it's smaller than
> > it was before ?? Why ?
> 
> The shrinking actually makes a sense, it's the size of the underlying
> device. But what I see is that in the previous step the PV is resized
> to be larger than the physical size available. Which is something I
> don't think we should allow even with ssm -f and needs to be
> addressed.

yes, that's what I was referring to. I am not sure what and how it's
happening but I am surprised that pvresize would allow the resize past
device size without complaining. Especially since the standard way is to
enlarge the disk/partition first and then resize the pv

> 
> >
> > I'd also like to see some more comperhensive testing
> >
> >  - extending/shrinking
> >  - using specified size/no size/+-size
> >  - change the size of the device then try to resize again
> >  - how about pv that's part of the pool
> >  - how about pv that's being completely used in the lv
> >  - how about pv that's being partially used in the lv
> >  - how about requesting size bigger than the device
> >  - how about requesting size smaller than the used portion of the pv
> >  ...
> > I am sure you can figure out other scenarios. I'd like the test to check
> > as much possible cases as possible even those that do not make sense and
> > should fail.
> >
> > We need additional testing for other backends and plain devices.
> >
> > Also unit tests are missing completely so that's something that needs to
> > be added as well.
> >
> > Moreover I there is no documentation change at all. This definitelly
> > needs to be documented in all of our documentation. Jan can you please
> > explain Nitin how the documentation changes are done ?
> 
> Thanks for remembering the doc. :-) I'll follow it up with Nitin or
> irc or something.
> 
> Cheers,
> Jan

Thanks!
-Lukas

> 
> 
> --
> Jan Tulak

Re: [ssm-devel] [PATCH 4/4] Patch to check whether the device is a valid device for pvresize

[Nitin Y. <ny...@re...>]

Thank you Jan.

Will make the changes accordingly.


On Tue, Mar 19, 2019 at 3:18 PM Jan Tulak <jt...@re...> wrote:

> On Tue, Mar 19, 2019 at 10:34 AM Nitin U. Yewale <ny...@re...>
> wrote:
> >
> > ---
> >
> > Hi,
> >
> > This patch checks whether the device submitted for resize is
> > a valid PV or not.
> >
>
> Hi and thanks for the patch. :-) The usual approach is to make a new
> version for the patches you already have, rather than send additional
> patches. Especially as there were other issues too.
>
> > Tested for following scenarios
> >
> >
> > [root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1234  <-- for
> device does not exist
> > usage: ssm resize [-h] [-s SIZE] volume [device [device ...]]
> > ssm resize: error: argument volume: '/dev/sdd1234' is not a valid
> volume/device to resize
> >
> > [root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1  <-- device
> exist but not part of LVM. No PV signature on it.
> > SSM Error (2001): Device /dev/sdd1 is not a LVM2 member!
> >
> > [root@stest ssm]# ./bin/ssm.local -f resize -s +500M /dev/vg3/lv1 <--
> as usual working for LVM volumes
> >   Size of logical volume vg3/lv1 changed from 2.00 GiB (512 extents) to
> <2.49 GiB (637 extents).
> >   Logical volume vg3/lv1 successfully resized.
> >
> > [root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1  <-- after
> creating LVM PV signature
> >   WARNING: /dev/sdd1: Overriding real size 1.00 GiB. You could lose data.
> >   WARNING: /dev/sdd1: Pretending size is 8388608 not 2097152 sectors.
> >   Physical volume "/dev/sdd1" changed
> >   1 physical volume(s) resized / 0 physical volume(s) not resized
> >
> > [root@stest ssm]# ./bin/ssm.local -f resize /dev/sdd1  <--- resizing
> back to the device sze
> >   Physical volume "/dev/sdd1" changed
> >   1 physical volume(s) resized / 0 physical volume(s) not resized
> >
>
> Good beginning. Now, can you put these tests into the test suite? :-)
>
> > Thank you,
> > Nitin Yewale
> >
> >
> > ---
> > Signed-off-by: Jan Tulak <jt...@re...>
> > Signed-off-by: Nitin U. Yewale <ny...@re...>
> >
> >  ssmlib/main.py | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/ssmlib/main.py b/ssmlib/main.py
> > index 8913360..4a98ac0 100644
> > --- a/ssmlib/main.py
> > +++ b/ssmlib/main.py
> > @@ -1826,7 +1826,10 @@ class StorageHandle(object):
> >          """
> >          device_type = args.volume.type
> >          if device_type == 'device':
> > -            self.resize_pv(args)
> > +            if (args.volume['physical_size'] != ''):
> > +                self.resize_pv(args)
> > +            else:
> > +                PR.error("Device {0} is not a LVM2
> member".format(args.volume['dev_name']))
> >          else:
> >              self.resize_lv(args)
>
> Why is the decision based on wha looks like an arbitrary field? If it
> really makes sense, a comment in the code would be nice, but I suspect
> that there may be a better way. And we are still in main.py, so again,
> "not a LVM2 member" is not a good message. Use something generic, that
> is not specific to LVM. E.g. "device X is not in a resizeable
> pool/cannot be resized/..."
>
> Thanks,
> Jan
>
> >
> > @@ -2354,7 +2357,7 @@ class StorageHandle(object):
> >          device = self.dev[string]
> >          if device:
> >              return device
> > -        err = "'{0}' is not a valid volume to resize".format(string)
> > +        err = "'{0}' is not a valid volume/device to
> resize".format(string)
> >          raise argparse.ArgumentTypeError(err)
> >
> >      def can_snapshot(self, string):
> > --
> > 2.17.2
> >
>
>
> --
> Jan Tulak
>


-- 
Regards,
Nitin Yewale

Re: [ssm-devel] [PATCH 4/4] Patch to check whether the device is a valid device for pvresize

[Jan T. <jt...@re...>]

On Tue, Mar 19, 2019 at 10:34 AM Nitin U. Yewale <ny...@re...> wrote:
>
> ---
>
> Hi,
>
> This patch checks whether the device submitted for resize is
> a valid PV or not.
>

Hi and thanks for the patch. :-) The usual approach is to make a new
version for the patches you already have, rather than send additional
patches. Especially as there were other issues too.

> Tested for following scenarios
>
>
> [root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1234  <-- for device does not exist
> usage: ssm resize [-h] [-s SIZE] volume [device [device ...]]
> ssm resize: error: argument volume: '/dev/sdd1234' is not a valid volume/device to resize
>
> [root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1  <-- device exist but not part of LVM. No PV signature on it.
> SSM Error (2001): Device /dev/sdd1 is not a LVM2 member!
>
> [root@stest ssm]# ./bin/ssm.local -f resize -s +500M /dev/vg3/lv1 <-- as usual working for LVM volumes
>   Size of logical volume vg3/lv1 changed from 2.00 GiB (512 extents) to <2.49 GiB (637 extents).
>   Logical volume vg3/lv1 successfully resized.
>
> [root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1  <-- after creating LVM PV signature
>   WARNING: /dev/sdd1: Overriding real size 1.00 GiB. You could lose data.
>   WARNING: /dev/sdd1: Pretending size is 8388608 not 2097152 sectors.
>   Physical volume "/dev/sdd1" changed
>   1 physical volume(s) resized / 0 physical volume(s) not resized
>
> [root@stest ssm]# ./bin/ssm.local -f resize /dev/sdd1  <--- resizing back to the device sze
>   Physical volume "/dev/sdd1" changed
>   1 physical volume(s) resized / 0 physical volume(s) not resized
>

Good beginning. Now, can you put these tests into the test suite? :-)

> Thank you,
> Nitin Yewale
>
>
> ---
> Signed-off-by: Jan Tulak <jt...@re...>
> Signed-off-by: Nitin U. Yewale <ny...@re...>
>
>  ssmlib/main.py | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/ssmlib/main.py b/ssmlib/main.py
> index 8913360..4a98ac0 100644
> --- a/ssmlib/main.py
> +++ b/ssmlib/main.py
> @@ -1826,7 +1826,10 @@ class StorageHandle(object):
>          """
>          device_type = args.volume.type
>          if device_type == 'device':
> -            self.resize_pv(args)
> +            if (args.volume['physical_size'] != ''):
> +                self.resize_pv(args)
> +            else:
> +                PR.error("Device {0} is not a LVM2 member".format(args.volume['dev_name']))
>          else:
>              self.resize_lv(args)

Why is the decision based on wha looks like an arbitrary field? If it
really makes sense, a comment in the code would be nice, but I suspect
that there may be a better way. And we are still in main.py, so again,
"not a LVM2 member" is not a good message. Use something generic, that
is not specific to LVM. E.g. "device X is not in a resizeable
pool/cannot be resized/..."

Thanks,
Jan

>
> @@ -2354,7 +2357,7 @@ class StorageHandle(object):
>          device = self.dev[string]
>          if device:
>              return device
> -        err = "'{0}' is not a valid volume to resize".format(string)
> +        err = "'{0}' is not a valid volume/device to resize".format(string)
>          raise argparse.ArgumentTypeError(err)
>
>      def can_snapshot(self, string):
> --
> 2.17.2
>


-- 
Jan Tulak

[ssm-devel] [PATCH 4/4] Patch to check whether the device is a valid device for pvresize

[Nitin U. Y. <ny...@re...>]

---

Hi,

This patch checks whether the device submitted for resize is
a valid PV or not.

Tested for following scenarios


[root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1234  <-- for device does not exist
usage: ssm resize [-h] [-s SIZE] volume [device [device ...]]
ssm resize: error: argument volume: '/dev/sdd1234' is not a valid volume/device to resize

[root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1  <-- device exist but not part of LVM. No PV signature on it.
SSM Error (2001): Device /dev/sdd1 is not a LVM2 member!

[root@stest ssm]# ./bin/ssm.local -f resize -s +500M /dev/vg3/lv1 <-- as usual working for LVM volumes
  Size of logical volume vg3/lv1 changed from 2.00 GiB (512 extents) to <2.49 GiB (637 extents).
  Logical volume vg3/lv1 successfully resized.

[root@stest ssm]# ./bin/ssm.local -f resize -s 4G /dev/sdd1  <-- after creating LVM PV signature
  WARNING: /dev/sdd1: Overriding real size 1.00 GiB. You could lose data.
  WARNING: /dev/sdd1: Pretending size is 8388608 not 2097152 sectors.
  Physical volume "/dev/sdd1" changed
  1 physical volume(s) resized / 0 physical volume(s) not resized

[root@stest ssm]# ./bin/ssm.local -f resize /dev/sdd1  <--- resizing back to the device sze
  Physical volume "/dev/sdd1" changed
  1 physical volume(s) resized / 0 physical volume(s) not resized

Thank you,
Nitin Yewale


---
Signed-off-by: Jan Tulak <jt...@re...>
Signed-off-by: Nitin U. Yewale <ny...@re...>

 ssmlib/main.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ssmlib/main.py b/ssmlib/main.py
index 8913360..4a98ac0 100644
--- a/ssmlib/main.py
+++ b/ssmlib/main.py
@@ -1826,7 +1826,10 @@ class StorageHandle(object):
         """
         device_type = args.volume.type
         if device_type == 'device':
-            self.resize_pv(args)
+            if (args.volume['physical_size'] != ''):
+                self.resize_pv(args)
+            else:
+                PR.error("Device {0} is not a LVM2 member".format(args.volume['dev_name']))
         else:
             self.resize_lv(args)
 
@@ -2354,7 +2357,7 @@ class StorageHandle(object):
         device = self.dev[string]
         if device:
             return device
-        err = "'{0}' is not a valid volume to resize".format(string)
+        err = "'{0}' is not a valid volume/device to resize".format(string)
         raise argparse.ArgumentTypeError(err)
 
     def can_snapshot(self, string):
-- 
2.17.2

Re: [ssm-devel] [PATCH 3/3] Method to resize PV.

[Nitin Y. <ny...@re...>]

Hello Jan and Lukas,

Thank you for your guidance so far.

I have started working on the changes. I shall update here shortly.


On Mon, Mar 18, 2019 at 7:04 PM Jan Tulak <jt...@re...> wrote:

> Hi guys
>
> Welcome back Nitin, I hope you had a good time on holiday. :-)
>
>
> On Thu, Mar 7, 2019 at 2:24 PM Lukas Czerner <lcz...@re...> wrote:
> >
> > On Tue, Mar 05, 2019 at 06:36:03PM +0530, Nitin U. Yewale wrote:
> > > ---
> > > From f02d6d142ae9ba2d24bf15ed56bdef59b3a42f1a Mon Sep 17 00:00:00 2001
> > > From: "Nitin U. Yewale" <ny...@re...>
> > > Date: Thu, 21 Feb 2019 19:23:38 +0530
> > >
> > >       Method to resize PV.
> > >
> > > Signed-off-by: Jan Tulak <jt...@re...>
> > > Signed-off-by: Nitin U. Yewale <ny...@re...>
> > >
> > >  ssmlib/main.py                    | 36 ++++++++++++++++++++++++-------
> > >  tests/bashtests/004-lvm-resize.sh | 10 ++++++++-
> > >  2 files changed, 37 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/ssmlib/main.py b/ssmlib/main.py
> > > index 2c44caf..8913360 100644
> > > --- a/ssmlib/main.py
> > > +++ b/ssmlib/main.py
> > > @@ -1744,14 +1744,17 @@ class StorageHandle(object):
> > >
> > >          return have_size, devices
> > >
> > > -    def resize(self, args):
> > > -        """
> > > -        Resize the volume to the given size. If more devices are
> provided as
> > > -        arguments, it will be added into the pool prior to the volume
> resize
> > > -        only if the space in the pool is not sufficient. That said,
> only the
> > > -        number of devices are added into the pool to be able to cover
> the
> > > -        resize.
> > > -        """
> > > +    def resize_pv(self, args):
> >
> > You're working in a generic part of the ssm so it would be better to
> > avoid backend specific names such as pv and lv.
> >
> > resize_dev
> > resize_vol
> >
> > may be better ?
>
> Yeah, this sounds reasonable - just a rename.
>
> >
> > > +        args.dev = args.volume.name
> > > +        if args.size is None:
> > > +            new_size = misc.get_file_size(args.dev)
> > > +        else:
> > > +            new_size = calculate_pvresize_size(args.size, args.dev)
> > > +            if new_size <= 0:
> > > +                PR.error("New volume size \'{0} KB\' is too
> small".format(new_size))
> >
> > Why not to do all of this in calculate_pvresize_size ? Also why to use
> > misc.get_file_size() since we already have the physical device size
> > information ?
> >
> > > +        args.volume.resize(new_size)
> > > +
> > > +    def resize_lv(self, args):
> > >          args.pool = self.pool[args.volume['pool_name']]
> > >          vol_size = float(args.volume['vol_size'])
> > >
> > > @@ -1813,6 +1816,20 @@ class StorageHandle(object):
> > >          if new_size != vol_size:
> > >              args.volume.resize(new_size, fs)
> > >
> > > +    def resize(self, args):
> > > +        """
> > > +        Resize the PV or volume to the given size. If more devices
> are provided as
> >
> > Again do not use backend specific terminology here. There coul dbe other
> > backends supporting device resize.
> >
> > > +        arguments, it will be added into the pool prior to the volume
> resize
> > > +        only if the space in the pool is not sufficient. That said,
> only the
> > > +        number of devices are added into the pool to be able to cover
> the
> > > +        resize.
> > > +        """
> > > +        device_type = args.volume.type
> > > +        if device_type == 'device':
> > > +            self.resize_pv(args)
> > > +        else:
> > > +            self.resize_lv(args)
> > > +
> > >      def get_check_passphrase(self):
> > >          """
> > >          Ask for a passphrase and check its quality.
> > > @@ -2334,6 +2351,9 @@ class StorageHandle(object):
> > >          vol = self.vol[string]
> > >          if vol:
> > >              return vol
> > > +        device = self.dev[string]
> > > +        if device:
> > > +            return device
> > >          err = "'{0}' is not a valid volume to resize".format(string)
> > >          raise argparse.ArgumentTypeError(err)
> > >
> > > diff --git a/tests/bashtests/004-lvm-resize.sh
> b/tests/bashtests/004-lvm-resize.sh
> > > index ed6c4cf..9282837 100755
> > > --- a/tests/bashtests/004-lvm-resize.sh
> > > +++ b/tests/bashtests/004-lvm-resize.sh
> > > @@ -365,12 +365,20 @@ check lv_field $SSM_LVM_DEFAULT_POOL/$tpool1
> lv_size ${size}.00m
> > >  not ssm resize --size 500% $SSM_LVM_DEFAULT_POOL/$tpool1
> > >  ssm  -f remove --all
> > >
> > > +#Resize PV
> > > +ssm create $TEST_DEVS
> > > +ssm -f resize -s 2048M  $dev3
> > > +check pv_field $dev3 pv_size "<2.00g"
> >
> > Hmmm, this is new to me - I've never seen '<' I am not sure what that
> > means and I do not think we're using this in the tests.
> >
> > Jan is it potential problem for us ? Maybe something that changed
> > recently ?
>
> From man pvs
>        --units r|R|h|H|b|B|s|S|k|K|m|M|g|G|t|T|p|P|e|E
>               All sizes are output in these units: human-(r)eadable with
> '<'
>               rounding indicator, (h)uman-readable, (b)ytes, (s)ectors,
> [snip]
>
> We do not use it anywhere else in tests. And I didn't see it cause any
> issues within ssm tests (or at least none that I would remember). For
> safety, I think we can add '--units h' to check.sh, which AFAIK omits
> the '<' sign and a test check seems results in [PASSED] for
> everything. SSM itself uses '--units k' all the time, so it is only
> about what values we want to use for 'check pv_field' and similar.
>
> >
> > > +
> > > +ssm resize $dev3
> > > +check pv_field $dev2 pv_size 96.00m
> >
> > Does it make sense to you ? You resize the pv to the size of the device
> > (which is I assume what no size specified means) and it's smaller than
> > it was before ?? Why ?
>
> The shrinking actually makes a sense, it's the size of the underlying
> device. But what I see is that in the previous step the PV is resized
> to be larger than the physical size available. Which is something I
> don't think we should allow even with ssm -f and needs to be
> addressed.
>
> >
> > I'd also like to see some more comperhensive testing
> >
> >  - extending/shrinking
> >  - using specified size/no size/+-size
> >  - change the size of the device then try to resize again
> >  - how about pv that's part of the pool
> >  - how about pv that's being completely used in the lv
> >  - how about pv that's being partially used in the lv
> >  - how about requesting size bigger than the device
> >  - how about requesting size smaller than the used portion of the pv
> >  ...
> > I am sure you can figure out other scenarios. I'd like the test to check
> > as much possible cases as possible even those that do not make sense and
> > should fail.
> >
> > We need additional testing for other backends and plain devices.
> >
> > Also unit tests are missing completely so that's something that needs to
> > be added as well.
> >
> > Moreover I there is no documentation change at all. This definitelly
> > needs to be documented in all of our documentation. Jan can you please
> > explain Nitin how the documentation changes are done ?
>
> Thanks for remembering the doc. :-) I'll follow it up with Nitin or
> irc or something.
>
> Cheers,
> Jan
>
>
> --
> Jan Tulak
>


-- 
Regards,
Nitin Yewale

Re: [ssm-devel] [PATCH 3/3] Method to resize PV.

[Jan T. <jt...@re...>]

Hi guys

Welcome back Nitin, I hope you had a good time on holiday. :-)


On Thu, Mar 7, 2019 at 2:24 PM Lukas Czerner <lcz...@re...> wrote:
>
> On Tue, Mar 05, 2019 at 06:36:03PM +0530, Nitin U. Yewale wrote:
> > ---
> > From f02d6d142ae9ba2d24bf15ed56bdef59b3a42f1a Mon Sep 17 00:00:00 2001
> > From: "Nitin U. Yewale" <ny...@re...>
> > Date: Thu, 21 Feb 2019 19:23:38 +0530
> >
> >       Method to resize PV.
> >
> > Signed-off-by: Jan Tulak <jt...@re...>
> > Signed-off-by: Nitin U. Yewale <ny...@re...>
> >
> >  ssmlib/main.py                    | 36 ++++++++++++++++++++++++-------
> >  tests/bashtests/004-lvm-resize.sh | 10 ++++++++-
> >  2 files changed, 37 insertions(+), 9 deletions(-)
> >
> > diff --git a/ssmlib/main.py b/ssmlib/main.py
> > index 2c44caf..8913360 100644
> > --- a/ssmlib/main.py
> > +++ b/ssmlib/main.py
> > @@ -1744,14 +1744,17 @@ class StorageHandle(object):
> >
> >          return have_size, devices
> >
> > -    def resize(self, args):
> > -        """
> > -        Resize the volume to the given size. If more devices are provided as
> > -        arguments, it will be added into the pool prior to the volume resize
> > -        only if the space in the pool is not sufficient. That said, only the
> > -        number of devices are added into the pool to be able to cover the
> > -        resize.
> > -        """
> > +    def resize_pv(self, args):
>
> You're working in a generic part of the ssm so it would be better to
> avoid backend specific names such as pv and lv.
>
> resize_dev
> resize_vol
>
> may be better ?

Yeah, this sounds reasonable - just a rename.

>
> > +        args.dev = args.volume.name
> > +        if args.size is None:
> > +            new_size = misc.get_file_size(args.dev)
> > +        else:
> > +            new_size = calculate_pvresize_size(args.size, args.dev)
> > +            if new_size <= 0:
> > +                PR.error("New volume size \'{0} KB\' is too small".format(new_size))
>
> Why not to do all of this in calculate_pvresize_size ? Also why to use
> misc.get_file_size() since we already have the physical device size
> information ?
>
> > +        args.volume.resize(new_size)
> > +
> > +    def resize_lv(self, args):
> >          args.pool = self.pool[args.volume['pool_name']]
> >          vol_size = float(args.volume['vol_size'])
> >
> > @@ -1813,6 +1816,20 @@ class StorageHandle(object):
> >          if new_size != vol_size:
> >              args.volume.resize(new_size, fs)
> >
> > +    def resize(self, args):
> > +        """
> > +        Resize the PV or volume to the given size. If more devices are provided as
>
> Again do not use backend specific terminology here. There coul dbe other
> backends supporting device resize.
>
> > +        arguments, it will be added into the pool prior to the volume resize
> > +        only if the space in the pool is not sufficient. That said, only the
> > +        number of devices are added into the pool to be able to cover the
> > +        resize.
> > +        """
> > +        device_type = args.volume.type
> > +        if device_type == 'device':
> > +            self.resize_pv(args)
> > +        else:
> > +            self.resize_lv(args)
> > +
> >      def get_check_passphrase(self):
> >          """
> >          Ask for a passphrase and check its quality.
> > @@ -2334,6 +2351,9 @@ class StorageHandle(object):
> >          vol = self.vol[string]
> >          if vol:
> >              return vol
> > +        device = self.dev[string]
> > +        if device:
> > +            return device
> >          err = "'{0}' is not a valid volume to resize".format(string)
> >          raise argparse.ArgumentTypeError(err)
> >
> > diff --git a/tests/bashtests/004-lvm-resize.sh b/tests/bashtests/004-lvm-resize.sh
> > index ed6c4cf..9282837 100755
> > --- a/tests/bashtests/004-lvm-resize.sh
> > +++ b/tests/bashtests/004-lvm-resize.sh
> > @@ -365,12 +365,20 @@ check lv_field $SSM_LVM_DEFAULT_POOL/$tpool1 lv_size ${size}.00m
> >  not ssm resize --size 500% $SSM_LVM_DEFAULT_POOL/$tpool1
> >  ssm  -f remove --all
> >
> > +#Resize PV
> > +ssm create $TEST_DEVS
> > +ssm -f resize -s 2048M  $dev3
> > +check pv_field $dev3 pv_size "<2.00g"
>
> Hmmm, this is new to me - I've never seen '<' I am not sure what that
> means and I do not think we're using this in the tests.
>
> Jan is it potential problem for us ? Maybe something that changed
> recently ?

>From man pvs
       --units r|R|h|H|b|B|s|S|k|K|m|M|g|G|t|T|p|P|e|E
              All sizes are output in these units: human-(r)eadable with '<'
              rounding indicator, (h)uman-readable, (b)ytes, (s)ectors, [snip]

We do not use it anywhere else in tests. And I didn't see it cause any
issues within ssm tests (or at least none that I would remember). For
safety, I think we can add '--units h' to check.sh, which AFAIK omits
the '<' sign and a test check seems results in [PASSED] for
everything. SSM itself uses '--units k' all the time, so it is only
about what values we want to use for 'check pv_field' and similar.

>
> > +
> > +ssm resize $dev3
> > +check pv_field $dev2 pv_size 96.00m
>
> Does it make sense to you ? You resize the pv to the size of the device
> (which is I assume what no size specified means) and it's smaller than
> it was before ?? Why ?

The shrinking actually makes a sense, it's the size of the underlying
device. But what I see is that in the previous step the PV is resized
to be larger than the physical size available. Which is something I
don't think we should allow even with ssm -f and needs to be
addressed.

>
> I'd also like to see some more comperhensive testing
>
>  - extending/shrinking
>  - using specified size/no size/+-size
>  - change the size of the device then try to resize again
>  - how about pv that's part of the pool
>  - how about pv that's being completely used in the lv
>  - how about pv that's being partially used in the lv
>  - how about requesting size bigger than the device
>  - how about requesting size smaller than the used portion of the pv
>  ...
> I am sure you can figure out other scenarios. I'd like the test to check
> as much possible cases as possible even those that do not make sense and
> should fail.
>
> We need additional testing for other backends and plain devices.
>
> Also unit tests are missing completely so that's something that needs to
> be added as well.
>
> Moreover I there is no documentation change at all. This definitelly
> needs to be documented in all of our documentation. Jan can you please
> explain Nitin how the documentation changes are done ?

Thanks for remembering the doc. :-) I'll follow it up with Nitin or
irc or something.

Cheers,
Jan


--
Jan Tulak

Re: [ssm-devel] [PATCH 2/3] Infrastructure to resize PV.

[Lukas C. <lcz...@re...>]

On Fri, Mar 08, 2019 at 03:40:26PM +0530, Nitin Yewale wrote:
> Got it Lukas.
> 
> I think we had implemented that, removed just because pvresize does not
> support the same. I will  need to search that patch though. I am on leave
> next week. Please expect some delay.

Ok no problem. Enjoy your time off.

-Lukas

> 
> On Fri, Mar 8, 2019 at 2:53 PM Lukas Czerner <lcz...@re...> wrote:
> 
> > On Fri, Mar 08, 2019 at 01:04:57PM +0530, Nitin Yewale wrote:
> > > Hello Lukas,
> > >
> > > Thank you for the response.
> > >
> > > Please find answers inline below
> > >
> > > On Thu, Mar 7, 2019 at 6:27 PM Lukas Czerner <lcz...@re...>
> > wrote:
> > >
> > > > On Tue, Mar 05, 2019 at 06:36:02PM +0530, Nitin U. Yewale wrote:
> > > > > ---
> > > > > From 3ac26c27f717e0e60d4b02460e5e1110f8b5e8c4 Mon Sep 17 00:00:00
> > 2001
> > > > > From: "Nitin U. Yewale" <ny...@re...>
> > > > > Date: Thu, 21 Feb 2019 18:24:18 +0530
> > > > >
> > > > >     Infrastructure to resize PV.
> > > > >
> > > > > Signed-off-by: Jan Tulak <jt...@re...>
> > > > > Signed-off-by: Nitin U. Yewale <ny...@re...>
> > > > >
> > > > >  ssmlib/backends/lvm.py | 15 ++++++++++++---
> > > > >  ssmlib/main.py         | 12 ++++++++++++
> > > > >  2 files changed, 24 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/ssmlib/backends/lvm.py b/ssmlib/backends/lvm.py
> > > > > index 3bc98c7..7fbea6e 100644
> > > > > --- a/ssmlib/backends/lvm.py
> > > > > +++ b/ssmlib/backends/lvm.py
> > > > > @@ -95,7 +95,12 @@ class LvmInfo(template.Backend):
> > > > >          if not self.binary:
> > > > >              self.problem.check(self.problem.TOOL_MISSING, 'lvm')
> > > > >          if self.options.force and not noforce:
> > > > > -            command.insert(1, "-f")
> > > > > +            #For the '--yes' or '-y' added in upstream lvm2 version
> > > > v2_02_169.
> > > > > +            if command[0] == 'pvresize':
> > > > > +                if LVM_VERSION[2] > 168:
> > > > > +                    command.insert(1, "--yes")
> > > > > +            else:
> > > > > +                command.insert(1, "-f")
> > > >
> > > > If I understand this correctly it means that pvresize before lvm
> > version
> > > > 2.02.169 did not require any confirmation whatsoever, Is that right ?
> > > >
> > >
> > > Yes. It is correct.
> > >
> > > Some operations are interactive after lvm2 version 168 and for the same
> > we
> > > added above code.
> > >
> > >
> > > > >          if self.options.verbose:
> > > > >              command.insert(1, "-v")
> > > > >          command.insert(0, "lvm")
> > > > > @@ -373,9 +378,9 @@ class PvsInfo(LvmInfo, template.BackendDevice):
> > > > >          super(PvsInfo, self).__init__(*args, **kwargs)
> > > > >          command = ["lvm", "pvs", "--separator", "|", "--noheadings",
> > > > >                     "--nosuffix", "--units", "k", "-o",
> > > > > -                   "pv_name,vg_name,pv_free,pv_used,pv_size"]
> > > > > +
> >  "pv_name,vg_name,pv_free,pv_used,pv_size,dev_size"]
> > > > >          self.attrs = ['dev_name', 'pool_name', 'dev_free',
> > > > > -                      'dev_used', 'dev_size']
> > > > > +                      'dev_used', 'dev_size', 'physical_size']
> > > > >
> > > > >          self._parse_data(command)
> > > > >
> > > > > @@ -407,6 +412,10 @@ class PvsInfo(LvmInfo, template.BackendDevice):
> > > > >          command.extend(devices)
> > > > >          self.run_lvm(command)
> > > > >
> > > > > +    def resize(self, device, size):
> > > > > +        command = ['pvresize', '--setphysicalvolumesize', str(size)
> > +
> > > > 'k', device]
> > > > > +        self.run_lvm(command)
> > > >
> > > > Again, just note that only PvsInfo() is using this method. Yes. As
> > this is
> > > > specifically for the PV.
> > > >
> > > > > +
> > > > >
> > > > >  class LvsInfo(LvmInfo, template.BackendVolume):
> > > > >
> > > > > diff --git a/ssmlib/main.py b/ssmlib/main.py
> > > > > index 06185e6..2c44caf 100644
> > > > > --- a/ssmlib/main.py
> > > > > +++ b/ssmlib/main.py
> > > > > @@ -160,6 +160,13 @@ def calculate_size(arg_size, pool):
> > > > >
> > > > >      return base_size *  mult
> > > > >
> > > > > +def calculate_pvresize_size(arg_size, volume):
> > > > > +    if arg_size[1] == 'K':
> > > > > +        base_size = float(arg_size[0])
> > > > > +    if arg_size[0][0] in ['+', '-']:
> > > > > +        raise PR.error("Extending PV is supported. Changing size
> > with
> > > > '+' or '-' is not supported")
> > > >
> > > > Why is it that we only support Extending ? Using -s +10G is also
> > > > extending, isn't it ? I do not understand why you decided not to
> > support
> > > > +,- signs to size. Am I missing something ?
> > > >
> > > > Also does that mean that we do not support shrinking ? Why ?
> > > >
> > >
> > > We didnot implemented supporting `ssm resize -s +/-size device`  is
> > because
> > > pvresize command does not support this.
> > >
> > > In order to extend and to reduce the size, same command is used. In a
> > > sense, pvresize asks for the size that would be final for the PV.
> > >
> > > For example, for 1 GB PV
> > >
> > > pvresize --setphysicalvolumesize 2G /dev/sdb2 --yes  <-- will extend the
> > PV
> > > to 2 GB
> > >
> > > pvresize --setphysicalvolumesize 500M /dev/sdb2  <-- will reduce the size
> > > to 500M
> >
> > Regardless of whether pvresize specifically, or any other tool in any
> > other backend supports it, ssm should be able to handle it since it
> > already does for any other resize operation.
> >
> > Again, ssm is not just a different way to call pvresize it's supposed to
> > provied added value of making things easier more comperhensive and uniform
> > across different backends. It's not always 100% possible to achieve, but
> > this definitelly is.
> >
> > Thanks!
> > -Lukas
> >
> > >
> > >
> > >
> > > > > +    return base_size
> > > > > +
> > > > >  def calculate_resize_size(arg_size, volume, pool):
> > > > >      vol_size = float(volume['vol_size'])
> > > > >
> > > > > @@ -469,6 +476,11 @@ class DeviceInfo(object):
> > > > >                   "to achieve by removing " +
> > > > >                   "{0}".format(device))
> > > > >
> > > > > +    def resize(self, device, size):
> > > > > +        obj = self.data[device]
> > > > > +        backend = obj['backend']
> > > > > +        backend.resize(device, size)
> > > >
> > > > So again, both 'backend' and resize() method are only defined in
> > > > PvsInfo, so what does happen when we call ssm resize <device> on the
> > > > device that's not pv ?
> > > >
> > > > I had tested by giving wrong PV name and it gives a message that the
> > > device is not valid
> > >
> > > # ./bin/ssm.local resize /dev/sdd12345678
> > > usage: ssm resize [-h] [-s SIZE] volume [device [device ...]]
> > > ssm resize: error: argument volume: '/dev/sdd12345678' is not a valid
> > > volume to resize
> > >
> > > Now we see that sdd1 is not a PV
> > >
> > > # pvs
> > >   PV         VG             Fmt  Attr PSize   PFree
> > >   /dev/sda2  rhel_vm252-140 lvm2 a--  <80.13g  4.00m
> > >   /dev/sda3                 lvm2 ---    5.00g  5.00g
> > >   /dev/sdb1  vg3            lvm2 a--   <5.00g <3.00g
> > >   /dev/sdb2                 lvm2 ---   10.00g 10.00g
> > >
> > > If we try to use this device it throws an error
> > >
> > > # ./bin/ssm.local resize /dev/sdd1
> > > Traceback (most recent call last):
> > >   File "/root/ssm_Feb21_2019/bin/ssm", line 59, in <module>
> > >     sys.exit(main.main())
> > >   File "/root/ssm_Feb21_2019/ssmlib/main.py", line 2952, in main
> > >     args.func(args)
> > >   File "/root/ssm_Feb21_2019/ssmlib/main.py", line 1829, in resize
> > >     self.resize_pv(args)
> > >
> > > I will fix this. Seems I didn't tested this or something has broken.
> > >
> > >
> > > Thanks!
> > > > -Lukas
> > > >
> > > > > +
> > > > >      def migrate(self, vg, source_dev, target_dev):
> > > > >          """ Migrate (in this case clone) the device onto another
> > one.
> > > > >              Pure dd is used.
> > > > > --
> > > > > 2.17.2
> > > > >
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > storagemanager-devel mailing list
> > > > > sto...@li...
> > > > > https://lists.sourceforge.net/lists/listinfo/storagemanager-devel
> > > > > System Storage Manager (
> > > > http://sourceforge.net/p/storagemanager/home/Home/)
> > > >
> > >
> > >
> > > --
> > > Thank you,
> > > Nitin Yewale
> >
> 
> 
> -- 
> Thank you,
> Nitin Yewale

Re: [ssm-devel] [PATCH 2/3] Infrastructure to resize PV.

[Nitin Y. <ny...@re...>]

Got it Lukas.

I think we had implemented that, removed just because pvresize does not
support the same. I will  need to search that patch though. I am on leave
next week. Please expect some delay.

On Fri, Mar 8, 2019 at 2:53 PM Lukas Czerner <lcz...@re...> wrote:

> On Fri, Mar 08, 2019 at 01:04:57PM +0530, Nitin Yewale wrote:
> > Hello Lukas,
> >
> > Thank you for the response.
> >
> > Please find answers inline below
> >
> > On Thu, Mar 7, 2019 at 6:27 PM Lukas Czerner <lcz...@re...>
> wrote:
> >
> > > On Tue, Mar 05, 2019 at 06:36:02PM +0530, Nitin U. Yewale wrote:
> > > > ---
> > > > From 3ac26c27f717e0e60d4b02460e5e1110f8b5e8c4 Mon Sep 17 00:00:00
> 2001
> > > > From: "Nitin U. Yewale" <ny...@re...>
> > > > Date: Thu, 21 Feb 2019 18:24:18 +0530
> > > >
> > > >     Infrastructure to resize PV.
> > > >
> > > > Signed-off-by: Jan Tulak <jt...@re...>
> > > > Signed-off-by: Nitin U. Yewale <ny...@re...>
> > > >
> > > >  ssmlib/backends/lvm.py | 15 ++++++++++++---
> > > >  ssmlib/main.py         | 12 ++++++++++++
> > > >  2 files changed, 24 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/ssmlib/backends/lvm.py b/ssmlib/backends/lvm.py
> > > > index 3bc98c7..7fbea6e 100644
> > > > --- a/ssmlib/backends/lvm.py
> > > > +++ b/ssmlib/backends/lvm.py
> > > > @@ -95,7 +95,12 @@ class LvmInfo(template.Backend):
> > > >          if not self.binary:
> > > >              self.problem.check(self.problem.TOOL_MISSING, 'lvm')
> > > >          if self.options.force and not noforce:
> > > > -            command.insert(1, "-f")
> > > > +            #For the '--yes' or '-y' added in upstream lvm2 version
> > > v2_02_169.
> > > > +            if command[0] == 'pvresize':
> > > > +                if LVM_VERSION[2] > 168:
> > > > +                    command.insert(1, "--yes")
> > > > +            else:
> > > > +                command.insert(1, "-f")
> > >
> > > If I understand this correctly it means that pvresize before lvm
> version
> > > 2.02.169 did not require any confirmation whatsoever, Is that right ?
> > >
> >
> > Yes. It is correct.
> >
> > Some operations are interactive after lvm2 version 168 and for the same
> we
> > added above code.
> >
> >
> > > >          if self.options.verbose:
> > > >              command.insert(1, "-v")
> > > >          command.insert(0, "lvm")
> > > > @@ -373,9 +378,9 @@ class PvsInfo(LvmInfo, template.BackendDevice):
> > > >          super(PvsInfo, self).__init__(*args, **kwargs)
> > > >          command = ["lvm", "pvs", "--separator", "|", "--noheadings",
> > > >                     "--nosuffix", "--units", "k", "-o",
> > > > -                   "pv_name,vg_name,pv_free,pv_used,pv_size"]
> > > > +
>  "pv_name,vg_name,pv_free,pv_used,pv_size,dev_size"]
> > > >          self.attrs = ['dev_name', 'pool_name', 'dev_free',
> > > > -                      'dev_used', 'dev_size']
> > > > +                      'dev_used', 'dev_size', 'physical_size']
> > > >
> > > >          self._parse_data(command)
> > > >
> > > > @@ -407,6 +412,10 @@ class PvsInfo(LvmInfo, template.BackendDevice):
> > > >          command.extend(devices)
> > > >          self.run_lvm(command)
> > > >
> > > > +    def resize(self, device, size):
> > > > +        command = ['pvresize', '--setphysicalvolumesize', str(size)
> +
> > > 'k', device]
> > > > +        self.run_lvm(command)
> > >
> > > Again, just note that only PvsInfo() is using this method. Yes. As
> this is
> > > specifically for the PV.
> > >
> > > > +
> > > >
> > > >  class LvsInfo(LvmInfo, template.BackendVolume):
> > > >
> > > > diff --git a/ssmlib/main.py b/ssmlib/main.py
> > > > index 06185e6..2c44caf 100644
> > > > --- a/ssmlib/main.py
> > > > +++ b/ssmlib/main.py
> > > > @@ -160,6 +160,13 @@ def calculate_size(arg_size, pool):
> > > >
> > > >      return base_size *  mult
> > > >
> > > > +def calculate_pvresize_size(arg_size, volume):
> > > > +    if arg_size[1] == 'K':
> > > > +        base_size = float(arg_size[0])
> > > > +    if arg_size[0][0] in ['+', '-']:
> > > > +        raise PR.error("Extending PV is supported. Changing size
> with
> > > '+' or '-' is not supported")
> > >
> > > Why is it that we only support Extending ? Using -s +10G is also
> > > extending, isn't it ? I do not understand why you decided not to
> support
> > > +,- signs to size. Am I missing something ?
> > >
> > > Also does that mean that we do not support shrinking ? Why ?
> > >
> >
> > We didnot implemented supporting `ssm resize -s +/-size device`  is
> because
> > pvresize command does not support this.
> >
> > In order to extend and to reduce the size, same command is used. In a
> > sense, pvresize asks for the size that would be final for the PV.
> >
> > For example, for 1 GB PV
> >
> > pvresize --setphysicalvolumesize 2G /dev/sdb2 --yes  <-- will extend the
> PV
> > to 2 GB
> >
> > pvresize --setphysicalvolumesize 500M /dev/sdb2  <-- will reduce the size
> > to 500M
>
> Regardless of whether pvresize specifically, or any other tool in any
> other backend supports it, ssm should be able to handle it since it
> already does for any other resize operation.
>
> Again, ssm is not just a different way to call pvresize it's supposed to
> provied added value of making things easier more comperhensive and uniform
> across different backends. It's not always 100% possible to achieve, but
> this definitelly is.
>
> Thanks!
> -Lukas
>
> >
> >
> >
> > > > +    return base_size
> > > > +
> > > >  def calculate_resize_size(arg_size, volume, pool):
> > > >      vol_size = float(volume['vol_size'])
> > > >
> > > > @@ -469,6 +476,11 @@ class DeviceInfo(object):
> > > >                   "to achieve by removing " +
> > > >                   "{0}".format(device))
> > > >
> > > > +    def resize(self, device, size):
> > > > +        obj = self.data[device]
> > > > +        backend = obj['backend']
> > > > +        backend.resize(device, size)
> > >
> > > So again, both 'backend' and resize() method are only defined in
> > > PvsInfo, so what does happen when we call ssm resize <device> on the
> > > device that's not pv ?
> > >
> > > I had tested by giving wrong PV name and it gives a message that the
> > device is not valid
> >
> > # ./bin/ssm.local resize /dev/sdd12345678
> > usage: ssm resize [-h] [-s SIZE] volume [device [device ...]]
> > ssm resize: error: argument volume: '/dev/sdd12345678' is not a valid
> > volume to resize
> >
> > Now we see that sdd1 is not a PV
> >
> > # pvs
> >   PV         VG             Fmt  Attr PSize   PFree
> >   /dev/sda2  rhel_vm252-140 lvm2 a--  <80.13g  4.00m
> >   /dev/sda3                 lvm2 ---    5.00g  5.00g
> >   /dev/sdb1  vg3            lvm2 a--   <5.00g <3.00g
> >   /dev/sdb2                 lvm2 ---   10.00g 10.00g
> >
> > If we try to use this device it throws an error
> >
> > # ./bin/ssm.local resize /dev/sdd1
> > Traceback (most recent call last):
> >   File "/root/ssm_Feb21_2019/bin/ssm", line 59, in <module>
> >     sys.exit(main.main())
> >   File "/root/ssm_Feb21_2019/ssmlib/main.py", line 2952, in main
> >     args.func(args)
> >   File "/root/ssm_Feb21_2019/ssmlib/main.py", line 1829, in resize
> >     self.resize_pv(args)
> >
> > I will fix this. Seems I didn't tested this or something has broken.
> >
> >
> > Thanks!
> > > -Lukas
> > >
> > > > +
> > > >      def migrate(self, vg, source_dev, target_dev):
> > > >          """ Migrate (in this case clone) the device onto another
> one.
> > > >              Pure dd is used.
> > > > --
> > > > 2.17.2
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > storagemanager-devel mailing list
> > > > sto...@li...
> > > > https://lists.sourceforge.net/lists/listinfo/storagemanager-devel
> > > > System Storage Manager (
> > > http://sourceforge.net/p/storagemanager/home/Home/)
> > >
> >
> >
> > --
> > Thank you,
> > Nitin Yewale
>


-- 
Thank you,
Nitin Yewale

Re: [ssm-devel] [PATCH 2/3] Infrastructure to resize PV.

[Lukas C. <lcz...@re...>]

On Fri, Mar 08, 2019 at 01:04:57PM +0530, Nitin Yewale wrote:
> Hello Lukas,
> 
> Thank you for the response.
> 
> Please find answers inline below
> 
> On Thu, Mar 7, 2019 at 6:27 PM Lukas Czerner <lcz...@re...> wrote:
> 
> > On Tue, Mar 05, 2019 at 06:36:02PM +0530, Nitin U. Yewale wrote:
> > > ---
> > > From 3ac26c27f717e0e60d4b02460e5e1110f8b5e8c4 Mon Sep 17 00:00:00 2001
> > > From: "Nitin U. Yewale" <ny...@re...>
> > > Date: Thu, 21 Feb 2019 18:24:18 +0530
> > >
> > >     Infrastructure to resize PV.
> > >
> > > Signed-off-by: Jan Tulak <jt...@re...>
> > > Signed-off-by: Nitin U. Yewale <ny...@re...>
> > >
> > >  ssmlib/backends/lvm.py | 15 ++++++++++++---
> > >  ssmlib/main.py         | 12 ++++++++++++
> > >  2 files changed, 24 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/ssmlib/backends/lvm.py b/ssmlib/backends/lvm.py
> > > index 3bc98c7..7fbea6e 100644
> > > --- a/ssmlib/backends/lvm.py
> > > +++ b/ssmlib/backends/lvm.py
> > > @@ -95,7 +95,12 @@ class LvmInfo(template.Backend):
> > >          if not self.binary:
> > >              self.problem.check(self.problem.TOOL_MISSING, 'lvm')
> > >          if self.options.force and not noforce:
> > > -            command.insert(1, "-f")
> > > +            #For the '--yes' or '-y' added in upstream lvm2 version
> > v2_02_169.
> > > +            if command[0] == 'pvresize':
> > > +                if LVM_VERSION[2] > 168:
> > > +                    command.insert(1, "--yes")
> > > +            else:
> > > +                command.insert(1, "-f")
> >
> > If I understand this correctly it means that pvresize before lvm version
> > 2.02.169 did not require any confirmation whatsoever, Is that right ?
> >
> 
> Yes. It is correct.
> 
> Some operations are interactive after lvm2 version 168 and for the same we
> added above code.
> 
> 
> > >          if self.options.verbose:
> > >              command.insert(1, "-v")
> > >          command.insert(0, "lvm")
> > > @@ -373,9 +378,9 @@ class PvsInfo(LvmInfo, template.BackendDevice):
> > >          super(PvsInfo, self).__init__(*args, **kwargs)
> > >          command = ["lvm", "pvs", "--separator", "|", "--noheadings",
> > >                     "--nosuffix", "--units", "k", "-o",
> > > -                   "pv_name,vg_name,pv_free,pv_used,pv_size"]
> > > +                   "pv_name,vg_name,pv_free,pv_used,pv_size,dev_size"]
> > >          self.attrs = ['dev_name', 'pool_name', 'dev_free',
> > > -                      'dev_used', 'dev_size']
> > > +                      'dev_used', 'dev_size', 'physical_size']
> > >
> > >          self._parse_data(command)
> > >
> > > @@ -407,6 +412,10 @@ class PvsInfo(LvmInfo, template.BackendDevice):
> > >          command.extend(devices)
> > >          self.run_lvm(command)
> > >
> > > +    def resize(self, device, size):
> > > +        command = ['pvresize', '--setphysicalvolumesize', str(size) +
> > 'k', device]
> > > +        self.run_lvm(command)
> >
> > Again, just note that only PvsInfo() is using this method. Yes. As this is
> > specifically for the PV.
> >
> > > +
> > >
> > >  class LvsInfo(LvmInfo, template.BackendVolume):
> > >
> > > diff --git a/ssmlib/main.py b/ssmlib/main.py
> > > index 06185e6..2c44caf 100644
> > > --- a/ssmlib/main.py
> > > +++ b/ssmlib/main.py
> > > @@ -160,6 +160,13 @@ def calculate_size(arg_size, pool):
> > >
> > >      return base_size *  mult
> > >
> > > +def calculate_pvresize_size(arg_size, volume):
> > > +    if arg_size[1] == 'K':
> > > +        base_size = float(arg_size[0])
> > > +    if arg_size[0][0] in ['+', '-']:
> > > +        raise PR.error("Extending PV is supported. Changing size with
> > '+' or '-' is not supported")
> >
> > Why is it that we only support Extending ? Using -s +10G is also
> > extending, isn't it ? I do not understand why you decided not to support
> > +,- signs to size. Am I missing something ?
> >
> > Also does that mean that we do not support shrinking ? Why ?
> >
> 
> We didnot implemented supporting `ssm resize -s +/-size device`  is because
> pvresize command does not support this.
> 
> In order to extend and to reduce the size, same command is used. In a
> sense, pvresize asks for the size that would be final for the PV.
> 
> For example, for 1 GB PV
> 
> pvresize --setphysicalvolumesize 2G /dev/sdb2 --yes  <-- will extend the PV
> to 2 GB
> 
> pvresize --setphysicalvolumesize 500M /dev/sdb2  <-- will reduce the size
> to 500M

Regardless of whether pvresize specifically, or any other tool in any
other backend supports it, ssm should be able to handle it since it
already does for any other resize operation.

Again, ssm is not just a different way to call pvresize it's supposed to
provied added value of making things easier more comperhensive and uniform
across different backends. It's not always 100% possible to achieve, but
this definitelly is.

Thanks!
-Lukas

> 
> 
> 
> > > +    return base_size
> > > +
> > >  def calculate_resize_size(arg_size, volume, pool):
> > >      vol_size = float(volume['vol_size'])
> > >
> > > @@ -469,6 +476,11 @@ class DeviceInfo(object):
> > >                   "to achieve by removing " +
> > >                   "{0}".format(device))
> > >
> > > +    def resize(self, device, size):
> > > +        obj = self.data[device]
> > > +        backend = obj['backend']
> > > +        backend.resize(device, size)
> >
> > So again, both 'backend' and resize() method are only defined in
> > PvsInfo, so what does happen when we call ssm resize <device> on the
> > device that's not pv ?
> >
> > I had tested by giving wrong PV name and it gives a message that the
> device is not valid
> 
> # ./bin/ssm.local resize /dev/sdd12345678
> usage: ssm resize [-h] [-s SIZE] volume [device [device ...]]
> ssm resize: error: argument volume: '/dev/sdd12345678' is not a valid
> volume to resize
> 
> Now we see that sdd1 is not a PV
> 
> # pvs
>   PV         VG             Fmt  Attr PSize   PFree
>   /dev/sda2  rhel_vm252-140 lvm2 a--  <80.13g  4.00m
>   /dev/sda3                 lvm2 ---    5.00g  5.00g
>   /dev/sdb1  vg3            lvm2 a--   <5.00g <3.00g
>   /dev/sdb2                 lvm2 ---   10.00g 10.00g
> 
> If we try to use this device it throws an error
> 
> # ./bin/ssm.local resize /dev/sdd1
> Traceback (most recent call last):
>   File "/root/ssm_Feb21_2019/bin/ssm", line 59, in <module>
>     sys.exit(main.main())
>   File "/root/ssm_Feb21_2019/ssmlib/main.py", line 2952, in main
>     args.func(args)
>   File "/root/ssm_Feb21_2019/ssmlib/main.py", line 1829, in resize
>     self.resize_pv(args)
> 
> I will fix this. Seems I didn't tested this or something has broken.
> 
> 
> Thanks!
> > -Lukas
> >
> > > +
> > >      def migrate(self, vg, source_dev, target_dev):
> > >          """ Migrate (in this case clone) the device onto another one.
> > >              Pure dd is used.
> > > --
> > > 2.17.2
> > >
> > >
> > >
> > > _______________________________________________
> > > storagemanager-devel mailing list
> > > sto...@li...
> > > https://lists.sourceforge.net/lists/listinfo/storagemanager-devel
> > > System Storage Manager (
> > http://sourceforge.net/p/storagemanager/home/Home/)
> >
> 
> 
> -- 
> Thank you,
> Nitin Yewale

Re: [ssm-devel] [PATCH 2/3] Infrastructure to resize PV.

[Nitin Y. <ny...@re...>]

Hello Lukas,

Thank you for the response.

Please find answers inline below

On Thu, Mar 7, 2019 at 6:27 PM Lukas Czerner <lcz...@re...> wrote:

> On Tue, Mar 05, 2019 at 06:36:02PM +0530, Nitin U. Yewale wrote:
> > ---
> > From 3ac26c27f717e0e60d4b02460e5e1110f8b5e8c4 Mon Sep 17 00:00:00 2001
> > From: "Nitin U. Yewale" <ny...@re...>
> > Date: Thu, 21 Feb 2019 18:24:18 +0530
> >
> >     Infrastructure to resize PV.
> >
> > Signed-off-by: Jan Tulak <jt...@re...>
> > Signed-off-by: Nitin U. Yewale <ny...@re...>
> >
> >  ssmlib/backends/lvm.py | 15 ++++++++++++---
> >  ssmlib/main.py         | 12 ++++++++++++
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/ssmlib/backends/lvm.py b/ssmlib/backends/lvm.py
> > index 3bc98c7..7fbea6e 100644
> > --- a/ssmlib/backends/lvm.py
> > +++ b/ssmlib/backends/lvm.py
> > @@ -95,7 +95,12 @@ class LvmInfo(template.Backend):
> >          if not self.binary:
> >              self.problem.check(self.problem.TOOL_MISSING, 'lvm')
> >          if self.options.force and not noforce:
> > -            command.insert(1, "-f")
> > +            #For the '--yes' or '-y' added in upstream lvm2 version
> v2_02_169.
> > +            if command[0] == 'pvresize':
> > +                if LVM_VERSION[2] > 168:
> > +                    command.insert(1, "--yes")
> > +            else:
> > +                command.insert(1, "-f")
>
> If I understand this correctly it means that pvresize before lvm version
> 2.02.169 did not require any confirmation whatsoever, Is that right ?
>

Yes. It is correct.

Some operations are interactive after lvm2 version 168 and for the same we
added above code.


> >          if self.options.verbose:
> >              command.insert(1, "-v")
> >          command.insert(0, "lvm")
> > @@ -373,9 +378,9 @@ class PvsInfo(LvmInfo, template.BackendDevice):
> >          super(PvsInfo, self).__init__(*args, **kwargs)
> >          command = ["lvm", "pvs", "--separator", "|", "--noheadings",
> >                     "--nosuffix", "--units", "k", "-o",
> > -                   "pv_name,vg_name,pv_free,pv_used,pv_size"]
> > +                   "pv_name,vg_name,pv_free,pv_used,pv_size,dev_size"]
> >          self.attrs = ['dev_name', 'pool_name', 'dev_free',
> > -                      'dev_used', 'dev_size']
> > +                      'dev_used', 'dev_size', 'physical_size']
> >
> >          self._parse_data(command)
> >
> > @@ -407,6 +412,10 @@ class PvsInfo(LvmInfo, template.BackendDevice):
> >          command.extend(devices)
> >          self.run_lvm(command)
> >
> > +    def resize(self, device, size):
> > +        command = ['pvresize', '--setphysicalvolumesize', str(size) +
> 'k', device]
> > +        self.run_lvm(command)
>
> Again, just note that only PvsInfo() is using this method. Yes. As this is
> specifically for the PV.
>
> > +
> >
> >  class LvsInfo(LvmInfo, template.BackendVolume):
> >
> > diff --git a/ssmlib/main.py b/ssmlib/main.py
> > index 06185e6..2c44caf 100644
> > --- a/ssmlib/main.py
> > +++ b/ssmlib/main.py
> > @@ -160,6 +160,13 @@ def calculate_size(arg_size, pool):
> >
> >      return base_size *  mult
> >
> > +def calculate_pvresize_size(arg_size, volume):
> > +    if arg_size[1] == 'K':
> > +        base_size = float(arg_size[0])
> > +    if arg_size[0][0] in ['+', '-']:
> > +        raise PR.error("Extending PV is supported. Changing size with
> '+' or '-' is not supported")
>
> Why is it that we only support Extending ? Using -s +10G is also
> extending, isn't it ? I do not understand why you decided not to support
> +,- signs to size. Am I missing something ?
>
> Also does that mean that we do not support shrinking ? Why ?
>

We didnot implemented supporting `ssm resize -s +/-size device`  is because
pvresize command does not support this.

In order to extend and to reduce the size, same command is used. In a
sense, pvresize asks for the size that would be final for the PV.

For example, for 1 GB PV

pvresize --setphysicalvolumesize 2G /dev/sdb2 --yes  <-- will extend the PV
to 2 GB

pvresize --setphysicalvolumesize 500M /dev/sdb2  <-- will reduce the size
to 500M



> > +    return base_size
> > +
> >  def calculate_resize_size(arg_size, volume, pool):
> >      vol_size = float(volume['vol_size'])
> >
> > @@ -469,6 +476,11 @@ class DeviceInfo(object):
> >                   "to achieve by removing " +
> >                   "{0}".format(device))
> >
> > +    def resize(self, device, size):
> > +        obj = self.data[device]
> > +        backend = obj['backend']
> > +        backend.resize(device, size)
>
> So again, both 'backend' and resize() method are only defined in
> PvsInfo, so what does happen when we call ssm resize <device> on the
> device that's not pv ?
>
> I had tested by giving wrong PV name and it gives a message that the
device is not valid

# ./bin/ssm.local resize /dev/sdd12345678
usage: ssm resize [-h] [-s SIZE] volume [device [device ...]]
ssm resize: error: argument volume: '/dev/sdd12345678' is not a valid
volume to resize

Now we see that sdd1 is not a PV

# pvs
  PV         VG             Fmt  Attr PSize   PFree
  /dev/sda2  rhel_vm252-140 lvm2 a--  <80.13g  4.00m
  /dev/sda3                 lvm2 ---    5.00g  5.00g
  /dev/sdb1  vg3            lvm2 a--   <5.00g <3.00g
  /dev/sdb2                 lvm2 ---   10.00g 10.00g

If we try to use this device it throws an error

# ./bin/ssm.local resize /dev/sdd1
Traceback (most recent call last):
  File "/root/ssm_Feb21_2019/bin/ssm", line 59, in <module>
    sys.exit(main.main())
  File "/root/ssm_Feb21_2019/ssmlib/main.py", line 2952, in main
    args.func(args)
  File "/root/ssm_Feb21_2019/ssmlib/main.py", line 1829, in resize
    self.resize_pv(args)

I will fix this. Seems I didn't tested this or something has broken.


Thanks!
> -Lukas
>
> > +
> >      def migrate(self, vg, source_dev, target_dev):
> >          """ Migrate (in this case clone) the device onto another one.
> >              Pure dd is used.
> > --
> > 2.17.2
> >
> >
> >
> > _______________________________________________
> > storagemanager-devel mailing list
> > sto...@li...
> > https://lists.sourceforge.net/lists/listinfo/storagemanager-devel
> > System Storage Manager (
> http://sourceforge.net/p/storagemanager/home/Home/)
>


-- 
Thank you,
Nitin Yewale

Re: [ssm-devel] [PATCH 3/3] Method to resize PV.

[Lukas C. <lcz...@re...>]

On Tue, Mar 05, 2019 at 06:36:03PM +0530, Nitin U. Yewale wrote:
> ---
> From f02d6d142ae9ba2d24bf15ed56bdef59b3a42f1a Mon Sep 17 00:00:00 2001
> From: "Nitin U. Yewale" <ny...@re...>
> Date: Thu, 21 Feb 2019 19:23:38 +0530
>  
>       Method to resize PV.
> 
> Signed-off-by: Jan Tulak <jt...@re...>
> Signed-off-by: Nitin U. Yewale <ny...@re...>
> 
>  ssmlib/main.py                    | 36 ++++++++++++++++++++++++-------
>  tests/bashtests/004-lvm-resize.sh | 10 ++++++++-
>  2 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/ssmlib/main.py b/ssmlib/main.py
> index 2c44caf..8913360 100644
> --- a/ssmlib/main.py
> +++ b/ssmlib/main.py
> @@ -1744,14 +1744,17 @@ class StorageHandle(object):
>  
>          return have_size, devices
>  
> -    def resize(self, args):
> -        """
> -        Resize the volume to the given size. If more devices are provided as
> -        arguments, it will be added into the pool prior to the volume resize
> -        only if the space in the pool is not sufficient. That said, only the
> -        number of devices are added into the pool to be able to cover the
> -        resize.
> -        """
> +    def resize_pv(self, args):

You're working in a generic part of the ssm so it would be better to
avoid backend specific names such as pv and lv.

resize_dev
resize_vol

may be better ?

> +        args.dev = args.volume.name
> +        if args.size is None:
> +            new_size = misc.get_file_size(args.dev)
> +        else:
> +            new_size = calculate_pvresize_size(args.size, args.dev)
> +            if new_size <= 0:
> +                PR.error("New volume size \'{0} KB\' is too small".format(new_size))

Why not to do all of this in calculate_pvresize_size ? Also why to use
misc.get_file_size() since we already have the physical device size
information ?

> +        args.volume.resize(new_size)
> +
> +    def resize_lv(self, args):
>          args.pool = self.pool[args.volume['pool_name']]
>          vol_size = float(args.volume['vol_size'])
>  
> @@ -1813,6 +1816,20 @@ class StorageHandle(object):
>          if new_size != vol_size:
>              args.volume.resize(new_size, fs)
>  
> +    def resize(self, args):
> +        """
> +        Resize the PV or volume to the given size. If more devices are provided as

Again do not use backend specific terminology here. There coul dbe other
backends supporting device resize.

> +        arguments, it will be added into the pool prior to the volume resize
> +        only if the space in the pool is not sufficient. That said, only the
> +        number of devices are added into the pool to be able to cover the
> +        resize.
> +        """
> +        device_type = args.volume.type
> +        if device_type == 'device':
> +            self.resize_pv(args)
> +        else:
> +            self.resize_lv(args)
> +
>      def get_check_passphrase(self):
>          """
>          Ask for a passphrase and check its quality.
> @@ -2334,6 +2351,9 @@ class StorageHandle(object):
>          vol = self.vol[string]
>          if vol:
>              return vol
> +        device = self.dev[string]
> +        if device:
> +            return device
>          err = "'{0}' is not a valid volume to resize".format(string)
>          raise argparse.ArgumentTypeError(err)
>  
> diff --git a/tests/bashtests/004-lvm-resize.sh b/tests/bashtests/004-lvm-resize.sh
> index ed6c4cf..9282837 100755
> --- a/tests/bashtests/004-lvm-resize.sh
> +++ b/tests/bashtests/004-lvm-resize.sh
> @@ -365,12 +365,20 @@ check lv_field $SSM_LVM_DEFAULT_POOL/$tpool1 lv_size ${size}.00m
>  not ssm resize --size 500% $SSM_LVM_DEFAULT_POOL/$tpool1
>  ssm  -f remove --all
>  
> +#Resize PV
> +ssm create $TEST_DEVS
> +ssm -f resize -s 2048M  $dev3
> +check pv_field $dev3 pv_size "<2.00g"

Hmmm, this is new to me - I've never seen '<' I am not sure what that
means and I do not think we're using this in the tests.

Jan is it potential problem for us ? Maybe something that changed
recently ?

> +
> +ssm resize $dev3
> +check pv_field $dev2 pv_size 96.00m

Does it make sense to you ? You resize the pv to the size of the device
(which is I assume what no size specified means) and it's smaller than
it was before ?? Why ?

I'd also like to see some more comperhensive testing

 - extending/shrinking
 - using specified size/no size/+-size
 - change the size of the device then try to resize again
 - how about pv that's part of the pool
 - how about pv that's being completely used in the lv
 - how about pv that's being partially used in the lv
 - how about requesting size bigger than the device
 - how about requesting size smaller than the used portion of the pv
 ...
I am sure you can figure out other scenarios. I'd like the test to check
as much possible cases as possible even those that do not make sense and
should fail.

We need additional testing for other backends and plain devices.

Also unit tests are missing completely so that's something that needs to
be added as well.

Moreover I there is no documentation change at all. This definitelly
needs to be documented in all of our documentation. Jan can you please
explain Nitin how the documentation changes are done ?

Thanks!
-Lukas

> +
>  ssm resize --help
> +ssm -f remove --all


>  
>  # Some cases which should fail
>  not ssm resize
>  not ssm resize _garbage_
> -not ssm resize $dev1
>  ssm create $TEST_DEVS
>  not ssm resize $DEFAULT_VOLUME
>  not ssm -f resize --size +10G $DEFAULT_VOLUME
> -- 
> 2.17.2
> 
> 
> 
> _______________________________________________
> storagemanager-devel mailing list
> sto...@li...
> https://lists.sourceforge.net/lists/listinfo/storagemanager-devel
> System Storage Manager (http://sourceforge.net/p/storagemanager/home/Home/)

Re: [ssm-devel] [PATCH 2/3] Infrastructure to resize PV.

[Lukas C. <lcz...@re...>]

On Tue, Mar 05, 2019 at 06:36:02PM +0530, Nitin U. Yewale wrote:
> ---
> From 3ac26c27f717e0e60d4b02460e5e1110f8b5e8c4 Mon Sep 17 00:00:00 2001
> From: "Nitin U. Yewale" <ny...@re...>
> Date: Thu, 21 Feb 2019 18:24:18 +0530
> 
>     Infrastructure to resize PV.
> 
> Signed-off-by: Jan Tulak <jt...@re...>
> Signed-off-by: Nitin U. Yewale <ny...@re...>
> 
>  ssmlib/backends/lvm.py | 15 ++++++++++++---
>  ssmlib/main.py         | 12 ++++++++++++
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/ssmlib/backends/lvm.py b/ssmlib/backends/lvm.py
> index 3bc98c7..7fbea6e 100644
> --- a/ssmlib/backends/lvm.py
> +++ b/ssmlib/backends/lvm.py
> @@ -95,7 +95,12 @@ class LvmInfo(template.Backend):
>          if not self.binary:
>              self.problem.check(self.problem.TOOL_MISSING, 'lvm')
>          if self.options.force and not noforce:
> -            command.insert(1, "-f")
> +            #For the '--yes' or '-y' added in upstream lvm2 version v2_02_169.
> +            if command[0] == 'pvresize':
> +                if LVM_VERSION[2] > 168:
> +                    command.insert(1, "--yes")
> +            else:
> +                command.insert(1, "-f")

If I understand this correctly it means that pvresize before lvm version
2.02.169 did not require any confirmation whatsoever, Is that right ?

>          if self.options.verbose:
>              command.insert(1, "-v")
>          command.insert(0, "lvm")
> @@ -373,9 +378,9 @@ class PvsInfo(LvmInfo, template.BackendDevice):
>          super(PvsInfo, self).__init__(*args, **kwargs)
>          command = ["lvm", "pvs", "--separator", "|", "--noheadings",
>                     "--nosuffix", "--units", "k", "-o",
> -                   "pv_name,vg_name,pv_free,pv_used,pv_size"]
> +                   "pv_name,vg_name,pv_free,pv_used,pv_size,dev_size"]
>          self.attrs = ['dev_name', 'pool_name', 'dev_free',
> -                      'dev_used', 'dev_size']
> +                      'dev_used', 'dev_size', 'physical_size']
>  
>          self._parse_data(command)
>  
> @@ -407,6 +412,10 @@ class PvsInfo(LvmInfo, template.BackendDevice):
>          command.extend(devices)
>          self.run_lvm(command)
>  
> +    def resize(self, device, size):
> +        command = ['pvresize', '--setphysicalvolumesize', str(size) + 'k', device]
> +        self.run_lvm(command)

Again, just note that only PvsInfo() is using this method.

> +
>  
>  class LvsInfo(LvmInfo, template.BackendVolume):
>  
> diff --git a/ssmlib/main.py b/ssmlib/main.py
> index 06185e6..2c44caf 100644
> --- a/ssmlib/main.py
> +++ b/ssmlib/main.py
> @@ -160,6 +160,13 @@ def calculate_size(arg_size, pool):
>  
>      return base_size *  mult
>  
> +def calculate_pvresize_size(arg_size, volume):
> +    if arg_size[1] == 'K':
> +        base_size = float(arg_size[0])
> +    if arg_size[0][0] in ['+', '-']:
> +        raise PR.error("Extending PV is supported. Changing size with '+' or '-' is not supported")

Why is it that we only support Extending ? Using -s +10G is also
extending, isn't it ? I do not understand why you decided not to support
+,- signs to size. Am I missing something ?

Also does that mean that we do not support shrinking ? Why ?

> +    return base_size
> +
>  def calculate_resize_size(arg_size, volume, pool):
>      vol_size = float(volume['vol_size'])
>  
> @@ -469,6 +476,11 @@ class DeviceInfo(object):
>                   "to achieve by removing " +
>                   "{0}".format(device))
>  
> +    def resize(self, device, size):
> +        obj = self.data[device]
> +        backend = obj['backend']
> +        backend.resize(device, size)

So again, both 'backend' and resize() method are only defined in
PvsInfo, so what does happen when we call ssm resize <device> on the
device that's not pv ?

Thanks!
-Lukas

> +
>      def migrate(self, vg, source_dev, target_dev):
>          """ Migrate (in this case clone) the device onto another one.
>              Pure dd is used.
> -- 
> 2.17.2
> 
> 
> 
> _______________________________________________
> storagemanager-devel mailing list
> sto...@li...
> https://lists.sourceforge.net/lists/listinfo/storagemanager-devel
> System Storage Manager (http://sourceforge.net/p/storagemanager/home/Home/)

Re: [ssm-devel] [PATCH 1/3] To keep PvsInfo accessible at later stages to keep Device/DeviceInfo structure to be the same as e.g. Volumes.

[Lukas C. <lcz...@re...>]

On Tue, Mar 05, 2019 at 06:36:01PM +0530, Nitin U. Yewale wrote:
> Hello,
> 
> Below patches are for the following reason.
> 
> Due to the way how class Devices handles it's initiation, access to
> PvsInfo instances is lost for every device. But we need to keep
> PvsInfo accessible even later on and changing Device/DeviceInfo
> structure to be the same as e.g. Volumes would lead to big changes
> through the whole main.py, so we pass it along as an attribute that's
> copied to DeviceInfo.
> 
> This entirely was an idea of Jan Tulak. This made writing the second and 
> third patch simpler.
> 
> Overall, the end result after all three patches is that we could use ssm 
> to resize the PV. However these three patches separately are equally
> important individually and would help search/edit changes/commits
> easily.
> 
> Thank you,
> Nitin Yewale

Hi Nitin

thanks for your work, I have couple of comments bellow and in the rest
of the patches in the series.

First of all it seems to me that we're not adding any value to calling
this via ssm, compared to calling pvresize. The idea behind ssm is it's
supposed to use the whole stack information and do the right thing. So
I'd think that when I want to resize the device then there are some
following considerations:

 - is the device a partition ? If so is it possible that that's what the
   user asked to resize as well ?
 - is the device used in a pool/volume ? If so is it possible that user
   wants to resize the device despite that ? Should ssm just handle that
   automatically ?
 - Are there any other backends that ssm supports that can benefit with
   device resize support ?

I do not think that simply just calling pvresize is adding much benefit.

Thanks!
-Lukas

> 
> ---
> From 0dde85bb17d810b175b1fa13d3f5f1d22f8a15db Mon Sep 17 00:00:00 2001
> From: "Nitin U. Yewale" <ny...@re...>
> Date: Thu, 21 Feb 2019 18:01:27 +0530
> 
> Due to the way how class Devices handles it's initiation, access to
> PvsInfo instances is lost for every device. But we need to keep
> PvsInfo accessible even later on and changing Device/DeviceInfo
> structure to be the same as e.g. Volumes would lead to big changes
> through the whole main.py, so we pass it along as an attribute that's
> copied to DeviceInfo.
> 
> Signed-off-by: Jan Tulak <jt...@re...> 
> Signed-off-by: Nitin U. Yewale <ny...@re...>
> 
>  ssmlib/backends/lvm.py | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/ssmlib/backends/lvm.py b/ssmlib/backends/lvm.py
> index 5ee57e5..3bc98c7 100644
> --- a/ssmlib/backends/lvm.py
> +++ b/ssmlib/backends/lvm.py
> @@ -384,6 +384,15 @@ class PvsInfo(LvmInfo, template.BackendDevice):
>  
>      def _fill_aditional_info(self, pv):
>          pv['hide'] = False
> +
> +        #Due to the way how class Devices handles it's initiation, access to
> +        #PvsInfo instances is lost for every device. But we need to keep
> +        #PvsInfo accessible even later on and changing Device/DeviceInfo
> +        #structure to be the same as e.g. Volumes would lead to big changes
> +        #through the whole main.py, so we pass it along as an attribute that's
> +        #copied to DeviceInfo.
> +        pv['backend'] = self

Ok, just note that it is only PvsInfo that

> +
>          # If the device is not in any group we do not need this info
>          # and we do not want it to show up in the device listing
>          if not pv['pool_name']:
> -- 
> 2.17.2
> 
> 
> 
> _______________________________________________
> storagemanager-devel mailing list
> sto...@li...
> https://lists.sourceforge.net/lists/listinfo/storagemanager-devel
> System Storage Manager (http://sourceforge.net/p/storagemanager/home/Home/)

[ssm-devel] [PATCH 3/3] Method to resize PV.

[Nitin U. Y. <ny...@re...>]

---
>From f02d6d142ae9ba2d24bf15ed56bdef59b3a42f1a Mon Sep 17 00:00:00 2001
From: "Nitin U. Yewale" <ny...@re...>
Date: Thu, 21 Feb 2019 19:23:38 +0530
 
      Method to resize PV.

Signed-off-by: Jan Tulak <jt...@re...>
Signed-off-by: Nitin U. Yewale <ny...@re...>

 ssmlib/main.py                    | 36 ++++++++++++++++++++++++-------
 tests/bashtests/004-lvm-resize.sh | 10 ++++++++-
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/ssmlib/main.py b/ssmlib/main.py
index 2c44caf..8913360 100644
--- a/ssmlib/main.py
+++ b/ssmlib/main.py
@@ -1744,14 +1744,17 @@ class StorageHandle(object):
 
         return have_size, devices
 
-    def resize(self, args):
-        """
-        Resize the volume to the given size. If more devices are provided as
-        arguments, it will be added into the pool prior to the volume resize
-        only if the space in the pool is not sufficient. That said, only the
-        number of devices are added into the pool to be able to cover the
-        resize.
-        """
+    def resize_pv(self, args):
+        args.dev = args.volume.name
+        if args.size is None:
+            new_size = misc.get_file_size(args.dev)
+        else:
+            new_size = calculate_pvresize_size(args.size, args.dev)
+            if new_size <= 0:
+                PR.error("New volume size \'{0} KB\' is too small".format(new_size))
+        args.volume.resize(new_size)
+
+    def resize_lv(self, args):
         args.pool = self.pool[args.volume['pool_name']]
         vol_size = float(args.volume['vol_size'])
 
@@ -1813,6 +1816,20 @@ class StorageHandle(object):
         if new_size != vol_size:
             args.volume.resize(new_size, fs)
 
+    def resize(self, args):
+        """
+        Resize the PV or volume to the given size. If more devices are provided as
+        arguments, it will be added into the pool prior to the volume resize
+        only if the space in the pool is not sufficient. That said, only the
+        number of devices are added into the pool to be able to cover the
+        resize.
+        """
+        device_type = args.volume.type
+        if device_type == 'device':
+            self.resize_pv(args)
+        else:
+            self.resize_lv(args)
+
     def get_check_passphrase(self):
         """
         Ask for a passphrase and check its quality.
@@ -2334,6 +2351,9 @@ class StorageHandle(object):
         vol = self.vol[string]
         if vol:
             return vol
+        device = self.dev[string]
+        if device:
+            return device
         err = "'{0}' is not a valid volume to resize".format(string)
         raise argparse.ArgumentTypeError(err)
 
diff --git a/tests/bashtests/004-lvm-resize.sh b/tests/bashtests/004-lvm-resize.sh
index ed6c4cf..9282837 100755
--- a/tests/bashtests/004-lvm-resize.sh
+++ b/tests/bashtests/004-lvm-resize.sh
@@ -365,12 +365,20 @@ check lv_field $SSM_LVM_DEFAULT_POOL/$tpool1 lv_size ${size}.00m
 not ssm resize --size 500% $SSM_LVM_DEFAULT_POOL/$tpool1
 ssm  -f remove --all
 
+#Resize PV
+ssm create $TEST_DEVS
+ssm -f resize -s 2048M  $dev3
+check pv_field $dev3 pv_size "<2.00g"
+
+ssm resize $dev3
+check pv_field $dev2 pv_size 96.00m
+
 ssm resize --help
+ssm -f remove --all
 
 # Some cases which should fail
 not ssm resize
 not ssm resize _garbage_
-not ssm resize $dev1
 ssm create $TEST_DEVS
 not ssm resize $DEFAULT_VOLUME
 not ssm -f resize --size +10G $DEFAULT_VOLUME
-- 
2.17.2

[ssm-devel] [PATCH 1/3] To keep PvsInfo accessible at later stages to keep Device/DeviceInfo structure to be the same as e.g. Volumes.

[Nitin U. Y. <ny...@re...>]

Hello,

Below patches are for the following reason.

Due to the way how class Devices handles it's initiation, access to
PvsInfo instances is lost for every device. But we need to keep
PvsInfo accessible even later on and changing Device/DeviceInfo
structure to be the same as e.g. Volumes would lead to big changes
through the whole main.py, so we pass it along as an attribute that's
copied to DeviceInfo.

This entirely was an idea of Jan Tulak. This made writing the second and 
third patch simpler.

Overall, the end result after all three patches is that we could use ssm 
to resize the PV. However these three patches separately are equally
important individually and would help search/edit changes/commits
easily.

Thank you,
Nitin Yewale

---
>From 0dde85bb17d810b175b1fa13d3f5f1d22f8a15db Mon Sep 17 00:00:00 2001
From: "Nitin U. Yewale" <ny...@re...>
Date: Thu, 21 Feb 2019 18:01:27 +0530

Due to the way how class Devices handles it's initiation, access to
PvsInfo instances is lost for every device. But we need to keep
PvsInfo accessible even later on and changing Device/DeviceInfo
structure to be the same as e.g. Volumes would lead to big changes
through the whole main.py, so we pass it along as an attribute that's
copied to DeviceInfo.

Signed-off-by: Jan Tulak <jt...@re...> 
Signed-off-by: Nitin U. Yewale <ny...@re...>

 ssmlib/backends/lvm.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ssmlib/backends/lvm.py b/ssmlib/backends/lvm.py
index 5ee57e5..3bc98c7 100644
--- a/ssmlib/backends/lvm.py
+++ b/ssmlib/backends/lvm.py
@@ -384,6 +384,15 @@ class PvsInfo(LvmInfo, template.BackendDevice):
 
     def _fill_aditional_info(self, pv):
         pv['hide'] = False
+
+        #Due to the way how class Devices handles it's initiation, access to
+        #PvsInfo instances is lost for every device. But we need to keep
+        #PvsInfo accessible even later on and changing Device/DeviceInfo
+        #structure to be the same as e.g. Volumes would lead to big changes
+        #through the whole main.py, so we pass it along as an attribute that's
+        #copied to DeviceInfo.
+        pv['backend'] = self
+
         # If the device is not in any group we do not need this info
         # and we do not want it to show up in the device listing
         if not pv['pool_name']:
-- 
2.17.2

[ssm-devel] [PATCH 2/3] Infrastructure to resize PV.

[Nitin U. Y. <ny...@re...>]

---
>From 3ac26c27f717e0e60d4b02460e5e1110f8b5e8c4 Mon Sep 17 00:00:00 2001
From: "Nitin U. Yewale" <ny...@re...>
Date: Thu, 21 Feb 2019 18:24:18 +0530

    Infrastructure to resize PV.

Signed-off-by: Jan Tulak <jt...@re...>
Signed-off-by: Nitin U. Yewale <ny...@re...>

 ssmlib/backends/lvm.py | 15 ++++++++++++---
 ssmlib/main.py         | 12 ++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/ssmlib/backends/lvm.py b/ssmlib/backends/lvm.py
index 3bc98c7..7fbea6e 100644
--- a/ssmlib/backends/lvm.py
+++ b/ssmlib/backends/lvm.py
@@ -95,7 +95,12 @@ class LvmInfo(template.Backend):
         if not self.binary:
             self.problem.check(self.problem.TOOL_MISSING, 'lvm')
         if self.options.force and not noforce:
-            command.insert(1, "-f")
+            #For the '--yes' or '-y' added in upstream lvm2 version v2_02_169.
+            if command[0] == 'pvresize':
+                if LVM_VERSION[2] > 168:
+                    command.insert(1, "--yes")
+            else:
+                command.insert(1, "-f")
         if self.options.verbose:
             command.insert(1, "-v")
         command.insert(0, "lvm")
@@ -373,9 +378,9 @@ class PvsInfo(LvmInfo, template.BackendDevice):
         super(PvsInfo, self).__init__(*args, **kwargs)
         command = ["lvm", "pvs", "--separator", "|", "--noheadings",
                    "--nosuffix", "--units", "k", "-o",
-                   "pv_name,vg_name,pv_free,pv_used,pv_size"]
+                   "pv_name,vg_name,pv_free,pv_used,pv_size,dev_size"]
         self.attrs = ['dev_name', 'pool_name', 'dev_free',
-                      'dev_used', 'dev_size']
+                      'dev_used', 'dev_size', 'physical_size']
 
         self._parse_data(command)
 
@@ -407,6 +412,10 @@ class PvsInfo(LvmInfo, template.BackendDevice):
         command.extend(devices)
         self.run_lvm(command)
 
+    def resize(self, device, size):
+        command = ['pvresize', '--setphysicalvolumesize', str(size) + 'k', device]
+        self.run_lvm(command)
+
 
 class LvsInfo(LvmInfo, template.BackendVolume):
 
diff --git a/ssmlib/main.py b/ssmlib/main.py
index 06185e6..2c44caf 100644
--- a/ssmlib/main.py
+++ b/ssmlib/main.py
@@ -160,6 +160,13 @@ def calculate_size(arg_size, pool):
 
     return base_size *  mult
 
+def calculate_pvresize_size(arg_size, volume):
+    if arg_size[1] == 'K':
+        base_size = float(arg_size[0])
+    if arg_size[0][0] in ['+', '-']:
+        raise PR.error("Extending PV is supported. Changing size with '+' or '-' is not supported")
+    return base_size
+
 def calculate_resize_size(arg_size, volume, pool):
     vol_size = float(volume['vol_size'])
 
@@ -469,6 +476,11 @@ class DeviceInfo(object):
                  "to achieve by removing " +
                  "{0}".format(device))
 
+    def resize(self, device, size):
+        obj = self.data[device]
+        backend = obj['backend']
+        backend.resize(device, size)
+
     def migrate(self, vg, source_dev, target_dev):
         """ Migrate (in this case clone) the device onto another one.
             Pure dd is used.
-- 
2.17.2

Re: [ssm-devel] [PATCH] ssm: use LC_ALL=C when running tools from ssm

[Lukas C. <lcz...@re...>]

On Thu, Feb 21, 2019 at 04:33:27PM +0100, Jan Tulak wrote:
> Previously, we used LC_NUMERIC, but if LC_ALL is set by the user/system,
> then LC_NUMERIC is ignored, leading to issues with locales with
> different decimal delimiter literal than a dot. So, set LC_ALL instead
> of LC_NUMERIC.
> 
> This forces other localized things like error messages to English. But
> because ssm does not have translations, we can have errors reported by
> tools (like lvm) within ssm always in English as well.
> 
> Resolves: rhbz#1679587
> Resolves: rhbz#1679591
> Resolves: #7 (github)

Looks good.

Reviewed-by: Lukas Czerner <lcz...@re...>

Thanks!
-LUkas

> 
> Signed-off-by: Jan Tulak <jt...@re...>
> ---
>  ssmlib/main.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ssmlib/main.py b/ssmlib/main.py
> index e87c0e6..6cc3219 100644
> --- a/ssmlib/main.py
> +++ b/ssmlib/main.py
> @@ -40,7 +40,7 @@ EXTN = ['ext2', 'ext3', 'ext4']
>  SUPPORTED_FS = ['xfs', 'btrfs'] + EXTN
>  SUPPORTED_BACKENDS = ['lvm', 'btrfs', 'crypt', 'multipath']
>  SUPPORTED_RAID = ['0', '1', '10']
> -os.environ['LC_NUMERIC'] = "C"
> +os.environ['LC_ALL'] = "C"
>  
>  # If you change this please change doc/conf.py as well
>  VERSION = '1.3'
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> storagemanager-devel mailing list
> sto...@li...
> https://lists.sourceforge.net/lists/listinfo/storagemanager-devel
> System Storage Manager (http://sourceforge.net/p/storagemanager/home/Home/)