Re: [Xen-devel] [PATCH v5 10/12] libxl: change nic to use generec add function

[Oleksandr Grytsov <al1img@xxxxxxxxx>]

On Tue, Sep 12, 2017 at 2:08 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> On Tue, Sep 12, 2017 at 11:52:17AM +0100, Wei Liu wrote:
>> On Mon, Sep 11, 2017 at 08:15:00PM +0300, Oleksandr Grytsov wrote:
>> > From: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>
>> >
>> > Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>
>> > ---
>> >  tools/libxl/libxl.h                   |   9 +-
>> >  tools/libxl/libxl_checkpoint_device.c |   9 +-
>> >  tools/libxl/libxl_colo_save.c         |   4 +-
>> >  tools/libxl/libxl_dm.c                |   4 +-
>> >  tools/libxl/libxl_internal.h          |   2 -
>> >  tools/libxl/libxl_nic.c               | 191 
>> > +++++++---------------------------
>> >  tools/ocaml/libs/xl/xenlight_stubs.c  |   3 +-
>> >  tools/xl/xl_nic.c                     |   3 +-
>> >  8 files changed, 52 insertions(+), 173 deletions(-)
>> >
>> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
>> > index d5a3ab7..abe129e 100644
>> > --- a/tools/libxl/libxl.h
>> > +++ b/tools/libxl/libxl.h
>> > @@ -1850,9 +1850,14 @@ int libxl_device_nic_destroy(libxl_ctx *ctx, 
>> > uint32_t domid,
>> >                               const libxl_asyncop_how *ao_how)
>> >                               LIBXL_EXTERNAL_CALLERS_ONLY;
>> >
>> > -libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, 
>> > int *num);
>> > +libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx,
>> > +                                        uint32_t domid, int *num)
>> > +                                        LIBXL_EXTERNAL_CALLERS_ONLY;
>> > +void libxl_device_nic_list_free(libxl_device_nic* list, int num)
>> > +                                LIBXL_EXTERNAL_CALLERS_ONLY;
>> >  int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
>> > -                              libxl_device_nic *nic, libxl_nicinfo 
>> > *nicinfo);
>> > +                             libxl_device_nic *nic, libxl_nicinfo 
>> > *nicinfo)
>> > +                             LIBXL_EXTERNAL_CALLERS_ONLY;
>>
>> The annotation has caused the following failures:
>>
>> libxl_colo_save.c: In function ‘libxl__colo_save_setup’:
>> libxl_colo_save.c:126:14: error: call to ‘libxl_device_nic_list’ declared 
>> with attribute warning: may not be called from within libxl [-Werror]
>> nics = libxl_device_nic_list(CTX, cds->domid, &nb);
>>          ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> libxl_colo_save.c:129:9: error: call to ‘libxl_device_nic_list_free’ 
>> declared with attribute warning: may not be called from within libxl 
>> [-Werror]
>> libxl_device_nic_list_free(nics, nb);
>>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> However, I think that's a bug in colo.
>
> Thinking about this a bit more: the list_free functions (here and in
> other patches) shouldn't be annotated as external callers only.
>
> And you can replace the nic_list function here to call the framework
> directly.

The idea was that libxl_device_xxx_list and free are for external callers only
because they don't aware about device framework. Internal callers shall use
device framework instead. I wonder why I don't have error here on my
build environment. I will put device framework get/free list here.

I will create new patch set.

-- 
Best Regards,
Oleksandr Grytsov.

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