mbox series

[ndctl,v3,0/6] cxl/monitor and ndctl/monitor fixes

Message ID 20230531021936.7366-1-lizhijian@fujitsu.com (mailing list archive)
Headers show
Series cxl/monitor and ndctl/monitor fixes | expand

Message

Zhijian Li (Fujitsu) May 31, 2023, 2:19 a.m. UTC
V3:
- update comit log of patch3 and patch6 per Dave's comments.

V2:
- exchange order of previous patch1 and patch2
- add reviewed tag in patch5
- commit log improvements

It mainly fix monitor not working when log file is specified. For
example
$ cxl monitor -l ./cxl-monitor.log
It seems that someone missed something at the begining.

Furture, it compares the filename with reserved word more accurately

patch1-2: It re-enables logfile(including default_log) functionality
and simplify the sanity check in the combination relative path file
and daemon mode.

patch3 and patch6 change strncmp to strcmp to compare the acurrate
reserved words.

Li Zhijian (6):
  cxl/monitor: Enable default_log and refactor sanity check
  cxl/monitor: replace monitor.log_file with monitor.ctx.log_file
  cxl/monitor: use strcmp to compare the reserved word
  cxl/monitor: always log started message
  Documentation/cxl/cxl-monitor.txt: Fix inaccurate description
  ndctl/monitor: use strcmp to compare the reserved word

 Documentation/cxl/cxl-monitor.txt |  3 +--
 cxl/monitor.c                     | 45 ++++++++++++++++---------------
 ndctl/monitor.c                   |  4 +--
 3 files changed, 26 insertions(+), 26 deletions(-)

Comments

Zhijian Li (Fujitsu) June 27, 2023, 10:17 a.m. UTC | #1
kindly ping

It has been almost a month, and it's doing some bugfix. The progress is a little bit slow anyway :)



On 31/05/2023 10:19, Li Zhijian wrote:
> V3:
> - update comit log of patch3 and patch6 per Dave's comments.
> 
> V2:
> - exchange order of previous patch1 and patch2
> - add reviewed tag in patch5
> - commit log improvements
> 
> It mainly fix monitor not working when log file is specified. For
> example
> $ cxl monitor -l ./cxl-monitor.log
> It seems that someone missed something at the begining.
> 
> Future, it compares the filename with reserved word more accurately
> 
> patch1-2: It re-enables logfile(including default_log) functionality
> and simplify the sanity check in the combination relative path file
> and daemon mode.
> 
> patch3 and patch6 change strncmp to strcmp to compare the acurrate
> reserved words.
> 
> Li Zhijian (6):
>    cxl/monitor: Enable default_log and refactor sanity check
>    cxl/monitor: replace monitor.log_file with monitor.ctx.log_file
>    cxl/monitor: use strcmp to compare the reserved word
>    cxl/monitor: always log started message
>    Documentation/cxl/cxl-monitor.txt: Fix inaccurate description
>    ndctl/monitor: use strcmp to compare the reserved word
> 
>   Documentation/cxl/cxl-monitor.txt |  3 +--
>   cxl/monitor.c                     | 45 ++++++++++++++++---------------
>   ndctl/monitor.c                   |  4 +--
>   3 files changed, 26 insertions(+), 26 deletions(-)
>
Verma, Vishal L July 5, 2023, 9:21 p.m. UTC | #2
On Tue, 2023-06-27 at 10:17 +0000, Zhijian Li (Fujitsu) wrote:
> kindly ping
> 
> It has been almost a month, and it's doing some bugfix. The progress
> is a little bit slow anyway :)
> 
Hi Li,

Sorry for the delay - the patches are mostly fine with the exception of
a few things I pointed out. Once those are fixed I'll pick these up for
the next release.
Alison Schofield July 5, 2023, 11:53 p.m. UTC | #3
On Wed, May 31, 2023 at 10:19:30AM +0800, Li Zhijian wrote:
> V3:
> - update comit log of patch3 and patch6 per Dave's comments.
> 
> V2:
> - exchange order of previous patch1 and patch2
> - add reviewed tag in patch5
> - commit log improvements
> 
> It mainly fix monitor not working when log file is specified. For
> example
> $ cxl monitor -l ./cxl-monitor.log
> It seems that someone missed something at the begining.
> 
> Furture, it compares the filename with reserved word more accurately
> 
> patch1-2: It re-enables logfile(including default_log) functionality
> and simplify the sanity check in the combination relative path file
> and daemon mode.
> 
> patch3 and patch6 change strncmp to strcmp to compare the acurrate
> reserved words.
> 
> Li Zhijian (6):
>   cxl/monitor: Enable default_log and refactor sanity check
>   cxl/monitor: replace monitor.log_file with monitor.ctx.log_file
>   cxl/monitor: use strcmp to compare the reserved word
>   cxl/monitor: always log started message
>   Documentation/cxl/cxl-monitor.txt: Fix inaccurate description
>   ndctl/monitor: use strcmp to compare the reserved word

Hi,

Patches 3 & 6 make the same change in 2 different files, with
near identical commit logs. Please consider combining them into
one patch, perhaps something like:

ndctl: use strcmp for reserved word in monitor commands

Thanks,
Alison

> 
>  Documentation/cxl/cxl-monitor.txt |  3 +--
>  cxl/monitor.c                     | 45 ++++++++++++++++---------------
>  ndctl/monitor.c                   |  4 +--
>  3 files changed, 26 insertions(+), 26 deletions(-)
> 
> -- 
> 2.29.2
>
Zhijian Li (Fujitsu) July 10, 2023, 1:06 p.m. UTC | #4
on 7/6/2023 7:53 AM, Alison Schofield wrote:
> On Wed, May 31, 2023 at 10:19:30AM +0800, Li Zhijian wrote:
>> V3:
>> - update comit log of patch3 and patch6 per Dave's comments.
>>
>> V2:
>> - exchange order of previous patch1 and patch2
>> - add reviewed tag in patch5
>> - commit log improvements
>>
>> It mainly fix monitor not working when log file is specified. For
>> example
>> $ cxl monitor -l ./cxl-monitor.log
>> It seems that someone missed something at the begining.
>>
>> Furture, it compares the filename with reserved word more accurately
>>
>> patch1-2: It re-enables logfile(including default_log) functionality
>> and simplify the sanity check in the combination relative path file
>> and daemon mode.
>>
>> patch3 and patch6 change strncmp to strcmp to compare the acurrate
>> reserved words.
>>
>> Li Zhijian (6):
>>    cxl/monitor: Enable default_log and refactor sanity check
>>    cxl/monitor: replace monitor.log_file with monitor.ctx.log_file
>>    cxl/monitor: use strcmp to compare the reserved word
>>    cxl/monitor: always log started message
>>    Documentation/cxl/cxl-monitor.txt: Fix inaccurate description
>>    ndctl/monitor: use strcmp to compare the reserved word
> Hi,
>
> Patches 3 & 6 make the same change in 2 different files, with
> near identical commit logs. Please consider combining them into
> one patch, perhaps something like:
>
> ndctl: use strcmp for reserved word in monitor commands

Okay, it sounds good to me :)

Thanks
Zhijian

>
> Thanks,
> Alison
>
>>   Documentation/cxl/cxl-monitor.txt |  3 +--
>>   cxl/monitor.c                     | 45 ++++++++++++++++---------------
>>   ndctl/monitor.c                   |  4 +--
>>   3 files changed, 26 insertions(+), 26 deletions(-)
>>
>> -- 
>> 2.29.2
>>