Re: [Qemu-devel] [PATCH V17 07/10] libqblock: libqblock API design and type defines

[Stefan Hajnoczi]

On Thu, Jan 31, 2013 at 04:53:47PM +0800, Wenchao Xia wrote:
>   Public API design header files: libqblock.h, libqblock-error.h.
> Public type define header files: libqblock-types.h. Private internal used
> header files: libqblock-internal. For ABI some reserved bytes are used in
> structure defines. Macro QEMU_DLL_PUBLIC is used to mark exported function.
> 
> Important APIs:
>   1 QBlockContext. This structure was used to retrieve errors, every thread
> must create one first.
>   2 QBlockImage. It stands for an block image object.
>   3 QBlockStaticInfo. It contains static information such as location, backing
> file, size.
>   4 Sync I/O. It is similar to C file open, read, write and close operations.
> 
> Signed-off-by: Wenchao Xia <address@hidden>
> ---
>  libqblock/libqblock-error.h    |   50 ++++++
>  libqblock/libqblock-internal.h |   68 ++++++++
>  libqblock/libqblock-types.h    |  245 +++++++++++++++++++++++++++
>  libqblock/libqblock.h          |  358 
> ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 721 insertions(+), 0 deletions(-)
>  create mode 100644 libqblock/libqblock-internal.h
> 
> diff --git a/libqblock/libqblock-error.h b/libqblock/libqblock-error.h
> index e69de29..1907ecf 100644
> --- a/libqblock/libqblock-error.h
> +++ b/libqblock/libqblock-error.h
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Wenchao Xia   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef LIBQBLOCK_ERROR
> +#define LIBQBLOCK_ERROR
> +
> +#include "libqblock-types.h"
> +
> +#define QB_ERR_INTERNAL_ERR (-1)
> +#define QB_ERR_FATAL_ERR (-2)
> +#define QB_ERR_INVALID_PARAM (-100)
> +#define QB_ERR_BLOCK_OUT_OF_RANGE (-101)
> +
> +/* error handling */
> +/**
> + * qb_error_get_human_str: get human readable error string.
> + *
> + * return a human readable string, it would be truncated if buf is not big
> + *  enough.
> + *
> + * @context: operation context, must be valid.
> + * @buf: buf to receive the string.
> + * @buf_size: the size of the string buf.
> + */
> +QEMU_DLL_PUBLIC
> +void qb_error_get_human_str(QBlockContext *context,
> +                            char *buf, size_t buf_size);

This function is broken.  There is no way to find out how big buf must
be and there is no way to find if the result is truncated or not.

This is actually worse than:
#define QB_ERROR_HUMAN_SIZE 512
/* buf must be QB_ERROR_HUMAN_SIZE */
void qb_error_get_human_str(QBlockContext *context, char *buf);

At least here the caller knows they will get the full error message
because the library designer knows the error message lengths better than
the application writer.

The nicest would be:

/* Return human-readable error message.  The caller must use free(3). */
char *qb_error_get_human_str(QBlockContext *context);

> +
> +/**
> + * qb_error_get_errno: get error number, only valid when err_ret is
> + *   QB_ERR_INTERNAL_ERR.
> + *
> + * return negative errno if last error is QB_ERR_INTERNAL_ERR, otherwise 0.
> + *
> + * @context: operation context.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_error_get_errno(QBlockContext *context);

QB_ERR_INTERNAL_ERR is an internal error which means the caller doesn't
know exactly what happened.  How is the application supposed to use the
errno?

The only thing the application could do is strerror(3) but
qb_error_get_human_str() should already do that.

Therefore qb_error_get_errno() serves no purpose.

> +
> +#endif
> diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
> new file mode 100644
> index 0000000..91521f5
> --- /dev/null
> +++ b/libqblock/libqblock-internal.h
> @@ -0,0 +1,68 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Wenchao Xia   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef LIBQBLOCK_INTERNAL
> +#define LIBQBLOCK_INTERNAL
> +
> +#include <glib.h>
> +
> +#include "block/block.h"
> +#include "libqblock-types.h"
> +
> +/* this file contains defines and types used inside the library. */
> +
> +#define QB_FREE(p) { \
> +        g_free(p); \
> +        (p) = NULL; \
> +}
> +
> +/* details should be hidden to user */
> +struct QBlockImage {
> +    BlockDriverState *bdrvs;
> +    /* internal used file name now, if it is not NULL, it means
> +       image was opened.
> +    */
> +    char *filename;
> +    int ref_count;
> +};
> +
> +struct QBlockContext {
> +    /* last error */
> +    GError *g_error;
> +    int err_ret; /* 1st level of error, the libqblock error number */
> +    int err_no; /* 2nd level of error, errno what below reports */
> +};
> +
> +/**
> + * QBlockStaticInfoAddr: a structure contains a set of pointer.
> + *
> + *    this struct contains a set of pointer pointing to some
> + *  property related to format or protocol. If a property is not available,
> + *  it will be set as NULL. User could use this to get properties directly.
> + *
> + *  @backing_loc: backing file location.
> + *  @encrypt: encryption flag.
> +*/
> +
> +typedef struct QBlockStaticInfoAddr {
> +    QBlockLocationInfo *backing_loc;
> +    bool *encrypt;
> +} QBlockStaticInfoAddr;
> +
> +#define G_LIBQBLOCK_ERROR g_libqblock_error_quark()
> +
> +static inline GQuark g_libqblock_error_quark(void)
> +{
> +    return g_quark_from_static_string("g-libqblock-error-quark");
> +}

