Re: [Xen-devel] [PATCH 24/28] libxl: ocaml: add NIC helper functions

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-03-25 at 14:45 +0000, Rob Hoes wrote:
> Signed-off-by: Rob Hoes <rob.hoes@xxxxxxxxxx>
> ---
>  tools/ocaml/libs/xl/genwrap.py       |    5 ++++-
>  tools/ocaml/libs/xl/xenlight_stubs.c |   36 
> ++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
> index 9f7895a..827fdb6 100644
> --- a/tools/ocaml/libs/xl/genwrap.py
> +++ b/tools/ocaml/libs/xl/genwrap.py
> @@ -31,7 +31,10 @@ functions = { # ( name , [type1,type2,....] )
>      "device_vfb":     DEVICE_FUNCTIONS,
>      "device_vkb":     DEVICE_FUNCTIONS,
>      "device_disk":    DEVICE_FUNCTIONS,
> -    "device_nic":     DEVICE_FUNCTIONS,
> +    "device_nic":     DEVICE_FUNCTIONS +
> +                      [ ("list",           ["ctx", "domid", "t list"]),
> +                        ("of_devid",       ["ctx", "domid", "int", "t"]),
> +                      ],
>      "device_pci":     DEVICE_FUNCTIONS,
>      "dominfo":        [ ("list",           ["ctx", "t list"]),
>                          ("get",            ["ctx", "domid", "t"]),
> diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c 
> b/tools/ocaml/libs/xl/xenlight_stubs.c
> index c136db7..ecc26ff 100644
> --- a/tools/ocaml/libs/xl/xenlight_stubs.c
> +++ b/tools/ocaml/libs/xl/xenlight_stubs.c
> @@ -380,6 +380,42 @@ DEVICE_ADDREMOVE(vfb)
>  DEVICE_ADDREMOVE(vkb)
>  DEVICE_ADDREMOVE(pci)
>  
> +value stub_xl_device_nic_of_devid(value ctx, value domid, value devid)
> +{
> +     CAMLparam3(ctx, domid, devid);
> +     libxl_device_nic nic;
> +     libxl_devid_to_device_nic(CTX, Int_val(domid), Int_val(devid), &nic);
> +     CAMLreturn(Val_device_nic(&nic));
> +}
> +
> +value stub_xl_device_nic_list(value ctx, value domid)
> +{
> +     CAMLparam2(ctx, domid);
> +     CAMLlocal2(list, temp);
> +     libxl_device_nic *c_list;
> +     int i, nb;
> +     uint32_t c_domid;
> +
> +     c_domid = Int_val(domid);
> +
> +     c_list = libxl_device_nic_list(CTX, c_domid, &nb);
> +     if (!c_list && nb > 0)

I don't think && nb > 0 can ever occur, the error handling in
libxl_device_nic_lsit does:
        out_err:
            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to list nics");
            while (*num) {
                (*num)--;
                libxl_device_nic_dispose(&nics[*num]);
            }
            free(nics);
            return NULL;
i.e. it counts *num back down to zero. I'd say you shouldn't/mustn't
make any assumptions about nb if the function call failed. 

> +             failwith_xl(ERROR_FAIL, "nic_list");
> +
> +     list = temp = Val_emptylist;
> +     for (i = 0; i < nb; i++) {
> +             list = caml_alloc_small(2, Tag_cons);
> +             Field(list, 0) = Val_int(0);
> +             Field(list, 1) = temp;
> +             temp = list;

This reverses the list, if you care. I don't suppose you do and libxl
probably doesn't actually guarantee anything abort the order.

I wouldn't have noticed except I saw you doing the counting backwards in
an earlier patch and it took me a second to work out why...

> +             Store_field(list, 0, Val_device_nic(&c_list[i]));
> +             libxl_device_nic_dispose(&c_list[i]);
> +     }
> +     free(c_list);
> +
> +     CAMLreturn(list);
> +}
> +
>  value stub_xl_physinfo_get(value ctx)
>  {
>       CAMLparam1(ctx);



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