Skip to content

Conversation

@ikegami-t
Copy link
Contributor

This is to monitor the system changes.

@ikegami-t
Copy link
Contributor Author

Will do consider to change the PR changes to add --continue or --delay option instead of the nvme top command. (Or just rename to nvme watch command instead.)

@ikegami-t ikegami-t force-pushed the nvme-top branch 2 times, most recently from 2a0fb7a to 50ece42 Compare December 29, 2025 07:23
@ikegami-t
Copy link
Contributor Author

Hi @igaw, @shroffni and @martin-belanger,

Just updated the PR changes to add --delay option instead of nvme top command (but not --continuous option) as mentioned by the matrix chat.

@shroffni
Copy link
Contributor

shroffni commented Jan 7, 2026

After looking at the code, it does not seem appropriate to add a --delay option at the global configuration level.

The main issue is that a global --delay does not make sense for all nvme-cli commands. For example, commands such as create-ns or delete-ns should not accept a --delay option. If create-ns were invoked with a non-zero delay, it could repeatedly create namespaces in a loop until the device runs out of space. Similarly, commands like delete-ns, set-feature, format, and several others have no meaningful semantics when executed periodically with a delay.

If the goal is simply to monitor or repeatedly execute a command, the existing watch utility already provides this functionality. While watch may not be the most efficient solution, it is sufficient for user-space tooling and works reasonably well today.

If we still believe that a --delay option is necessary, then it should be added selectively, only to those nvme-cli sub-commands where periodic execution actually makes sense. One such example is show-topology, and this could also be useful for upcoming tools like nvme-top.

In summary, rather than introducing a global --delay option, it would be better to:

  • Identify the specific sub-commands where delayed or periodic execution is meaningful, and
  • Add support for --delay only to those commands.

@igaw
Copy link
Collaborator

igaw commented Jan 7, 2026

I agree with @shroffni's arguments.

This is to monitor the system changes.

Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
@ikegami-t
Copy link
Contributor Author

Just changed the PR commit to add the option only for the show-topology command at first. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants