enable plugins and detailed EBS volume stats for nvme-cli #269

bcressey · 2024-11-15T19:02:53Z

Issue number:
Fixes #274

Description of changes:
nvme-cli requires the json-c library for plugins. Package it, enable plugins, and apply the recent patch to the "amzn" plugin so that it can report detailed EBS volume stats.

Also fixes a bug I noticed in rdma-core, where the devel package didn't correctly depend on the base package.

Testing done:
Verified that the "amzn" plugin worked as expected.

# nvme amzn id-ctrl /dev/nvme0n1|grep ^bdev
bdev      : xvda

# nvme amzn stats /dev/nvme0n1
Total Ops:
  Read: 2899
  Write: 68
Total Bytes:
  Read: 214631424
  Write: 4775936
Total Time (us):
  Read: 14061141
  Write: 76947

EBS Volume Performance Exceeded (us):
  IOPS: 0
  Throughput: 0

EC2 Instance EBS Performance Exceeded (us):
  IOPS: 0
  Throughput: 0

Queue Length (point in time): 0

Also confirmed that the EBS volume links were created as expected:

# ls -latr /dev/disk/by-ebs-id/
lrwxrwxrwx.  1 root root  13 Nov 15 18:57 xvdb -> ../../nvme1n1
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvdb1 -> ../../nvme1n1p1
lrwxrwxrwx.  1 root root  13 Nov 15 18:57 xvda -> ../../nvme0n1
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda7 -> ../../nvme0n1p7
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda6 -> ../../nvme0n1p6
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda5 -> ../../nvme0n1p5
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda4 -> ../../nvme0n1p4
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda3 -> ../../nvme0n1p3
lrwxrwxrwx.  1 root root  16 Nov 15 18:57 xvda13 -> ../../nvme0n1p13
lrwxrwxrwx.  1 root root  16 Nov 15 18:57 xvda11 -> ../../nvme0n1p11
lrwxrwxrwx.  1 root root  16 Nov 15 18:57 xvda10 -> ../../nvme0n1p10
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda1 -> ../../nvme0n1p1
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda9 -> ../../nvme0n1p9
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda8 -> ../../nvme0n1p8
lrwxrwxrwx.  1 root root  15 Nov 15 18:57 xvda2 -> ../../nvme0n1p2
lrwxrwxrwx.  1 root root  16 Nov 15 18:57 xvda12 -> ../../nvme0n1p12

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Signed-off-by: Ben Cressey <bcressey@amazon.com>

packages/rdma-core/rdma-core.spec

packages/libjson-c/libjson-c.spec

arnaldo2792 · 2024-11-19T17:37:19Z

packages/libjson-c/libjson-c.spec

+  -DDISABLE_WERROR:BOOL=OFF \
+  -DENABLE_RDRAND:BOOL=ON \
+  -DENABLE_THREADING:BOOL=ON \
+  -DCMAKE_INSTALL_PREFIX:PATH=%{_cross_prefix} \


Also not needed:

/~https://github.com/bottlerocket-os/bottlerocket-sdk/blob/8ef6f479f9ea4d2f0f8e0552580cda07282ce706/macros/shared#L163

Also not needed:

If I don't set this, then I don't get the right behavior; cmake --install sends the files to /usr/local instead:

#14 5.950 + DESTDIR=/home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64 #14 5.950 + cmake --install . #14 5.960 -- Install configuration: "Release" #14 5.961 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/lib/libjson-c.so.5.4.0 #14 5.961 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/lib/libjson-c.so.5 #14 5.961 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/lib/libjson-c.so #14 5.962 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/lib/cmake/json-c/json-c-targets.cmake #14 5.962 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/lib/cmake/json-c/json-c-targets-release.cmake #14 5.962 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/lib/cmake/json-c/json-c-config.cmake #14 5.963 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/lib/pkgconfig/json-c.pc #14 5.963 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_config.h #14 5.963 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json.h #14 5.963 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/arraylist.h #14 5.964 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/debug.h #14 5.964 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_c_version.h #14 5.964 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_inttypes.h #14 5.964 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_object.h #14 5.964 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_object_iterator.h #14 5.965 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_tokener.h #14 5.965 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_types.h #14 5.965 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_util.h #14 5.965 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/json_visit.h #14 5.965 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/linkhash.h #14 5.966 -- Installing: /home/builder/rpmbuild/BUILDROOT/bottlerocket-libjson-c-0.18-20240915.1731694836.c940f6ab.br1.aarch64/usr/local/include/json-c/printbuf.h

