Skip to content

nvme-print-stdout: add format command relatives prints#2833

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
ikegami-t:format-show-relatives
Jun 30, 2025
Merged

nvme-print-stdout: add format command relatives prints#2833
igaw merged 1 commit intolinux-nvme:masterfrom
ikegami-t:format-show-relatives

Conversation

@ikegami-t
Copy link
Contributor

The output changed as TBD by the commit 18de3a6 before.

{
int id;
int nsid;
int ret = sscanf(name, "nvme%dn%d", &id, &nsid);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid mixing declaration and assignment. Also order the variables so we get an inverse Christmas tree style.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed as mentioned. Thank you. By the way is it okay to assign the bool values below to remain?

	bool block = true;
	bool first = true;

@ikegami-t ikegami-t force-pushed the format-show-relatives branch from 77557f0 to ac5fde3 Compare June 2, 2025 14:27
nvme-print.c Outdated
return;
}

err = nvme_scan_topology(r, NULL, NULL);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead creating a new root and scan sysfs again, just pass in the nvme_root_t object into nvme_show_relatives.

nvme-print.h Outdated
void (*predictable_latency_event_agg_log)(struct nvme_aggregate_predictable_lat_event *pea_log, __u64 log_entries, __u32 size, const char *devname);
void (*predictable_latency_per_nvmset)(struct nvme_nvmset_predictable_lat_log *plpns_log, __u16 nvmset_id, const char *devname);
void (*primary_ctrl_cap)(const struct nvme_primary_ctrl_cap *caps);
void (*relatives)(const char *name, nvme_root_t r);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe swap the arguments, first nvme_root_t and then name. This is more consistent I think.

nvme.c Outdated
dev->name, cfg.namespace_id,
cfg.namespace_id == NVME_NSID_ALL ? "(ALL namespaces)" : "");
nvme_show_relatives(dev->name);
nvme_show_relatives(dev->name, flags);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see there is no root object here, but I think it is better to keep the nvme_create_root and nvme_scan_topology on the same level, that is we create and maintain this object on in this file/level/hierarchy. If we move it down into the print level it seems a bit off.

The output changed as TBD by the commit 18de3a6 before.

Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
@ikegami-t ikegami-t force-pushed the format-show-relatives branch from ac5fde3 to d63b1ca Compare June 3, 2025 16:47
@ikegami-t
Copy link
Contributor Author

Updated the changes as commented. Thank you.

@igaw igaw merged commit 0264cec into linux-nvme:master Jun 30, 2025
17 checks passed
@igaw
Copy link
Collaborator

igaw commented Jun 30, 2025

Thanks!

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.

2 participants