qb_error_quark() would be a better name.  g_*() is for GLib not for code
using glib.

The shorter name also means you can drop the macro.  Macros like
QB_FREE() and G_LIBQBLOCK_ERROR hide a very simple operation.  They
force people reading the code to jump to its definition; it interrupts
the reader and forces them to remember abstractions that add no value.

> +#endif
> diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
> index e69de29..d49b321 100644
> --- a/libqblock/libqblock-types.h
> +++ b/libqblock/libqblock-types.h
> @@ -0,0 +1,245 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Wenchao Xia   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef LIBQBLOCK_TYPES_H
> +#define LIBQBLOCK_TYPES_H
> +
> +#include <sys/types.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +/*
> + * In case libqblock is used by other project which have not defined
> + * QEMU_DLL_PUBLIC.
> + */
> +#ifndef QEMU_DLL_PUBLIC
> +#define QEMU_DLL_PUBLIC
> +#endif
> +
> +/* this library is designed around this core struct. */
> +typedef struct QBlockImage QBlockImage;
> +
> +/* every thread should have a context. */
> +typedef struct QBlockContext QBlockContext;
> +
> +/* flag used in open and create */
> +#define LIBQBLOCK_O_RDWR        0x0002
> +/* do not use the host page cache */
> +#define LIBQBLOCK_O_NOCACHE     0x0020
> +/* use write-back caching */
> +#define LIBQBLOCK_O_CACHE_WB    0x0040
> +/* don't open the backing file */
> +#define LIBQBLOCK_O_NO_BACKING  0x0100
> +/* disable flushing on this disk */
> +#define LIBQBLOCK_O_NO_FLUSH    0x0200
> +
> +#define LIBQBLOCK_O_CACHE_MASK \
> +   (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH)
> +
> +#define LIBQBLOCK_O_VALID_MASK \
> +   (LIBQBLOCK_O_RDWR | LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | \
> +    LIBQBLOCK_O_NO_BACKING | LIBQBLOCK_O_NO_FLUSH)

It is tempting to simplify the libqblock cache options so that:

 * The host page cache is always used.
 * Flush operations are required to ensure writes reach the disk.

This means disk images would behave like regular files.  And it would
prevent a lot of confusion about the meaning of the cache modes.

However, advanced applications might want to use O_DIRECT or even
NO_FLUSH.  So I guess it's okay to expose this.

> +
> +typedef enum QBlockProtocol {
> +    QB_PROTO_NONE = 0,
> +    QB_PROTO_FILE,
> +    QB_PROTO_MAX
> +} QBlockProtocol;

What prevents libqblock from supporting all protocols?

> +typedef struct QBlockProtocolOptionsFile {
> +    const char *filename;
> +} QBlockProtocolOptionsFile;
> +
> +/**
> + * struct QBlockLocationInfo: contains information about how to find the 
> image
> + *
> + * @prot_type: protocol type, now only support FILE.
> + * @o_file: file protocol related attributes.
> + * @reserved: reserved bytes for ABI.
> + */
> +#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
> +typedef struct QBlockLocationInfo {
> +    QBlockProtocol prot_type;
> +    union {
> +        QBlockProtocolOptionsFile o_file;
> +        uint64_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE/sizeof(uint64_t)];
> +    };
> +} QBlockLocationInfo;
> +
> +
> +/* format related options */
> +typedef enum QBlockFormat {
> +    QB_FORMAT_NONE = 0,
> +    QB_FORMAT_COW,
> +    QB_FORMAT_QED,
> +    QB_FORMAT_QCOW,
> +    QB_FORMAT_QCOW2,
> +    QB_FORMAT_RAW,
> +    QB_FORMAT_RBD,
> +    QB_FORMAT_SHEEPDOG,
> +    QB_FORMAT_VDI,
> +    QB_FORMAT_VMDK,
> +    QB_FORMAT_VPC,
> +    QB_FORMAT_MAX
> +} QBlockFormat;
> +
> +typedef struct QBlockFormatOptionsCOW {
> +    QBlockLocationInfo backing_loc;
> +} QBlockFormatOptionsCOW;

