| Message ID | 20221024061319.1133470-3-yi.zhang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix for xfs log size change from new version of xfsprogs | expand |
On Oct 24, 2022 / 14:13, Yi Zhang wrote: Short explanation will help to understand why we do this: something like, nvme/035 has minimum TEST_DEV size requirement. Add a helper function to check it. > Signed-off-by: Yi Zhang <yi.zhang@redhat.com> > --- > common/rc | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/common/rc b/common/rc > index e490041..847be1b 100644 > --- a/common/rc > +++ b/common/rc > @@ -324,6 +324,14 @@ _get_pci_parent_from_blkdev() { > tail -2 | head -1 > } > > +_get_test_dev_size_mb() { > + local test_dev_sz Nit: one empty line will make it easier to read. > + test_dev_sz=$(blockdev --getsize64 "$TEST_DEV") > + > + echo $((test_dev_sz / 1024 / 1024)) > + Nit: an empty line not needed. > +} > + I suggest to improve this new function to _require_test_dev_size_mb(). It takes 1st argument as the minimum size size in MB, and if TEST_DEV size is smaller than that, it set SKIP_REASON and return 1. We can add device_requires() to nvme/035 to call _require_test_dev_size_mb(). This will skip the test case when the TEST_DEV is small, and do not report it as a failure. I also suggest to include nvme/035 change for the size check in this patch. I think one shot change for function addition and function call will be simpler for this charge. > _require_test_dev_in_hotplug_slot() { > local parent > parent="$(_get_pci_parent_from_blkdev)" > -- > 2.34.1 >
On Tue, Oct 25, 2022 at 10:29 AM Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> wrote: > > On Oct 24, 2022 / 14:13, Yi Zhang wrote: > > Short explanation will help to understand why we do this: something like, > > nvme/035 has minimum TEST_DEV size requirement. Add a helper > function to check it. > > > Signed-off-by: Yi Zhang <yi.zhang@redhat.com> > > --- > > common/rc | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/common/rc b/common/rc > > index e490041..847be1b 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -324,6 +324,14 @@ _get_pci_parent_from_blkdev() { > > tail -2 | head -1 > > } > > > > +_get_test_dev_size_mb() { > > + local test_dev_sz > > Nit: one empty line will make it easier to read. > > > + test_dev_sz=$(blockdev --getsize64 "$TEST_DEV") > > + > > + echo $((test_dev_sz / 1024 / 1024)) > > + > > Nit: an empty line not needed. > > > +} > > + > > I suggest to improve this new function to _require_test_dev_size_mb(). It takes > 1st argument as the minimum size size in MB, and if TEST_DEV size is smaller > than that, it set SKIP_REASON and return 1. We can add device_requires() to > nvme/035 to call _require_test_dev_size_mb(). This will skip the test case when > the TEST_DEV is small, and do not report it as a failure. > > I also suggest to include nvme/035 change for the size check in this patch. I > think one shot change for function addition and function call will be simpler > for this charge. Yeah, that looks better, I already send V2 to fix it, thanks. > > > _require_test_dev_in_hotplug_slot() { > > local parent > > parent="$(_get_pci_parent_from_blkdev)" > > -- > > 2.34.1 > > > > -- > Shin'ichiro Kawasaki >
diff --git a/common/rc b/common/rc index e490041..847be1b 100644 --- a/common/rc +++ b/common/rc @@ -324,6 +324,14 @@ _get_pci_parent_from_blkdev() { tail -2 | head -1 } +_get_test_dev_size_mb() { + local test_dev_sz + test_dev_sz=$(blockdev --getsize64 "$TEST_DEV") + + echo $((test_dev_sz / 1024 / 1024)) + +} + _require_test_dev_in_hotplug_slot() { local parent parent="$(_get_pci_parent_from_blkdev)"
Signed-off-by: Yi Zhang <yi.zhang@redhat.com> --- common/rc | 8 ++++++++ 1 file changed, 8 insertions(+)