[Xen-devel] Re: [patch/rfc] multiprotocol blkback drivers (32-on-64)

[Gerd Hoffmann <kraxel@xxxxxxx>]

Jan Beulich wrote:
> There are a couple of things I'd like to see changed if this is what we want 
> to
> go with:

yep, it's a first at least partly working rfc patch, certainly not final
yet ;)

> - "if (protocol == 1) {} else {}" should be switches, failing (or even 
> BUGing) for
>   all protocol versions other than 1 and 2

BUG() should be ok, in theory the code should never ever reach one of
the switches with an uninitialized protocol.

> - assuming the abstraction is meant to scale to future protocol versions, 
> adding
>   many such explicit version handling code paths seems undesirable, as seems
>   adding extra version specific variables or (non-union) structure members

Using unions is one of the things I plan to change.

> - using all error possible values returned from xenbus_gather to indicate an 
> old
>   frontend seems odd at least - one specific error value should be
>   recognized here

Yep, would be a bit cleaner, although I don't see any other possible
reason than a nonexisting node why it should fail at that point ...

> - unconditionally using #pragma pack(), __attribute__(()), and __i386__ or
>   __x86_64__ in public Xen headers is, in my opinion, a no-go (these header
>   should all be suitable for building e.g. Windows drivers, too - I know this 
> isn't
>   generally the case at present, but I don't think anything else can be the 
> goal,
>   and hence the situation shouldn't be made worse)

Yep, we need some solution for that.  The sun folks will veto at least
the attribute stuff too.  Not sure pragma pack is a portability issue.
I think we need some compiler.h which provides that kind of stuff, i.e.

#ifdef __GNUC__
#define __align8__ __attribute__((__aligned__(8)))
#endif

#ifdef __suncc__
#define __align8__ something_else
#endif

Also some of the bits I've placed into blkif.h for now should go to a
linux-kernel header instead I think.

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@xxxxxxx>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel