Re: [Xen-devel] [PATCH 10 of 16 RFC] blktap3: Introduce the blktap header file

[Thanos Makatos <thanos.makatos@xxxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell
> Sent: 26 October 2012 12:37
> To: Thanos Makatos
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH 10 of 16 RFC] blktap3: Introduce the
> blktap header file
> 
> > +/*
> > + * TODO Document what each define is.
> > + */
> > +#define BLKTAP3_SYSFS_DIR                   "/sys/class/blktap3"
> 
> I thought there was no kernel side to blktap3?
> 
> > +#define BLKTAP3_CONTROL_NAME                "blktap-control"
> > +#define BLKTAP3_CONTROL_DIR
> "/var/run/"BLKTAP3_CONTROL_NAME
> > +#define BLKTAP3_CONTROL_SOCKET              "ctl"
> 
> > +#define BLKTAP3_DIRECTORY                   "/dev/xen/blktap-3"
> > +#define BLKTAP3_RING_DEVICE
> BLKTAP3_DIRECTORY"/blktap"
> 
> Likewise these two.

Correct, I removed them.

> 
> > +#define BLKTAP3_IO_DEVICEBLKTAP3_DIRECTORY  "/tapdev"
> > +#define BLKTAP3_ENOSPC_SIGNAL_FILE          "/var/run/tapdisk3-
> enospc"
> > +
> > +/*
> > + * TODO Can these be supplied from somewhere else?
> > + */
> > +#define likely(_cond)   __builtin_expect(!!(_cond), 1)
> > +#define unlikely(_cond) __builtin_expect(!!(_cond), 0)
> > +
> > +/*
> > + * FIXME taken from list.h
> > + */
> > +#define containerof(_ptr, _type, _memb) \
> > +    ((_type*)((void*)(_ptr) - offsetof(_type, _memb)))
> > +
> > +/*
> > + * FIXME shouldn't these be supplied from somewhere?
> > + */
> > +#define __printf(a, b) __attribute__((format(printf, a, b))) #define
> > +__scanf(_f, _a) __attribute__((format (scanf, _f, _a)))
> 
> These are all abstractions of gcc-isms (presumably to ease ports to
> other compilers). We generally don't bother with this sort of thing in
> the tools, although you can if you like. You might want to put into a
> compiler.h and add suitable #if like those in
> xen/include/xen/compiler.h.

I'll put them in a compiler.h

> 
> > +
> > +#define TAILQ_MOVE_HEAD(node, src, dst, entry)  \
> > +    TAILQ_REMOVE(src, node, entry);             \
> > +    TAILQ_INSERT_HEAD(dst, node, entry);
> > +
> > +#define TAILQ_MOVE_TAIL(node, src, dst, entry)  \
> > +    TAILQ_REMOVE(src, node, entry);             \
> > +    TAILQ_INSERT_TAIL(dst, node, entry);
> > +
> > +/*
> > + * TODO This is defined in xen/lib.h, use that instead of redefining
> > +it
> > + * here.
> 
> xen/lib.h is a hypervisor internal header so isn't available to you
> here. I think it is fine to define your own (although it should be in a
> private header, not part of the libblktap API)

Right I'll put them in a private header.

> 
> > + */
> > +#ifndef ARRAY_SIZE
> > +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) #endif /*
> > +ARRAY_SIZE */
> > +
> > +#endif /* __BLKTAP_3_H__ */
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel
> 

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