Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines

[Wenchao Xia]

于 2012-9-11 5:27, Eric Blake 写道:
> On 09/10/2012 02:26 AM, Wenchao Xia wrote:
> >    This patch contains type and defines used in APIs, one file for public usage
> > by user, one for libqblock internal usage.
> >
> > Signed-off-by: Wenchao Xia <address@hidden>
> > ---
> >   libqblock/libqblock-internal.h |   50 ++++++++
> >   libqblock/libqblock-types.h    |  251 ++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 301 insertions(+), 0 deletions(-)
> >   create mode 100644 libqblock/libqblock-internal.h
> >   create mode 100644 libqblock/libqblock-types.h
>
> As mentioned in 1/6, this should come earlier in the series, as it lays
> the fundamentals used by other new files.
  OK.

> > diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
> > new file mode 100644
> > index 0000000..fa27ed4
> > --- /dev/null
> > +++ b/libqblock/libqblock-internal.h
> > @@ -0,0 +1,50 @@
>
> > +
> > +#define QB_ERR_STRING_SIZE (1024)
> > +struct QBroker {
> > +    /* last error */
> > +    char err_msg[QB_ERR_STRING_SIZE];
>
> Is this fixed-width struct going to bite us in the future?  Suppose I
> pass in a file name that is already 1000 bytes long; it seems like I
> might be able to get you to overflow this buffer if your error message
> includes the name of my offending file.
  Yes it will, thanks for mention me. The resource will always have a
limit, I guess I can just increase the size to 4k to solve the issue.

> > +++ b/libqblock/libqblock-types.h
> > +/* this library is designed around this core struct. */
> > +struct QBlockState;
> > +
> > +/* every thread would have a broker. */
>
> s/would/should/
 OK.

> > +
> > +/**
> > + * QBlockStaticInfo: information about the block image.
> > + *
> > + * @loc: location info.
> > + * @fmt_type: format type.
> > + * @virt_size: virtual size in bytes.
> > + * @backing_loc: backing file location, its type is QB_PROT_NONE if not exist.
> > + * @encrypt: encrypt flag.
> > + * @sector_size: how many bytes in a sector, it is 512 usually.
> > + */
> > +struct QBlockStaticInfo {
> > +    struct QBlockProtInfo loc;
> > +    enum QBlockFmtType fmt_type;
> > +    uint64_t virt_size;
> > +    /* advance info */
> > +    struct QBlockProtInfo backing_loc;
> > +    bool encrypt;
> > +    int sector_size;
> > +};
>
> No reserved space for potential growth in this structure?
  It could have or not. If new members are added in the tail, then
reserved bytes is not needed, for that this structure's instance
will be allocated in libqblock API.


--
Best Regards

Wenchao Xia