[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH] Remote 2/8: Client-server protocol (updated)



On Sat, May 05, 2007 at 11:53:32AM +0100, Richard W.M. Jones wrote:
> Richard W.M. Jones wrote:
> >2 Client-server protocol
> >------------------------
> >
> >A qemud/remote_protocol.x
> >A qemud/rpcgen_fix.pl
> >A qemud/remote_protocol.c
> >A qemud/remote_protocol.h
> 
> These are the additional files required to describe the XDR-based 
> protocol for client-server communications, with the homebrew RPC 
> mechanism on top.  The first file is the protocol itself.
> 
> The second file is a Perl script to run after rpcgen which allows the 
> code to compile with -Wall -Werror -fstrict-aliasing, and fixes a 64 bit 
> bug (that fix really should go upstream BTW).  The strict-aliasing issue 
> isn't fully fixed yet - I expect that eventually we should compile 
> remote_protocol.c with -fno-strict-aliasing, for safety.  Daniel 
> Veillard also asked that this be rewritten in Python (pah!) which I 
> might do at the same time.
> 
> The last two files are the actual rpcgen-generated rpcgen_fix-fixed 
> code.  I put these two files into EXTRA_DIST because I feel that people 
> shouldn't be required to have rpcgen & Perl just to compile libvirt. 
> (These files are arch-independent so there is no need to recompile them 
> unless remote_protocol.x itself changes).

While I generally don't like having auto-generated files in CVS, I wonder
if this makes us better off for portability to Solaris / BSD. eg, the
Perl post-processor probably wouldn't work with someone else's rpcgen
output. As long as the generated files are calling 100% public RPC related
APIs internally we're probably best off distributing the generated files
as part of the release & only manually doing re-generation when we have
real changes.

> struct remote_open_args {
>     /* NB. "name" might be NULL although in practice you can't
>      * yet do that using the remote_internal driver.
>      */
>     remote_string name;
>     int flags;
> };

Historically NULL == 'Xen', so should we just force 'name = Xen' in src/libvirt.c
and then no internal driver ever has to worry about NULLs in this scenario again.

> struct remote_get_max_vcpus_args {
>     /* The only backend which supports this call is Xen HV, and
>      * there the type is ignored so it could be NULL.
>      */
>     remote_string type;
> };

Good point. I'll implement this API for QEMU real soon now....

> struct remote_domain_lookup_by_id_ret {
>     /* XXX "Not found" semantic is ill-defined. */
>     remote_nonnull_domain dom;
> };
> 
> struct remote_domain_lookup_by_uuid_ret {
>     /* XXX "Not found" semantic is ill-defined. */
>     remote_nonnull_domain dom;
> };
> 
> struct remote_domain_lookup_by_name_ret {
>     /* XXX "Not found" semantic is ill-defined. */
>     remote_nonnull_domain dom;
> };

We should remember to formally define the semantics before release :-)

> struct remote_domain_get_info_ret {
>     unsigned char state;
>     unsigned hyper max_mem;
>     unsigned hyper memory;
>     unsigned short nr_virt_cpu;
>     /* XXX cpu_time is declared as unsigned long long */
>     unsigned hyper cpu_time;
> };

hyper == 64-bits according to the latest XDR spec isn't it. The
only guarentee we need is that  cpu_time is at least 64-bits. Given
the granularity we should never hit a scenario where cpu_time would
overflow 64-bits. So if long long ever happens to be 128, then it
shouldn't matter that hyper were only 64. So I reckon we can remove
the XXX there.

> struct remote_network_lookup_by_uuid_ret {
>     /* XXX "Not found" semantic is ill-defined. */
>     remote_nonnull_network net;
> };
> 
> struct remote_network_lookup_by_name_ret {
>     /* XXX "Not found" semantic is ill-defined. */
>     remote_nonnull_network net;
> };

As above.

> const REMOTE_PROGRAM = 0x20008086;

Any significance to this number :-)

Regards,
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  -=| 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]