Erik recommended several changes to the mdev series that was just merged and
suggested to address them in a follow-up patch series. They're mostly related
to simplifying the testing, and shouldn't actually change any behavior (aside
from switching to using the long commandline options when executing mdevctl).

Changes in v2:
 - rebase to lastest git master

Erik Skultety (4):
  nodedev: driver: Swap virMdevctlStart and virMdevctlCreate
  nodedev: driver: Introduce internal mdevctl commands enum
  nodedev: driver: Create a generic mdevctl command translator
  tests: nodedev: Make the mdevctl test function and helper generic

Jonathon Jongsma (8):
  nodedev: don't log error in nodeDeviceFindAddressByName()
  nodedev: avoid use of VIR_ERR_NO_* errors internally
  tests: nodedev: switch all test macros to accept a filename
  nodedev: Switch to using long options for mdevctl
  nodedev: Remove GetMdevctl*Command() wrappers
  tests: nodedev: simplify test macros
  tests: nodedev: remove unused variable
  tests: nodedev: remove unnecessary cleanup label

 src/node_device/node_device_driver.c          | 181 +++++++--------
 src/node_device/node_device_driver.h          |  43 ++--
 ...19_36ea_4111_8f0a_8c9a70e21366-create.argv |   5 +
 ...9_36ea_4111_8f0a_8c9a70e21366-create.json} |   0
 ...19_36ea_4111_8f0a_8c9a70e21366-define.argv |   4 +-
 ...019_36ea_4111_8f0a_8c9a70e21366-start.argv |   2 -
 ...d019_36ea_4111_8f0a_8c9a70e21366-stop.argv |   3 +
 ..._36ea_4111_8f0a_8c9a70e21366-undefine.argv |   3 +
 ...39_495e_4243_ad9f_beb3f14c23d9-create.argv |   4 +
 ...9_495e_4243_ad9f_beb3f14c23d9-create.json} |   0
 ...39_495e_4243_ad9f_beb3f14c23d9-define.argv |   4 +-
 ...d39_495e_4243_ad9f_beb3f14c23d9-start.argv |   4 -
 ...16_1ca8_49ac_b176_871d16c13076-create.argv |   4 +
 ...6_1ca8_49ac_b176_871d16c13076-create.json} |   0
 ...16_1ca8_49ac_b176_871d16c13076-define.argv |   4 +-
 ...916_1ca8_49ac_b176_871d16c13076-start.argv |   4 -
 tests/nodedevmdevctldata/mdevctl-create.argv  |   3 -
 tests/nodedevmdevctldata/mdevctl-stop.argv    |   3 -
 .../nodedevmdevctldata/mdevctl-undefine.argv  |   3 -
 tests/nodedevmdevctltest.c                    | 211 ++++++------------
 20 files changed, 197 insertions(+), 288 deletions(-)
 create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9=
a70e21366-create.argv
 rename tests/nodedevmdevctldata/{mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-s=
tart.json =3D> mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-create.json} (100%)
 create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9=
a70e21366-stop.argv
 create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9=
a70e21366-undefine.argv
 create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb=
3f14c23d9-create.argv
 rename tests/nodedevmdevctldata/{mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-s=
tart.json =3D> mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-create.json} (100%)
 delete mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb=
3f14c23d9-start.argv
 create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871=
d16c13076-create.argv
 rename tests/nodedevmdevctldata/{mdev_fedc4916_1ca8_49ac_b176_871d16c13076-s=
tart.json =3D> mdev_fedc4916_1ca8_49ac_b176_871d16c13076-create.json} (100%)
 delete mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871=
d16c13076-start.argv
 delete mode 100644 tests/nodedevmdevctldata/mdevctl-create.argv
 delete mode 100644 tests/nodedevmdevctldata/mdevctl-stop.argv
 delete mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv

--=20
2.26.3

On Tue, Apr 13, 2021 at 03:39:36PM -0500, Jonathon Jongsma wrote:
> Erik recommended several changes to the mdev series that was just merged and
> suggested to address them in a follow-up patch series. They're mostly related
> to simplifying the testing, and shouldn't actually change any behavior (aside
> from switching to using the long commandline options when executing mdevctl).
> 
> Changes in v2:
>  - rebase to lastest git master
> 
> Erik Skultety (4):
>   nodedev: driver: Swap virMdevctlStart and virMdevctlCreate
>   nodedev: driver: Introduce internal mdevctl commands enum
>   nodedev: driver: Create a generic mdevctl command translator
>   tests: nodedev: Make the mdevctl test function and helper generic
> 
> Jonathon Jongsma (8):
>   nodedev: don't log error in nodeDeviceFindAddressByName()
>   nodedev: avoid use of VIR_ERR_NO_* errors internally
>   tests: nodedev: switch all test macros to accept a filename
>   nodedev: Switch to using long options for mdevctl
>   nodedev: Remove GetMdevctl*Command() wrappers
>   tests: nodedev: simplify test macros
>   tests: nodedev: remove unused variable
>   tests: nodedev: remove unnecessary cleanup label

The changes look good to me and I'm tempted to put my R-b even on the rest of
the patches, but since I have a fair share on the patches that I didn't comment
on, it suddenly feels like circumventing the rules (I know I suggested you
go ahead and submit the patches). Let's give it a few days and hopefully
someone else gives you a second opinion especially on the patches that bear my
signoff.

Regards,
Erik

On 4/14/21 6:49 AM, Erik Skultety wrote:
> On Tue, Apr 13, 2021 at 03:39:36PM -0500, Jonathon Jongsma wrote:
>> Erik recommended several changes to the mdev series that was just merged and
>> suggested to address them in a follow-up patch series. They're mostly related
>> to simplifying the testing, and shouldn't actually change any behavior (aside
>> from switching to using the long commandline options when executing mdevctl).
>>
>> Changes in v2:
>>   - rebase to lastest git master
>>
>> Erik Skultety (4):
>>    nodedev: driver: Swap virMdevctlStart and virMdevctlCreate
>>    nodedev: driver: Introduce internal mdevctl commands enum
>>    nodedev: driver: Create a generic mdevctl command translator
>>    tests: nodedev: Make the mdevctl test function and helper generic
>>
>> Jonathon Jongsma (8):
>>    nodedev: don't log error in nodeDeviceFindAddressByName()
>>    nodedev: avoid use of VIR_ERR_NO_* errors internally
>>    tests: nodedev: switch all test macros to accept a filename
>>    nodedev: Switch to using long options for mdevctl
>>    nodedev: Remove GetMdevctl*Command() wrappers
>>    tests: nodedev: simplify test macros
>>    tests: nodedev: remove unused variable
>>    tests: nodedev: remove unnecessary cleanup label
> 
> The changes look good to me and I'm tempted to put my R-b even on the rest of
> the patches, but since I have a fair share on the patches that I didn't comment
> on, it suddenly feels like circumventing the rules (I know I suggested you
> go ahead and submit the patches). Let's give it a few days and hopefully
> someone else gives you a second opinion especially on the patches that bear my
> signoff.

Yeah, sorry, I went through and picked out the easy ones at the end of 
the day, meaning to go back later and get the rest (unless someone else 
did it), but then forgot to do that. I'll try to concentrate long enough 
to get through them, or if anyone else feels inspired they can jump in 
in front of me.