Are these struct definitions frozen?

An application could do:

QBlockFormatInfo info = ...;
QBlockFormatOptionsCOW opts = info.o_cow; /* broken */

If QBlockFormatOptionsCOW changes size then the application can break.
It's hard to demand that applications don't use these structs directly
and once broken applications rely on it we may have a hard time arguing
with them that it's their fault the application is incompatible with new
versions of libqblock (even if we're technically right).

> +typedef struct QBlockFormatOptionsQCOW2 {
> +    QBlockLocationInfo backing_loc;
> +    QBlockFormat backing_fmt;
> +    bool encrypt;
> +    uint64_t cluster_size; /* unit is bytes */
> +    QBlockFormatOptionsQCOW2CompatLv cpt_lv;
> +    QBlockFormatOptionsQCOW2PreAlloc pre_mode;
> +} QBlockFormatOptionsQCOW2;

...this is an example of a struct where we still need to add fields.
qcow2 is still under development and will get new options.

> diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
> index e69de29..d3fcaf5 100644
> --- a/libqblock/libqblock.h
> +++ b/libqblock/libqblock.h
> @@ -0,0 +1,358 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Wenchao Xia   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +/* Note: This library is not thread safe yet. */
> +
> +#ifndef LIBQBLOCK_H
> +#define LIBQBLOCK_H
> +
> +#include "libqblock-types.h"
> +#include "libqblock-error.h"
> +
> +/**
> + * qb_context_new: allocate a new context.
> + *
> + * Broker is used to pass operation to libqblock, and get feedback from it.

Broker?

> + *
> + * Returns 0 on success, libqblock negative error value on fail.
> + *
> + * Note this function will try init the library, so it must be called the
> + * first.
> + *
> + * @p_context: used to receive the created struct.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_context_new(QBlockContext **p_context);
> +
> +/**
> + * qb_context_delete: delete context.
> + *
> + * Broker will be freed and set to NULL.
> + *
> + * @p_context: pointer to the struct pointer, *p_context will be set to NULL.
> + */
> +QEMU_DLL_PUBLIC
> +void qb_context_delete(QBlockContext **p_context);
> +
> +/**
> + * qb_image_new: allocate a new QBlockImage struct.
> + *
> + * Subsequent qblock actions will use this struct, ref is set to 1.
> + *
> + * Returns 0 if succeed, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @p_qbi: used to receive the created struct.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_image_new(QBlockContext *context,
> +                 QBlockImage **p_qbi);
> +
> +/**
> + * qb_image_ref: increase the ref of QBlockImage.
> + *
> + * @context: operation context.
> + * @p_qbi: pointer to the struct's pointer.
> + */
> +QEMU_DLL_PUBLIC
> +void qb_image_ref(QBlockContext *context,
> +                  QBlockImage **p_qbi);

Why QBlockImage** instead of QBlockImage*?

What is the relationship between QBlockImage and its context?  Can I
have two contexts A and B like this:

qb_image_ref(ctx_a, &qbi);
qb_image_unref(ctx_b, &qbi);

Is this okay?

> +
> +/**
> + * qb_image_unref: decrease the ref of QBlockImage.
> + *
> + * if ref is reduced to 0, p_qbi would be freed and set to NULL, at which 
> time
> + *  if image was opened and qb_close was not called before, it would be
> + *  automatically closed.
> + *
> + * @context: operation context.
> + * @p_qbi: pointer to the struct's pointer.
> + */
> +QEMU_DLL_PUBLIC
> +void qb_image_unref(QBlockContext *context,
> +                    QBlockImage **p_qbi);
> +
> +/**
> + * qb_location_info_new: create a new QBlockLocationInfo object.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @p_loc: pointer to receive the new created one.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_location_info_new(QBlockContext *context,
> +                         QBlockLocationInfo **p_loc);
> +
> +/**
> + * qb_location_info_delete: free a QBlockLocationInfo.
> + *
> + * @context: operation context.
> + * @p_loc: pointer to the object, *p_loc would be set to NULL.
> + */
> +QEMU_DLL_PUBLIC
> +void qb_location_info_delete(QBlockContext *context,
> +                             QBlockLocationInfo **p_loc);

Why are these functions necessary?  The user has the struct definitions
for QBlockLocationInfo so they can allocate/free them.

> +
> +/**
> + * qb_format_info_new: create a new QBlockFormatInfo structure.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @p_fmt: pointer that will receive created struct.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_format_info_new(QBlockContext *context,
> +                       QBlockFormatInfo **p_fmt);
> +
> +/**
> + * qb_format_info_delete: free QBlockFormatInfo structure.
> + *
> + * @context: operation context.
> + * @p_fmt: pointer to the struct, *p_fmt would be set to NULL.
> + */
> +QEMU_DLL_PUBLIC
> +void qb_format_info_delete(QBlockContext *context,
> +                           QBlockFormatInfo **p_fmt);

Same here, I don't see a need for these functions.

> +
> +/**
> + * qb_open: open a block object.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @qbi: pointer to QBlockImage.
> + * @loc: location options for open, how to find the image.
> + * @fmt: format options, how to extract the data, only valid member now is
> + *    fmt->fmt_type, set to NULL if you want to auto discovery the format.
> + * @flag: behavior control flags, it is LIBQBLOCK_O_XXX's combination.
> + *
> + * Note: For raw image, there is a risk that it's content is changed to some
> + *  magic value resulting a wrong probing done by libqblock, so don't do
> + * probing on raw images.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_open(QBlockContext *context,
> +            QBlockImage *qbi,
> +            QBlockLocationInfo *loc,
> +            QBlockFormatInfo *fmt,
> +            int flag);
> +
> +/**
> + * qb_close: close a block object.
> + *
> + * qb_flush is automatically done inside.
> + *
> + * @context: operation context.
> + * @qbi: pointer to QBlockImage.
> + */
> +QEMU_DLL_PUBLIC
> +void qb_close(QBlockContext *context,
> +              QBlockImage *qbi);
> +
> +/**
> + * qb_create: create a block image or object.
> + *
> + * Note: Create operation would not open the image automatically.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @qbi: pointer to QBlockImage.
> + * @loc: location options for open, how to find the image.
> + * @fmt: format options, how to extract the data.
> + * @flag: behavior control flags, LIBQBLOCK_O_XXX's combination.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_create(QBlockContext *context,
> +              QBlockImage *qbi,

Why is qbi required if the image isn't opened?  We should only need
context.

> +              QBlockLocationInfo *loc,
> +              QBlockFormatInfo *fmt,
> +              int flag);

What is the purpose of LIBQBLOCK_O_* flags if the image is not opened?

> +
> +
> +/* sync access */
> +/**
> + * qb_read: block sync read.
> + *
> + * return number of bytes read, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @qbi: pointer to QBlockImage.
> + * @buf: buffer that receive the content.
> + * @len: length to read.
> + * @offset: offset in the block data.
> + */
> +QEMU_DLL_PUBLIC
> +int32_t qb_read(QBlockContext *context,
> +                QBlockImage *qbi,
> +                uint8_t *buf,
> +                uint32_t len,
> +                uint64_t offset);

The uint32_t len and int32_t return types don't match up.  The return
value needs to be int64_t to handle the full uint32_t range.

However, the return value is only necessary if we want to do short
reads.  How about making the return value int with 0 on success and
negative on error?  Don't do short reads.

> +
> +/**
> + * qb_write: block sync write.
> + *
> + * return number of bytes written, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @qbi: pointer to QBlockImage.
> + * @buf: buffer that receive the content.
> + * @len: length to write.
> + * @offset: offset in the block data.
> + */
> +QEMU_DLL_PUBLIC
> +int32_t qb_write(QBlockContext *context,
> +                 QBlockImage *qbi,
> +                 const uint8_t *buf,
> +                 uint32_t len,
> +                 uint64_t offset);

Same issue with uint32_t len versus int32_t return types.

> +/**
> + * qb_flush: block sync flush.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @qbi: pointer to QBlockImage.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_flush(QBlockContext *context,
> +             QBlockImage *qbi);
> +
> +
> +/* advance image APIs */
> +/**
> + * qb_check_allocation: check if [start, start+lenth-1] was allocated on the

s/lenth/length/

> + *  image.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @qbi: pointer to QBlockImage.
> + * @start: start position, unit is byte.
> + * @length: length to check, unit is byte, max is 1TB, otherwise will return
> + *   QB_ERR_INVALID_PARAM.
> + * @pstatus: pointer to receive the status, 1 means allocated,
> + *  0 means unallocated.
> + * @plength: pointer to receive the length that all have the same status as
> + *  *pstatus.
> + *
> + * Note: after return, start+*plength may have the same status as
> + *  start+*plength-1.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_check_allocation(QBlockContext *context,
> +                        QBlockImage *qbi,
> +                        uint64_t start,
> +                        int64_t length,
> +                        int *pstatus,
> +                        int64_t *plength);

Why uint64_t start but int64_t length and *plength?  They should all be
unsigned.

> +
> +/* image information */
> +/**
> + * qb_get_image_info: get image info.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @context: operation context.
> + * @qbi: pointer to QBlockImage.
> + * @p_info: pointer that would receive the information.
> + *
> + * *info must be not modified after return, qb_info_image_static_delete will
> + *   use the information in it.
> + */
> +QEMU_DLL_PUBLIC
> +int qb_info_image_static_get(QBlockContext *context,
> +                             QBlockImage *qbi,
> +                             QBlockStaticInfo **p_info);
> +
> +/**
> + * qb_delete_image_info: free image info.
> + *
> + * @context: operation context.
> + * @p_info: pointer to the information struct, *p_info will be set to NULL.
> + */
> +QEMU_DLL_PUBLIC
> +void qb_info_image_static_delete(QBlockContext *context,
> +                                 QBlockStaticInfo **p_info);
> +
> +/* helper functions */
> +/**
> + * qb_str2fmttype: translate format string to libqblock format enum type.
> + *
> + * return the type, or QB_FORMAT_NONE if string matches none of supported 
> types,
> + *  never return invalid value or QB_FORMAT_MAX.
> + *
> + * @fmt: the format string, if NULL it will return QB_FORMAT_NONE.
> + */
> +QEMU_DLL_PUBLIC
> +QBlockFormat qb_str2fmttype(const char *fmt_str);
> +
> +/**
> + * qb_formattype2str: translate libqblock format enum type to a string.
> + *
> + * return a pointer to the string, or NULL if type is not supported, and
> + *  returned pointer must NOT be freed.
> + *
> + * @fmt: the format enum type.
> + */
> +QEMU_DLL_PUBLIC
> +const char *qb_formattype2str(QBlockFormat fmt_type);

Why does the QBlockFormat enum exist?  Is there an issue with using strings?

> +/**
> + * qb_location_info_dup: duplicate a QBlockLocationInfo instance.
> + *
> + * return a pointer to new allocated one having the same values with input,
> + *  it need to be freed by qb_location_info_delete later. Never fail except 
> OOM.
> + *
> + * @loc: pointer to the source instance.
> + */
> +QEMU_DLL_PUBLIC
> +QBlockLocationInfo *qb_location_info_dup(const QBlockLocationInfo *loc);
> +
> +/**
> + * qb_get_virt_size: get virtual size.
> + *
> + * return a pointer, which pointer to a member in info, or NULL if info is
> + *  not valid.
> + *
> + * @info: pointer to the QBlockStaticInfo structure.
> + */
> +QEMU_DLL_PUBLIC
> +const uint64_t *qb_get_virt_size(const QBlockStaticInfo *info);
> +
> +/**
> + * qb_get_backing_loc: get backing file location.
> + *
> + * return a pointer, which pointer to a member in info, or NULL if info is
> + *  not valid, or image have no such property.
> + *
> + * @info: pointer to the QBlockStaticInfo structure.
> + */
> +QEMU_DLL_PUBLIC
> +const QBlockLocationInfo *qb_get_backing_loc(const QBlockStaticInfo *info);
> +
> +/**
> + * qb_get_encrypt: get encrytion flag.
> + *
> + * return a pointer, which pointer to a member in info, or NULL if info is
> + *  not valid, or image have no such property.
> + *
> + * @info: pointer to the QBlockStaticInfo structure.
> + */
> +QEMU_DLL_PUBLIC
> +const bool *qb_get_encrypt(const QBlockStaticInfo *info);

These functions suggest the caller shouldn't look inside
QBLockStaticInfo although the struct definition seems to be public?

Stefan