Re: [Xen-devel] [PATCH v3 14/15] tools: implement new generic set value interface and MBA set value command

[Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>]

On 17-09-19 12:30:59, Roger Pau Monn� wrote:
> >  int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
> > @@ -458,7 +432,33 @@ int libxl_psr_set_val(libxl_ctx *ctx, uint32_t domid,
> >                        libxl_psr_type type, libxl_bitmap *target_map,
> >                        uint64_t val)
> >  {
> > -    return ERROR_FAIL;
> > +    GC_INIT(ctx);
> > +    int rc;
> > +    int socketid, nr_sockets;
> 
> You could fit them all in a single line.
> 
Yes, thanks.

> > +int main_psr_mba_set(int argc, char **argv)
> > +{
> > +    uint32_t domid;
> > +    libxl_psr_type type;
> > +    uint64_t thrtl;
> > +    int ret, opt = 0;
> > +    libxl_bitmap target_map;
> > +    char *value;
> > +    libxl_string_list socket_list;
> > +    unsigned long start, end;
> > +    unsigned int i, j, len;
> > +
> > +    static const struct option opts[] = {
> > +        {"socket", 1, 0, 's'},
> > +        COMMON_LONG_OPTS
> > +    };
> > +
> > +    libxl_socket_bitmap_alloc(ctx, &target_map, 0);
> > +    libxl_bitmap_set_none(&target_map);
> > +
> > +    SWITCH_FOREACH_OPT(opt, "s:", opts, "psr-mba-set", 0) {
> > +    case 's':
> > +        trim(isspace, optarg, &value);
> > +        split_string_into_string_list(value, ",", &socket_list);
> > +        len = libxl_string_list_length(&socket_list);
> > +        for (i = 0; i < len; i++) {
> > +           parse_range(socket_list[i], &start, &end);
> 
> Indentation.
> 
Sorry.

> > +            for (j = start; j <= end; j++)
> > +                libxl_bitmap_set(&target_map, j);
> > +        }
> > +
> > +        libxl_string_list_dispose(&socket_list);
> > +        free(value);
> > +        break;
> > +    }
> > +
> > +    type = LIBXL_PSR_CBM_TYPE_MBA_THRTL;
> > +
> > +    if (libxl_bitmap_is_empty(&target_map))
> > +        libxl_bitmap_set_any(&target_map);
> > +
> > +    if (argc != optind + 2) {
> > +        help("psr-mba-set");
> > +        return 2;
> > +    }
> 
> Can you do this check at the beginning of the function? Also why
> return 2 instead of EXIT_FAILURE?
> 
Yes, will move it to the beginning. Will return EXIT_FAILURE.

> Thanks, Roger.

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