Skip to content

amzn: enable amzn-nvme.c build without json-c dependencies checking #2627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
igaw merged 2 commits into from
Dec 20, 2024
Merged

amzn: enable amzn-nvme.c build without json-c dependencies checking #2627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d92e2ef
amzn: enable amzn-nvme.c build without json-c dependencies checking
ikegami-t Dec 18, 2024
bd6850c
amzn: use PRIu64 printf format string instead of llu
ikegami-t Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 32 additions & 17 deletions plugins/amzn/amzn-nvme.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,21 @@
#include "nvme.h"
#include "libnvme.h"
#include "plugin.h"
#include "nvme-print.h"

#define CREATE_CMD
#include "amzn-nvme.h"

#define AMZN_NVME_STATS_LOGPAGE_ID 0xD0
#define AMZN_NVME_STATS_MAGIC 0x3C23B510

#ifdef CONFIG_JSONC
#define array_add_obj json_array_add_value_object
#define obj_add_array json_object_add_value_array
#define obj_add_obj json_object_add_value_object
#define obj_add_uint json_object_add_value_uint
#define obj_add_uint64 json_object_add_value_uint64
#endif /* CONFIG_JSONC */

struct nvme_vu_id_ctrl_field {
__u8 bdev[32];
Expand Down Expand Up @@ -105,13 +108,14 @@ static void amzn_print_latency_histogram(struct amzn_latency_histogram *hist)
for (int b = 0; b < hist->num_bins && b < 64; b++) {
struct amzn_latency_histogram_bin *bin = &hist->bins[b];

printf("[%-8llu - %-8llu] => %-8u\n",
bin->lower, bin->upper, bin->count);
printf("[%-8"PRIu64" - %-8"PRIu64"] => %-8u\n",
(uint64_t)bin->lower, (uint64_t)bin->upper, bin->count);
}

printf("=================================\n\n");
}

#ifdef CONFIG_JSONC
static void amzn_json_add_histogram(struct json_object *root,
struct amzn_latency_histogram *hist)
{
Expand Down Expand Up @@ -165,31 +169,32 @@ static void amzn_print_json_stats(struct amzn_latency_log_page *log)

json_free_object(root);
}
#endif /* CONFIG_JSONC */

static void amzn_print_normal_stats(struct amzn_latency_log_page *log)
{
printf("Total Ops:\n");
printf(" Read: %llu\n", log->total_read_ops);
printf(" Write: %llu\n", log->total_write_ops);
printf(" Read: %"PRIu64"\n", (uint64_t)log->total_read_ops);
printf(" Write: %"PRIu64"\n", (uint64_t)log->total_write_ops);
printf("Total Bytes:\n");
printf(" Read: %llu\n", log->total_read_bytes);
printf(" Write: %llu\n", log->total_write_bytes);
printf(" Read: %"PRIu64"\n", (uint64_t)log->total_read_bytes);
printf(" Write: %"PRIu64"\n", (uint64_t)log->total_write_bytes);
printf("Total Time (us):\n");
printf(" Read: %llu\n", log->total_read_time);
printf(" Write: %llu\n\n", log->total_write_time);
printf(" Read: %"PRIu64"\n", (uint64_t)log->total_read_time);
printf(" Write: %"PRIu64"\n\n", (uint64_t)log->total_write_time);

printf("EBS Volume Performance Exceeded (us):\n");
printf(" IOPS: %llu\n", log->ebs_volume_performance_exceeded_iops);
printf(" Throughput: %llu\n\n",
log->ebs_volume_performance_exceeded_tp);
printf(" IOPS: %"PRIu64"\n", (uint64_t)log->ebs_volume_performance_exceeded_iops);
printf(" Throughput: %"PRIu64"\n\n",
(uint64_t)log->ebs_volume_performance_exceeded_tp);
printf("EC2 Instance EBS Performance Exceeded (us):\n");
printf(" IOPS: %llu\n",
log->ec2_instance_ebs_performance_exceeded_iops);
printf(" Throughput: %llu\n\n",
log->ec2_instance_ebs_performance_exceeded_tp);
printf(" IOPS: %"PRIu64"\n",
(uint64_t)log->ec2_instance_ebs_performance_exceeded_iops);
printf(" Throughput: %"PRIu64"\n\n",
(uint64_t)log->ec2_instance_ebs_performance_exceeded_tp);

printf("Queue Length (point in time): %llu\n\n",
log->volume_queue_length);
printf("Queue Length (point in time): %"PRIu64"\n\n",
(uint64_t)log->volume_queue_length);

printf("Read IO Latency Histogram\n");
amzn_print_latency_histogram(&log->read_io_latency_histogram);
Expand All @@ -205,6 +210,8 @@ static int get_stats(int argc, char **argv, struct command *cmd,
struct nvme_dev *dev;
struct amzn_latency_log_page log = { 0 };
int rc;
nvme_print_flags_t flags;
int err;

struct config {
char *output_format;
Expand Down Expand Up @@ -253,9 +260,17 @@ static int get_stats(int argc, char **argv, struct command *cmd,
return -ENOTSUP;
}

err = validate_output_format(cfg.output_format, &flags);
if (err < 0) {
nvme_show_error("Invalid output format");
return err;
}

#ifdef CONFIG_JSONC
if (!strcmp(cfg.output_format, "json"))
amzn_print_json_stats(&log);
else
Copy link
Collaborator

@igaw igaw Dec 18, 2024

Choose a reason for hiding this comment

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

Okay, looks reasonable. One thing we could do is to extend validate_output_format to inform the user that there is no json output supported and use this function here. But that is just bonus :)

Copy link
Collaborator

@igaw igaw Dec 20, 2024

Choose a reason for hiding this comment

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

Thanks for adding validate_output_format. Instead parsing the cfg.output_format again you could use the flags variable directly

if (flag & JSON`)
  amzn_print_json_stats

BTW, I would prefer to introduce an empty amzn_prin-json_stats implementation in case json-c is missing. This avoid sprinkling the ifdefs around in the code. IMO, this helps maintaining the code base in the long run.

Copy link
Contributor Author

@ikegami-t ikegami-t Dec 20, 2024

Choose a reason for hiding this comment

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

Yes I agree with you so just fixed this by the PR #2636. Thank you.

#endif /* CONFIG_JSONC */
amzn_print_normal_stats(&log);

return 0;
Expand Down
2 changes: 1 addition & 1 deletion plugins/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

if json_c_dep.found()
sources += [
'plugins/amzn/amzn-nvme.c',
'plugins/dapustor/dapustor-nvme.c',
'plugins/dell/dell-nvme.c',
'plugins/fdp/fdp.c',
Expand All @@ -23,6 +22,7 @@ if json_c_dep.found()
endif

sources += [
'plugins/amzn/amzn-nvme.c',
'plugins/dera/dera-nvme.c',
'plugins/innogrit/innogrit-nvme.c',
'plugins/inspur/inspur-nvme.c',
Expand Down
Loading