nvme-cli & json vs blktests

nvme-cli & json vs blktests

[Christoph Hellwig]

Hi all,

I recently did a long overdue update of blktests on my main test
machine, and a large part of the nvme tests started failing.  I
eventually bisected this down to:

commit 444f0df9f0c537335f59c1721240212852656e5b
Author: Hannes Reinecke <hare@suse.de>
Date:   Wed Oct 30 15:51:45 2024 +0900

    nvme: check for controller device in _nvme_connect_subsys()


which adds a requirements for json output from nvme connect, and
just silently fails if not present.

Now after figuring this out I went to rebuild my nvme-cli after installing
the json-c development package.

... and nothing changes.

It looks like the nvme-cli build system unfortunately doesn't pick up
changes in available dependencies even after a make clean.  It turns
out a make purge eventually fixed it, but that's a little unexpected.

Can we get proper feature check for json support in blktests and maybe
make the nvme-cli build a little less arcane?

Re: nvme-cli & json vs blktests

[Daniel Wagner]

On Mon, May 19, 2025 at 07:49:05AM +0200, Christoph Hellwig wrote:
> make the nvme-cli build a little less arcane?

There is no magic build into the meson/ninja build system. meson creates
the ninja build instruction which are fully hardcoded, thus it wont pick
up any newly installed libraries on automatically. Running meson with
'setup --reconfigure' is a design choice of this build system.

I don't really see a problem with this approach.

Re: nvme-cli & json vs blktests

[Christoph Hellwig]

On Mon, May 19, 2025 at 11:51:47AM +0200, Daniel Wagner wrote:
> On Mon, May 19, 2025 at 07:49:05AM +0200, Christoph Hellwig wrote:
> > make the nvme-cli build a little less arcane?
> 
> There is no magic build into the meson/ninja build system. meson creates
> the ninja build instruction which are fully hardcoded, thus it wont pick
> up any newly installed libraries on automatically. Running meson with
> 'setup --reconfigure' is a design choice of this build system.
> 
> I don't really see a problem with this approach.

The fact that a make clean is not clean enough to find newly installed
dependencies is very unusual.

Re: nvme-cli & json vs blktests

[Daniel Wagner]

On Mon, May 19, 2025 at 11:56:42AM +0200, Christoph Hellwig wrote:
> On Mon, May 19, 2025 at 11:51:47AM +0200, Daniel Wagner wrote:
> > On Mon, May 19, 2025 at 07:49:05AM +0200, Christoph Hellwig wrote:
> > > make the nvme-cli build a little less arcane?
> > 
> > There is no magic build into the meson/ninja build system. meson creates
> > the ninja build instruction which are fully hardcoded, thus it wont pick
> > up any newly installed libraries on automatically. Running meson with
> > 'setup --reconfigure' is a design choice of this build system.
> > 
> > I don't really see a problem with this approach.
> 
> The fact that a make clean is not clean enough to find newly installed
> dependencies is very unusual.

Okay, so the Makefile is not really working in your workflow as expected. I see
there are two targets 'clean' and 'purge'. I suppose it would make sense
to model the 'clean' target more what autotools  & co are doing. Really
wiping everything away. Let me fix this.

Re: nvme-cli & json vs blktests

[Chaitanya Kulkarni]

On 5/19/25 03:04, Daniel Wagner wrote:
> On Mon, May 19, 2025 at 11:56:42AM +0200, Christoph Hellwig wrote:
>> On Mon, May 19, 2025 at 11:51:47AM +0200, Daniel Wagner wrote:
>>> On Mon, May 19, 2025 at 07:49:05AM +0200, Christoph Hellwig wrote:
>>>> make the nvme-cli build a little less arcane?
>>> There is no magic build into the meson/ninja build system. meson creates
>>> the ninja build instruction which are fully hardcoded, thus it wont pick
>>> up any newly installed libraries on automatically. Running meson with
>>> 'setup --reconfigure' is a design choice of this build system.
>>>
>>> I don't really see a problem with this approach.
>> The fact that a make clean is not clean enough to find newly installed
>> dependencies is very unusual.
> Okay, so the Makefile is not really working in your workflow as expected. I see
> there are two targets 'clean' and 'purge'. I suppose it would make sense
> to model the 'clean' target more what autotools  & co are doing. Really
> wiping everything away. Let me fix this.

yes please hope should fix the issue that I'm also seeing from last week.

-ck

Re: nvme-cli & json vs blktests

[Daniel Wagner]

On Mon, May 19, 2025 at 11:31:36PM +0000, Chaitanya Kulkarni wrote:
> > Okay, so the Makefile is not really working in your workflow as expected. I see
> > there are two targets 'clean' and 'purge'. I suppose it would make sense
> > to model the 'clean' target more what autotools  & co are doing. Really
> > wiping everything away. Let me fix this.
> 
> yes please hope should fix the issue that I'm also seeing from last week.

I've updated the Makefile. The 'clean' target is now doing a proper
cleanup and deletes all the build artifacts and configuration files.

Re: nvme-cli & json vs blktests

[Christoph Hellwig]

On Tue, May 20, 2025 at 09:18:19AM +0200, Daniel Wagner wrote:
> I've updated the Makefile. The 'clean' target is now doing a proper
> cleanup and deletes all the build artifacts and configuration files.

Thanks!

Re: nvme-cli & json vs blktests

[Shinichiro Kawasaki]

On May 19, 2025 / 07:49, Christoph Hellwig wrote:
> Hi all,
> 
> I recently did a long overdue update of blktests on my main test
> machine, and a large part of the nvme tests started failing.  I
> eventually bisected this down to:
> 
> commit 444f0df9f0c537335f59c1721240212852656e5b
> Author: Hannes Reinecke <hare@suse.de>
> Date:   Wed Oct 30 15:51:45 2024 +0900
> 
>     nvme: check for controller device in _nvme_connect_subsys()
> 
> 
> which adds a requirements for json output from nvme connect, and
> just silently fails if not present.

I uninstalled json-c-devel package from my Fedora system, and built nvme-cli.
Then confirmed the slient fail. That is a painful pit-fall.

...

> 
> Can we get proper feature check for json support in blktests and maybe
> make the nvme-cli build a little less arcane?

As to blktests, I quickly created a patch below, and confirmed it informs that
nvme-cli is missing json output support. Will do some more testing and post as
a formal patch.

diff --git a/tests/nvme/rc b/tests/nvme/rc
index d86dc13..f5880ed 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -55,7 +55,7 @@ _set_nvmet_blkdev_type() {
 }
 
 _nvme_requires() {
-	_have_program nvme
+	_have_nvme_cli_with_json_support
 	_require_nvme_test_img_size 4m
 	case ${nvme_trtype} in
 	loop)
@@ -626,6 +626,14 @@ _have_systemd_tlshd_service() {
 	fi
 }
 
+_have_nvme_cli_with_json_support() {
+	_have_program nvme || return $?
+
+	if ! nvme list --output-format=json &> /dev/null; then
+		SKIP_REASONS+=("nvme-cli does not support json output format")
+	fi
+}
+
 _have_libnvme_ver() {
 	local ver