* [PATCH] ceph/001: skip metrics check if no copyfrom mount option is used @ 2022-05-20 10:50 Luís Henriques 2022-05-24 4:45 ` Zorro Lang 0 siblings, 1 reply; 4+ messages in thread From: Luís Henriques @ 2022-05-20 10:50 UTC (permalink / raw) To: fstests; +Cc: ceph-devel, Luís Henriques Checking the metrics is only valid if 'copyfrom' mount option is explicitly set, otherwise the kernel won't be doing any remote object copies. Fix the logic to skip this metrics checking if 'copyfrom' isn't used. Signed-off-by: Luís Henriques <lhenriques@suse.de> --- tests/ceph/001 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/ceph/001 b/tests/ceph/001 index 7970ce352bab..2e6a5e6be2d6 100755 --- a/tests/ceph/001 +++ b/tests/ceph/001 @@ -86,11 +86,15 @@ check_copyfrom_metrics() local copies=$4 local c1=$(get_copyfrom_total_copies) local s1=$(get_copyfrom_total_size) + local hascopyfrom=$(_fs_options $TEST_DEV | grep "copyfrom") local sum if [ ! -d $metrics_dir ]; then return # skip metrics check if debugfs isn't mounted fi + if [ -z $hascopyfrom ]; then + return # ... or if we don't have copyfrom mount option + fi sum=$(($c0+$copies)) if [ $sum -ne $c1 ]; then ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph/001: skip metrics check if no copyfrom mount option is used 2022-05-20 10:50 [PATCH] ceph/001: skip metrics check if no copyfrom mount option is used Luís Henriques @ 2022-05-24 4:45 ` Zorro Lang 2022-05-24 5:04 ` Zorro Lang 0 siblings, 1 reply; 4+ messages in thread From: Zorro Lang @ 2022-05-24 4:45 UTC (permalink / raw) To: Luís Henriques; +Cc: fstests, ceph-devel On Fri, May 20, 2022 at 11:50:55AM +0100, Luís Henriques wrote: > Checking the metrics is only valid if 'copyfrom' mount option is > explicitly set, otherwise the kernel won't be doing any remote object > copies. Fix the logic to skip this metrics checking if 'copyfrom' isn't > used. > > Signed-off-by: Luís Henriques <lhenriques@suse.de> > --- The code logic looks good to me, but I'm not familiar with cephfs, so if there's not review points or objection from ceph-fs, I'll merge this patch this week. Reviewed-by: Zorro Lang <zlang@redhat.com> Thanks, Zorro > tests/ceph/001 | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tests/ceph/001 b/tests/ceph/001 > index 7970ce352bab..2e6a5e6be2d6 100755 > --- a/tests/ceph/001 > +++ b/tests/ceph/001 > @@ -86,11 +86,15 @@ check_copyfrom_metrics() > local copies=$4 > local c1=$(get_copyfrom_total_copies) > local s1=$(get_copyfrom_total_size) > + local hascopyfrom=$(_fs_options $TEST_DEV | grep "copyfrom") > local sum > > if [ ! -d $metrics_dir ]; then > return # skip metrics check if debugfs isn't mounted > fi > + if [ -z $hascopyfrom ]; then > + return # ... or if we don't have copyfrom mount option > + fi > > sum=$(($c0+$copies)) > if [ $sum -ne $c1 ]; then > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph/001: skip metrics check if no copyfrom mount option is used 2022-05-24 4:45 ` Zorro Lang @ 2022-05-24 5:04 ` Zorro Lang 2022-05-24 9:37 ` Luís Henriques 0 siblings, 1 reply; 4+ messages in thread From: Zorro Lang @ 2022-05-24 5:04 UTC (permalink / raw) To: Luís Henriques; +Cc: fstests, ceph-devel On Tue, May 24, 2022 at 12:45:07PM +0800, Zorro Lang wrote: > On Fri, May 20, 2022 at 11:50:55AM +0100, Luís Henriques wrote: > > Checking the metrics is only valid if 'copyfrom' mount option is > > explicitly set, otherwise the kernel won't be doing any remote object > > copies. Fix the logic to skip this metrics checking if 'copyfrom' isn't > > used. > > > > Signed-off-by: Luís Henriques <lhenriques@suse.de> > > --- > > The code logic looks good to me, but I'm not familiar with cephfs, so if there's > not review points or objection from ceph-fs, I'll merge this patch this week. > > Reviewed-by: Zorro Lang <zlang@redhat.com> > > Thanks, > Zorro > > > tests/ceph/001 | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/tests/ceph/001 b/tests/ceph/001 > > index 7970ce352bab..2e6a5e6be2d6 100755 > > --- a/tests/ceph/001 > > +++ b/tests/ceph/001 > > @@ -86,11 +86,15 @@ check_copyfrom_metrics() > > local copies=$4 > > local c1=$(get_copyfrom_total_copies) > > local s1=$(get_copyfrom_total_size) > > + local hascopyfrom=$(_fs_options $TEST_DEV | grep "copyfrom") Oh, I forgot to metion that: I don't know what's the mount option output format at here, can't sure if "grep -w" is needed, or need special pattern. > > local sum > > > > if [ ! -d $metrics_dir ]; then > > return # skip metrics check if debugfs isn't mounted > > fi > > + if [ -z $hascopyfrom ]; then I think better to use quotes ("$hascopyfrom") at here. > > + return # ... or if we don't have copyfrom mount option > > + fi > > > > sum=$(($c0+$copies)) > > if [ $sum -ne $c1 ]; then > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph/001: skip metrics check if no copyfrom mount option is used 2022-05-24 5:04 ` Zorro Lang @ 2022-05-24 9:37 ` Luís Henriques 0 siblings, 0 replies; 4+ messages in thread From: Luís Henriques @ 2022-05-24 9:37 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests, ceph-devel Zorro Lang <zlang@redhat.com> writes: > On Tue, May 24, 2022 at 12:45:07PM +0800, Zorro Lang wrote: >> On Fri, May 20, 2022 at 11:50:55AM +0100, Luís Henriques wrote: >> > Checking the metrics is only valid if 'copyfrom' mount option is >> > explicitly set, otherwise the kernel won't be doing any remote object >> > copies. Fix the logic to skip this metrics checking if 'copyfrom' isn't >> > used. >> > >> > Signed-off-by: Luís Henriques <lhenriques@suse.de> >> > --- >> >> The code logic looks good to me, but I'm not familiar with cephfs, so if there's >> not review points or objection from ceph-fs, I'll merge this patch this week. >> >> Reviewed-by: Zorro Lang <zlang@redhat.com> >> >> Thanks, >> Zorro >> >> > tests/ceph/001 | 4 ++++ >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/tests/ceph/001 b/tests/ceph/001 >> > index 7970ce352bab..2e6a5e6be2d6 100755 >> > --- a/tests/ceph/001 >> > +++ b/tests/ceph/001 >> > @@ -86,11 +86,15 @@ check_copyfrom_metrics() >> > local copies=$4 >> > local c1=$(get_copyfrom_total_copies) >> > local s1=$(get_copyfrom_total_size) >> > + local hascopyfrom=$(_fs_options $TEST_DEV | grep "copyfrom") > > Oh, I forgot to metion that: > > I don't know what's the mount option output format at here, can't sure if > "grep -w" is needed, or need special pattern. I don't think there's a need for the '-w' here; the mount option is exactly that string: 192.168.155.1:40768:/ on /mnt type ceph (rw,relatime,name=user,secret=<hidden>,acl,copyfrom) >> > local sum >> > >> > if [ ! -d $metrics_dir ]; then >> > return # skip metrics check if debugfs isn't mounted >> > fi >> > + if [ -z $hascopyfrom ]; then > > I think better to use quotes ("$hascopyfrom") at here. I'll send v2 with that changed. Thank you for your feedback. Cheers, -- Luís >> > + return # ... or if we don't have copyfrom mount option >> > + fi >> > >> > sum=$(($c0+$copies)) >> > if [ $sum -ne $c1 ]; then >> > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-24 9:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-05-20 10:50 [PATCH] ceph/001: skip metrics check if no copyfrom mount option is used Luís Henriques 2022-05-24 4:45 ` Zorro Lang 2022-05-24 5:04 ` Zorro Lang 2022-05-24 9:37 ` Luís Henriques
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.