cephfs-top: fix exceptions on small/large sized windows by joscollin · Pull Request #59566 · ceph/ceph

joscollin · 2024-09-02T14:32:01Z

Fixes exception: curses function returned NULL when the window width is larger than expected.
Reproduced this issue on a 32 inch horizontal monitor. Tested the fix on 32 inch horizontal + 24 inch vertical monitor(s) (dual).
Fixes exception: addwstr() returned ERR when the window width is smaller than expected.

Fixes: https://tracker.ceph.com/issues/67859

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

joscollin · 2024-09-03T13:16:28Z

jenkins test make check

joscollin · 2024-09-03T13:16:43Z

jenkins test make check arm64

joscollin · 2024-09-04T12:53:32Z

jenkins test api

joscollin · 2024-09-04T12:53:42Z

jenkins test make check

joscollin · 2024-09-05T03:57:43Z

jenkins test api

src/tools/cephfs/top/cephfs-top

neesingh-rh · 2024-09-05T09:06:55Z

@joscollin I looked into the second issue earlier through the BZ https://bugzilla.redhat.com/show_bug.cgi?id=2272580, and had this fix. Anyways, thanks for adding this fix into this PR. I have linked the BZ with the tracker for this PR.
If there's no issue, should I assign the respective BZ to you, as you have the PR for the fix?

joscollin · 2024-09-05T09:14:06Z

@joscollin I looked into the second issue earlier through the BZ https://bugzilla.redhat.com/show_bug.cgi?id=2272580, and had this fix. Anyways, thanks for adding this fix into this PR. I have linked the BZ with the tracker for this PR. If there's no issue, should I assign the respective BZ to you, as you have the PR for the fix?

Please assign it to me, I wasn't aware that the BZ exists. Thanks @neesingh-rh

src/tools/cephfs/top/cephfs-top

Fixes "exception: curses function returned NULL" when the window width is larger than expected. Fixes: https://tracker.ceph.com/issues/67859 Signed-off-by: Jos Collin <jcollin@redhat.com>

joscollin · 2024-09-06T05:04:26Z

Fixed the tox failures.

joscollin · 2024-09-06T05:17:11Z

jenkins test make check

joscollin · 2024-09-06T09:54:15Z

jenkins test make check

Fixes "exception: addwstr() returned ERR" when the window width is smaller than expected. Fixes: https://tracker.ceph.com/issues/67859 Signed-off-by: Jos Collin <jcollin@redhat.com>

joscollin · 2024-09-09T03:59:53Z

@vshankar Work is done with this. Please do a review and QA.

joscollin · 2024-09-09T11:19:55Z

jenkins test make check arm64

vshankar

LGTM. I assume this has been manually verified since there is no way to test this in teuthology, yes? @joscollin

joscollin · 2024-09-10T08:41:49Z

Yes, its manually verified, as mentioned in the PR description. No issues found.

…

On Tuesday 10 September 2024, Venky Shankar ***@***.***> wrote: ***@***.**** approved this pull request. LGTM. I assume this has been manually verified since there is no way to test this in teuthology, yes? @joscollin <https://github.com/joscollin> — Reply to this email directly, view it on GitHub <#59566 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFVDREQESCMEPY23PIML4NTZV2PTJAVCNFSM6AAAAABNQOTOC2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOJRGYZTANBRHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

joscollin · 2024-09-16T05:05:18Z

LGTM. I assume this has been manually verified since there is no way to test this in teuthology, yes? @joscollin

@vshankar Can you merge this then?

joscollin · 2024-09-20T04:59:55Z

@vshankar ping

vshankar · 2024-09-20T09:49:38Z

LGTM. I assume this has been manually verified since there is no way to test this in teuthology, yes? @joscollin

@vshankar Can you merge this then?

yes. For some reason your comment via email did't show up as a notification.

joscollin marked this pull request as draft September 2, 2024 14:32

github-actions bot added the cephfs Ceph File System label Sep 2, 2024

