Re: [Xen-devel] [PATCH v2 3/3] xl: new "loglvl" command

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.03.16 at 17:20, <wei.liu2@xxxxxxxxxx> wrote:
> On Fri, Mar 04, 2016 at 09:48:16AM -0700, Jan Beulich wrote:
>> This is pretty simplistic for now, but I'd rather have someone better
>> friends with the tools improve it (if desired).
>> 
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> 
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -5958,6 +5958,26 @@ int libxl_send_debug_keys(libxl_ctx *ctx
>>      return 0;
>>  }
>>  
>> +int libxl_log_level(libxl_ctx *ctx, bool set, bool guest,
>> +                    int *lower_thresh, int *upper_thresh)
>> +{
>> +    int ret;
>> +    GC_INIT(ctx);
>> +    if (set) {
>> +        ret = xc_set_log_level(ctx->xch, guest, *lower_thresh, 
>> *upper_thresh);
>> +    } else {
>> +        ret = xc_get_log_level(ctx->xch, guest, lower_thresh, upper_thresh);
>> +    }
>> +    if ( ret < 0 ) {
>> +        LOGE(ERROR, "%s %slog level",
>> +             set ? "setting" : "getting", guest ? "guest " : "");
>> +        GC_FREE;
>> +        return ERROR_FAIL;
>> +    }
>> +    GC_FREE;
>> +    return 0;
>> +}
>> +
> 
> As Dario said, libxl tends to have getter and setter pair.

"Tends to have" still leaves room for me to get away without
adjustments. Could you please clearly state whether splitting
this is required for acceptance?

>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -6469,6 +6469,84 @@ int main_debug_keys(int argc, char **arg
>>      return 0;
>>  }
>>  
>> +static const struct {
>> +    int level;
>> +    char string[8];
>> +} loglvls[] = {
>> +    { 0, "none" },
>> +    { 1, "error" },
>> +    { 2, "warning" },
>> +    { 3, "info" },
>> +    { 4, "all" },
>> +    { 4, "debug" },
> 
> The semantics of the numbers should go into libxl_types.idl. Maybe
> something like
> 
> # Keep in sync with Xen log level.
> libxl_xen_log_level = Enumeration (...);
> 
> Then in here
> 
>     static const struct {
>         int level;
>         char string[8];
>     } loglvls[] = {
>         { LIBXL_XEN_LOG_LEVEL_NONE, "none" },
>         { ..., "error" },
>         { ..., "warning" },
>         { ..., "info" },
>         { ..., "all" },
>         { ..., "debug" },
> 
> Please also note that after the introduction of this API, Xen log level
> will become part of the stable API and subject to some compatibility
> constraints. Is this what you wanted?

No, and I actually I'm having trouble following your request: This
lives in xl_cmdimpl.c, which I didn't think is subject to any stability
requirements in the mapping of strings (from the xl command line)
to numbers. It is _specifically_ that I do not want to fix those
mappings.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel