mbox series

[blktests,v3,00/13] Switch to allowed_host

Message ID 20230811093614.28005-1-dwagner@suse.de
Headers
Series Switch to allowed_host |

Message

Daniel Wagner Aug. 11, 2023, 9:36 a.m. UTC 
  Addressed the comments from v2. I also added cleanup code to _nvmet_cleanup() to
make sure we do not leak resources when something goes wrong. I run into this
while testing and all tests after the first failure failed then.

changes:
v3:
 - added new patch: "nvme/043: Use hostnqn to generate DHCAP key"
 - removed unused variable in "nvme/rc: Add helper for adding/removing to allow list"
 - added cleanup code to _nvmet_cleanup().

v2:
 - updated commit messages
 - moved the removal of subsys_name to the right patch
 - added _nvmet_target_{setup|cleanup} helpers
   this addresses also the 'appears unused' warning by ShellCheck
 - https://lore.kernel.org/linux-nvme/20230810111317.25273-1-dwagner@suse.de/

v1:
 - initial version
   https://lore.kernel.org/linux-nvme/20230726124644.12619-1-dwagner@suse.de/


*** BLURB HERE ***

Daniel Wagner (13):
  nvme/{003,004,005,013,046,049}: Group all variables declarations
  nvme: Reorganize test preamble code section
  nvme/043: Use hostnqn to generate DHCAP key
  nvme/rc: Add common subsystem nqn define
  nvme: Use def_subsysnqn variable instead local variable
  nvme/{041,042,043,044,045,048}: Remove local variable hostnqn and
    hostid
  nvme/rc: Add common file_path name define
  nvme: Use def_file_path variable instead local variable
  nvme/rc: Add common def_subsys_uuid define
  nvme: Use def_subsys_uuid variable
  nvme/rc: Add helper for adding/removing to allow list
  nvme: Add explicitly host to allow_host list
  nvme: Introduce nvmet_target_{setup/cleanup} common code

 tests/nvme/003 | 12 ++-----
 tests/nvme/004 | 23 ++++--------
 tests/nvme/005 | 22 +++---------
 tests/nvme/006 | 21 ++---------
 tests/nvme/007 | 19 ++--------
 tests/nvme/008 | 26 +++-----------
 tests/nvme/009 | 21 +++--------
 tests/nvme/010 | 26 +++-----------
 tests/nvme/011 | 22 +++---------
 tests/nvme/012 | 26 +++-----------
 tests/nvme/013 | 22 +++---------
 tests/nvme/014 | 26 +++-----------
 tests/nvme/015 | 21 +++--------
 tests/nvme/016 | 17 +++++----
 tests/nvme/017 | 26 ++++++--------
 tests/nvme/018 | 21 +++--------
 tests/nvme/019 | 26 +++-----------
 tests/nvme/020 | 21 +++--------
 tests/nvme/021 | 21 +++--------
 tests/nvme/022 | 21 +++--------
 tests/nvme/023 | 26 +++-----------
 tests/nvme/024 | 21 +++--------
 tests/nvme/025 | 21 +++--------
 tests/nvme/026 | 21 +++--------
 tests/nvme/027 | 20 +++--------
 tests/nvme/028 | 20 +++--------
 tests/nvme/029 | 26 +++-----------
 tests/nvme/030 | 19 +++++-----
 tests/nvme/031 | 14 ++++----
 tests/nvme/033 |  9 ++---
 tests/nvme/034 |  9 ++---
 tests/nvme/035 |  9 ++---
 tests/nvme/036 |  9 ++---
 tests/nvme/037 |  8 ++---
 tests/nvme/038 |  6 ++--
 tests/nvme/039 |  4 +--
 tests/nvme/040 | 28 +++++----------
 tests/nvme/041 | 49 ++++++++-----------------
 tests/nvme/042 | 55 ++++++++++------------------
 tests/nvme/043 | 52 +++++++++------------------
 tests/nvme/044 | 71 ++++++++++++++----------------------
 tests/nvme/045 | 62 ++++++++++++--------------------
 tests/nvme/046 |  1 +
 tests/nvme/047 | 30 ++++------------
 tests/nvme/048 | 42 +++++++---------------
 tests/nvme/049 |  1 +
 tests/nvme/rc  | 97 +++++++++++++++++++++++++++++++++++++++++++++++---
 47 files changed, 404 insertions(+), 766 deletions(-)

Comments

Sagi Grimberg Aug. 13, 2023, 2:59 p.m. UTC | #1 
> Addressed the comments from v2. I also added cleanup code to _nvmet_cleanup() to
> make sure we do not leak resources when something goes wrong. I run into this
> while testing and all tests after the first failure failed then.

The name of the patch series suggest that it switches to allowed_hosts
where it does that in 2 patches 11+12 out of 13 patches. The rest are
just bug fixes and unifications. It's true that any series will include
fixes, cleanups and prep patches, but this is too far :)

I'll let Shinichiro accept as he wish though.

The cleanups look fine to me.
Shinichiro Kawasaki Aug. 16, 2023, 12:18 p.m. UTC | #2 
On Aug 16, 2023 / 11:31, Daniel Wagner wrote:
> On Sun, Aug 13, 2023 at 05:59:53PM +0300, Sagi Grimberg wrote:
> > 
> > > Addressed the comments from v2. I also added cleanup code to _nvmet_cleanup() to
> > > make sure we do not leak resources when something goes wrong. I run into this
> > > while testing and all tests after the first failure failed then.
> > 
> > The name of the patch series suggest that it switches to allowed_hosts
> > where it does that in 2 patches 11+12 out of 13 patches. The rest are
> > just bug fixes and unifications. It's true that any series will include
> > fixes, cleanups and prep patches, but this is too far :)
> 
> I see your point. The whole series started smaller, but just grew over
> time. I suppose if we agree with the general direction we could just get
> the first part done (bug fixes and refactoring).
> 
> > I'll let Shinichiro accept as he wish though.
> 
> I am fine either way, just let me know what you prefer.

I think the 13th patch worth spending some more time, and other 12 patches
from 1 to 12 have consensus. If there is no other voice, I will apply the
patches from 1 to 12 tomorrow.