joscollin force-pushed the wip-B67859-cephfs-top-exception-large-windows branch from a6abfcc to 77e46c1 Compare September 4, 2024 07:09

joscollin marked this pull request as ready for review September 4, 2024 07:11

joscollin force-pushed the wip-B67859-cephfs-top-exception-large-windows branch from 77e46c1 to 2eafe5f Compare September 4, 2024 08:27

joscollin added the bug-fix label Sep 4, 2024

joscollin force-pushed the wip-B67859-cephfs-top-exception-large-windows branch from 2eafe5f to 1c3b2c4 Compare September 4, 2024 10:25

joscollin requested a review from a team September 5, 2024 08:12

dparmar18 approved these changes Sep 5, 2024

View reviewed changes

dparmar18 reviewed Sep 5, 2024

View reviewed changes

src/tools/cephfs/top/cephfs-top Show resolved Hide resolved

joscollin changed the title ~~cephfs-top: fix exception on large sized windows~~ cephfs-top: fix exceptions on small/large sized windows Sep 5, 2024

joscollin force-pushed the wip-B67859-cephfs-top-exception-large-windows branch 2 times, most recently from af21d45 to 5441144 Compare September 5, 2024 08:55

neesingh-rh approved these changes Sep 5, 2024

View reviewed changes

dparmar18 reviewed Sep 5, 2024

View reviewed changes

src/tools/cephfs/top/cephfs-top Outdated Show resolved Hide resolved

joscollin force-pushed the wip-B67859-cephfs-top-exception-large-windows branch 2 times, most recently from 8a2898d to 9136289 Compare September 5, 2024 12:10

cephfs-top: fix exception on large sized windows

a59f007

Fixes "exception: curses function returned NULL" when the window width is larger than expected. Fixes: https://tracker.ceph.com/issues/67859 Signed-off-by: Jos Collin <jcollin@redhat.com>

joscollin force-pushed the wip-B67859-cephfs-top-exception-large-windows branch from 9136289 to ecf9bb5 Compare September 6, 2024 05:03

cephfs-top: fix exception on small sized windows

926f91c

Fixes "exception: addwstr() returned ERR" when the window width is smaller than expected. Fixes: https://tracker.ceph.com/issues/67859 Signed-off-by: Jos Collin <jcollin@redhat.com>

joscollin force-pushed the wip-B67859-cephfs-top-exception-large-windows branch from ecf9bb5 to 926f91c Compare September 6, 2024 11:37

joscollin requested a review from vshankar September 6, 2024 11:54

gregsfortytwo added the needs-qa label Sep 9, 2024

vshankar approved these changes Sep 10, 2024

View reviewed changes

vshankar merged commit e0b974a into ceph:main Sep 20, 2024

joscollin deleted the wip-B67859-cephfs-top-exception-large-windows branch September 20, 2024 09:53

This was referenced Sep 20, 2024

reef: cephfs-top: fix exceptions on small/large sized windows #59898

Merged

squid: cephfs-top: fix exceptions on small/large sized windows #59899

Merged

Conversation

joscollin commented Sep 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

joscollin commented Sep 3, 2024

Uh oh!

joscollin commented Sep 3, 2024

Uh oh!

joscollin commented Sep 4, 2024

Uh oh!

joscollin commented Sep 4, 2024

Uh oh!

joscollin commented Sep 5, 2024

Uh oh!

Uh oh!

neesingh-rh commented Sep 5, 2024

Uh oh!

joscollin commented Sep 5, 2024

Uh oh!

Uh oh!

joscollin commented Sep 6, 2024

Uh oh!

joscollin commented Sep 6, 2024

Uh oh!

joscollin commented Sep 6, 2024

Uh oh!

joscollin commented Sep 9, 2024

Uh oh!

joscollin commented Sep 9, 2024

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

joscollin commented Sep 10, 2024 via email

Uh oh!

joscollin commented Sep 16, 2024

Uh oh!

joscollin commented Sep 20, 2024

Uh oh!

vshankar commented Sep 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

joscollin commented Sep 2, 2024 •

edited

Loading