RE: [PATCH for-4.14] xen/hypfs: fix loglvl parameter setting

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 11 June 2020 10:20
> To: Julien Grall <julien.grall.oss@xxxxxxxxx>; Jürgen Groß <jgross@xxxxxxxx>; 
> Paul Durrant
> <paul@xxxxxxx>
> Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Andrew Cooper 
> <andrew.cooper3@xxxxxxxxxx>; George
> Dunlap <george.dunlap@xxxxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; 
> Jan Beulich
> <jbeulich@xxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Wei Liu 
> <wl@xxxxxxx>
> Subject: Re: [PATCH for-4.14] xen/hypfs: fix loglvl parameter setting
> 
> 
> 
> On 10/06/2020 22:47, Julien Grall wrote:
> > On Wed, 10 Jun 2020 at 19:49, Jürgen Groß <jgross@xxxxxxxx> wrote:
> >>
> >> On 10.06.20 19:55, Julien Grall wrote:
> >>> Hi Juergen,
> >>>
> >>> On 09/06/2020 16:45, Juergen Gross wrote:
> >>>> Writing the runtime parameters loglvl or guest_loglvl omits setting the
> >>>> new length of the resulting parameter value.
> >>>>
> >>>> Reported-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> >>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> >>>
> >>> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
> >>>
> >>> Although one unrelated comment below.
> >>>
> >>>> ---
> >>>>    xen/drivers/char/console.c | 19 +++++++++++++++----
> >>>>    1 file changed, 15 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> >>>> index 56e24821b2..861ad53a8f 100644
> >>>> --- a/xen/drivers/char/console.c
> >>>> +++ b/xen/drivers/char/console.c
> >>>> @@ -241,14 +241,25 @@ static int _parse_loglvl(const char *s, int
> >>>> *lower, int *upper, char *val)
> >>>>    static int parse_loglvl(const char *s)
> >>>>    {
> >>>> -    return _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh,
> >>>> -                         xenlog_val);
> >>>> +    int ret;
> >>>> +
> >>>> +    ret = _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh,
> >>>> +                        xenlog_val);
> >>>> +    custom_runtime_set_var(param_2_parfs(parse_loglvl), xenlog_val);
> >>>
> >>> Mixing function and variable name is pretty confusing. It took me
> >>> sometimes to go through the macro maze to figure out what's happening.
> >>>
> >>> It might be worth thinking to document more the custom_runtime_* 
> >>> interface.
> >>
> >> I have already some streamlining ideas for 4.15.
> >
> > Cool! I will commit it tomorrow morning.
> 
> Actually I am missing a Released-acked-by from Paul on this patch.
> 

Release-acked-by: Paul Durrant <paul@xxxxxxx>

> Cheers,
> 
> --
> Julien Grall