[Libvir] Documentation errors and shortcomings
Hello! I am currently working on java bindings for libvirt, and in the process I have found a few bugs with the documentation, and also what I believe to be a design problem in the API. 1. The parameterized C macros do not have their arguments listed either on libvirt.org, or in the docs folder in the distribution list, even though there is a a documentation block in the actual source file. I guess it's some kind of problem with the tool used for generating the docs. 2. The virConnectGetCapabilities function docs says that it returns an opaque pointer to a struct, when in fact it returns an XML string describing the capabilities. 3. I have found no sanctioned way to actually get the data from DomainBlockStats and DomainInterfaceStats. The strucures are defined as _virDomainBlockStats and struct _virDomainInterfaceStats, and there is only an *Ptr type typedef-ed on them, as in the case of, say virConnectPtr, which IS really meant to be opaque. I believe that the fields of these structs ARE meant to be read by the applications linking the library, and are not really meant to be opaque. (or if they are, they should have some functions/macros that you can access their contents with) I think the right way to fix this would be simply to typedef them as typedef struct _virDomainBlockStats virDomainBlockStats; typedef struct _virDomainInterfaceStats DomainInterfaceStats; expose their fields in the documentation, and not call them opaque ( i.e. treat them just like virSchedParameter) best regards István
On Fri, Sep 07, 2007 at 06:52:21PM +0200, T?th Istv?n wrote:
Hello!
I am currently working on java bindings for libvirt, and in the process I have found a few bugs with the documentation, and also what I believe to be a design problem in the API.
1. The parameterized C macros do not have their arguments listed either on libvirt.org, or in the docs folder in the distribution list, even though there is a a documentation block in the actual source file. I guess it's some kind of problem with the tool used for generating the docs.
Sounds like its a bug - will let Daniel debug that one.
2. The virConnectGetCapabilities function docs says that it returns an opaque pointer to a struct, when in fact it returns an XML string describing the capabilities.
Yes, that's a bug in the docs from previous iteration of the patches for that API. Will remove it.
3. I have found no sanctioned way to actually get the data from DomainBlockStats and DomainInterfaceStats. The strucures are defined as _virDomainBlockStats and struct _virDomainInterfaceStats, and there is only an *Ptr type typedef-ed on them, as in the case of, say virConnectPtr, which IS really meant to be opaque.
Well the struct definition is in the header file - you don't need a typedef to be able to access the fields.
I believe that the fields of these structs ARE meant to be read by the applications linking the library, and are not really meant to be opaque. (or if they are, they should have some functions/macros that you can access their contents with)
Yes, anything you see in libvirt.h is fair game for applications to use. Private stuff is simply not installed into /usr/include
I think the right way to fix this would be simply to typedef them as typedef struct _virDomainBlockStats virDomainBlockStats; typedef struct _virDomainInterfaceStats DomainInterfaceStats;
For the sake of consistency with other structs, yes we should add these typedefs - they're not required for sake of accessing the fields in the struct though. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Fri, Sep 07, 2007 at 06:52:21PM +0200, Tóth István wrote:
Hello!
I am currently working on java bindings for libvirt, and in the process
Interesting, so you're using JNI direct access to the C library, right ? I looked at this a few weeks ago, but it was looking like accessing using the remote access would have made the Java bindings more platform independant but I had troubles with the RPC/TLS support, and didn't go very far. Maybe a JNI based solution is good enough for most potential Java users.
I have found a few bugs with the documentation, and also what I believe to be a design problem in the API.
1. The parameterized C macros do not have their arguments listed either on libvirt.org, or in the docs folder in the distribution list, even though there is a a documentation block in the actual source file. I guess it's some kind of problem with the tool used for generating the docs.
Yes that's a problem in the generator. Also it's rather hard for it to distinguish #define foo (bar) from #define foo2 (16+1) , the first one is a parametrized macro, the second is not, and we use both ATM.
2. The virConnectGetCapabilities function docs says that it returns an opaque pointer to a struct, when in fact it returns an XML string describing the capabilities.
Right, fixed in CVS, thanks !
3. I have found no sanctioned way to actually get the data from DomainBlockStats and DomainInterfaceStats. The strucures are defined as _virDomainBlockStats and struct _virDomainInterfaceStats, and there is only an *Ptr type typedef-ed on them, as in the case of, say virConnectPtr, which IS really meant to be opaque.
That's a generator problem because the stats structure were defined in a slightly different way than usual.
I believe that the fields of these structs ARE meant to be read by the applications linking the library, and are not really meant to be opaque.
yes
I think the right way to fix this would be simply to typedef them as typedef struct _virDomainBlockStats virDomainBlockStats; typedef struct _virDomainInterfaceStats DomainInterfaceStats;
expose their fields in the documentation, and not call them opaque ( i.e. treat them just like virSchedParameter)
Yes except virDomainBlockStats and DomainInterfaceStats are the name of the associated functions, I had to pick different names but this is now fixed: http://libvirt.org/html/libvirt-libvirt.html#virDomainBlockStatsStruct thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Tóth István wrote:
I believe that the fields of these structs ARE meant to be read by the applications linking the library, and are not really meant to be opaque. (or if they are, they should have some functions/macros that you can access their contents with)
Yes, you should access the fields directly. If an individual stats field isn't supported by the hypervisor, it will be returned as ((long long) -1) [for various reasons we're using long long here, but really we mean 64 bit signed int]. If a field is returned as 0, that means "no activity". However I have noticed there is a problem with fullvirt domains on Xen 3.0.3 which means that network stats are reported as 0. I don't understand yet what causes this problem, but obviously it's a bug somewhere, seemingly in Xen itself. The code works fine with recent Xen hypervisors. If the hypervisor doesn't support stats collection at all then you'll get an error from the function call (ie. the function itself will return -1). At the moment the functions aren't supported on QEMU or KVM at all, something which really needs to be fixed as soon as possible. They also have rather partial support on Xen, in particular fullvirt Xen domains cannot return disk stats for what is essentially a very trivial reason which could be fixed in about 3 lines of code. For updated status, please track this page: http://libvirt.org/hvsupport.html You have to pass the size of the structure to the calls. This allows us to extend the structure in future with additional fields, but also to detect the new caller / old libvirt case and avoid a segfault. Such are the joys of type unsafe dynamic linking. To see these functions being called from C code, go to the following page and search down for 'ocaml_libvirt_domain_block_stats' and 'ocaml_libvirt_domain_interface_stats'. http://hg.et.redhat.com/virt/applications/virt-top--devel?f=da16c97fea65;fil... Daniel Veillard has fixed the missing typedef problem in CVS. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones -
Tóth István