🫠 , why cmake? I wonder if there is a bug in cmake and it ignores CMAKE_INSTALL_PREFIX when set through the toolchain file?

arnaldo2792 · 2024-11-19T17:40:19Z

packages/libjson-c/libjson-c.spec

+  -DCMAKE_INSTALL_PREFIX:PATH=%{_cross_prefix} \
+  -G Ninja
+
+cmake --build .


Are these two steps (_cross_cmake and cmake build) required in separate lines? If _cross_cmake is used to generate the actual Make files, why not move that to %prep?

Are these two steps (_cross_cmake and cmake build) required in separate lines?

Even if they're not, I prefer them to be separate for symmetry with autotools (configure then make), meson (meson setup then meson build), etc.

If _cross_cmake is used to generate the actual Make files, why not move that to %prep?

%prep is only meant to prepare the source tree for building. It's not meant to encompass build steps, and by convention the entry point to the build is that first configure / meson setup / cmake invocation. In the configure case this often involves building and running a lot of test programs; it isn't a "pure" operation on files in the way that %prep ideally is.

With sufficient motivation you could run build logic in the %prep stage or patch sources in the %build stage (and our grub and glibc packages kind of do that, respectively). In extreme cases (like our libgcc) the entire build can happen elsewhere. But it's not the convention and it makes the spec "weird" when you do that.

Signed-off-by: Ben Cressey <bcressey@amazon.com>

bcressey added 3 commits November 15, 2024 18:20

rdma-core: require parent package for devel

c688b32

Signed-off-by: Ben Cressey <bcressey@amazon.com>

libnvme: update to 1.11

212caa8

Signed-off-by: Ben Cressey <bcressey@amazon.com>

nvme-cli: update to 2.11

295c2d3

Signed-off-by: Ben Cressey <bcressey@amazon.com>

bcressey requested review from arnaldo2792 and ytsssun November 15, 2024 19:02

ytsssun approved these changes Nov 15, 2024

View reviewed changes

packages/rdma-core/rdma-core.spec Show resolved Hide resolved

arnaldo2792 reviewed Nov 19, 2024

View reviewed changes

bcressey added 3 commits November 20, 2024 00:53

packages: add libjson-c

cd739ab

Signed-off-by: Ben Cressey <bcressey@amazon.com>

nvme-cli: build with libjson-c for plugins

2a2f603

Signed-off-by: Ben Cressey <bcressey@amazon.com>

nvme-cli: add patch for EBS stats

db75545

Signed-off-by: Ben Cressey <bcressey@amazon.com>

bcressey force-pushed the nvme-cli-amzn branch from c940f6a to db75545 Compare November 20, 2024 00:54

arnaldo2792 approved these changes Nov 20, 2024

View reviewed changes

bcressey merged commit f2b8e9a into bottlerocket-os:develop Nov 25, 2024
2 checks passed

bcressey deleted the nvme-cli-amzn branch November 25, 2024 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable plugins and detailed EBS volume stats for nvme-cli #269

enable plugins and detailed EBS volume stats for nvme-cli #269

bcressey commented Nov 15, 2024 •

edited

Loading

arnaldo2792 Nov 19, 2024

bcressey Nov 20, 2024

arnaldo2792 Nov 20, 2024

arnaldo2792 Nov 19, 2024

bcressey Nov 20, 2024

enable plugins and detailed EBS volume stats for nvme-cli #269

enable plugins and detailed EBS volume stats for nvme-cli #269

Conversation

bcressey commented Nov 15, 2024 • edited Loading

arnaldo2792 Nov 19, 2024

Choose a reason for hiding this comment

bcressey Nov 20, 2024

Choose a reason for hiding this comment

arnaldo2792 Nov 20, 2024

Choose a reason for hiding this comment

arnaldo2792 Nov 19, 2024

Choose a reason for hiding this comment

bcressey Nov 20, 2024

Choose a reason for hiding this comment

bcressey commented Nov 15, 2024 •

edited

Loading