linux-kernel - Re: [PATCH blktests v3 13/13] nvme: Introduce nvmet_target

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <cb468a17-c653-b257-8f5d-bf5981c22de5@grimberg.me>
Date:   Thu, 17 Aug 2023 11:33:36 +0300
From:   Sagi Grimberg <sagi@...mberg.me>
To:     Daniel Wagner <dwagner@...e.de>
Cc:     linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-block@...r.kernel.org, Chaitanya Kulkarni <kch@...dia.com>,
        Shin'ichiro Kawasaki <shinichiro.kawasaki@....com>,
        Max Gurtovoy <mgurtovoy@...dia.com>,
        Hannes Reinecke <hare@...e.de>,
        James Smart <jsmart2021@...il.com>,
        Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH blktests v3 13/13] nvme: Introduce
 nvmet_target_{setup/cleanup} common code


>> It is very very strange that _setup returns a port
>> which is passed to _cleanup...
> 
> This is the only information the _cleanup helper needs and that is why
> it survived the refactoring so far.

Doesn't change the fact that it is awkward.

>> I think that _cleanup should simply remove all
>> ports, and that setup should not return a port
>> to begin with.
> 
> This assumes that blktests is the single user and can blindly remove
> everything. I would like to play nice here and only cleanup resources
> blktests actually allocates.

I suggest that the test records resources that it is using
and destroy them in the cleanup (IIRC Shinichiro did something
similar with the modules in some other place).

>> If someone needs the actual port number, then it
>> should either not use this _setup helper or
>> query it somehow.
> 
> I try to figure out how to implement such a query helper then.

That would probably be needed when a test wants to cherry-pick
which stuff it is destroying. But on the other hand, in this
case it also doesn't make sense that this test will be using
the default basic setup function. So I don't think it is
very much needed.