[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [qemu-devel] [PATCH V2 2/3] [RFC] libqblock-API design
|
From: |
Wenchao Xia |
|
Subject: |
Re: [Qemu-devel] [qemu-devel] [PATCH V2 2/3] [RFC] libqblock-API design |
|
Date: |
Mon, 13 Aug 2012 19:20:51 +0800 |
|
User-agent: |
Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120713 Thunderbird/14.0 |
于 2012-8-10 19:02, Kevin Wolf 写道:
Am 09.08.2012 12:12, schrieb Wenchao Xia:
This patch is the API design.
Signed-off-by: Wenchao Xia <address@hidden>
---
libqblock.c | 670 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
libqblock.h | 447 +++++++++++++++++++++++++++++++++++++++
2 files changed, 1117 insertions(+), 0 deletions(-)
create mode 100644 libqblock.c
create mode 100644 libqblock.h
Ignoring the implementation for now as the design should be visible in
the header file.
diff --git a/libqblock.h b/libqblock.h
new file mode 100644
index 0000000..d2e9502
--- /dev/null
+++ b/libqblock.h
@@ -0,0 +1,447 @@
+/*
+ * Copyright IBM Corp. 2012
+ *
+ * Authors:
+ * Wenchao Xia <address@hidden>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef LIBQBLOCK_H
+#define LIBQBLOCK_H
+
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+#define bool _Bool
Why not use stdbool.h?
forgot to use that, will change it, thanks.
+
+#define QB_ERR_MEM_ERR (-1)
+#define QB_ERR_INTERNAL_ERR (-2)
+#define QB_ERR_INVALID_PARAM (-3)
qemu uses errno values internally, I think they would make sense in the
library interface as well.
+
+/* this library is designed around this core struct. */
+struct QBlockState;
+
+/*
+ libarary init
+ This function get the library ready to use.
+ */
+void libqblock_init(void);
+
+/*
+ create a new qbs object
+ params:
+ qbs: out, pointer that will receive created obj.
+ return:
+ 0 on succeed, negative on failure.
+ */
+int qb_state_new(struct QBlockState **qbs);
+
+/*
+ delete a qbs object
+ params:
+ qbs: in, pointer that will be freed. *qbs will be set to NULL.
+ return:
+ void.
+ */
+void qb_state_free(struct QBlockState **qbs);
What happens if the qbs is open? Will it be flushed and closed? If so,
can it fail and we need to allow an error return?
user should call qb_close() if he have called qb_open(), other wise
unexpected thing happens such as this file is not flushed. I need
document this in the comments.
+
+/* flag used in open and create */
+#define LIBQBLOCK_O_RDWR 0x0002
+/* open the file read only and save writes in a snapshot */
+#define LIBQBLOCK_O_SNAPSHOT 0x0008
+/* do not use the host page cache */
+#define LIBQBLOCK_O_NOCACHE 0x0020
+/* use write-back caching */
+#define LIBQBLOCK_O_CACHE_WB 0x0040
+/* use native AIO instead of the thread pool */
+#define LIBQBLOCK_O_NATIVE_AIO 0x0080
+/* don't open the backing file */
+#define LIBQBLOCK_O_NO_BACKING 0x0100
+/* disable flushing on this disk */
+#define LIBQBLOCK_O_NO_FLUSH 0x0200
+/* copy read backing sectors into image */
+#define LIBQBLOCK_O_COPY_ON_READ 0x0400
+/* consistency hint for incoming migration */
+#define LIBQBLOCK_O_INCOMING 0x0800
+
+#define LIBQBLOCK_O_CACHE_MASK \
+ (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH)
+
+enum QBlockProtocol {
+ QB_PROTO_FILE = 0,
+ QB_PROTO_MAX
+};
+
+enum QBlockFormat {
+ QB_FMT_NONE = 0,
+ QB_FMT_COW,
+ QB_FMT_QED,
+ QB_FMT_QCOW,
+ QB_FMT_QCOW2,
+ QB_FMT_RAW,
+ QB_FMT_RBD,
+ QB_FMT_SHEEPDOG,
+ QB_FMT_VDI,
+ QB_FMT_VMDK,
+ QB_FMT_VPC,
+ QB_FMT_MAX
+};
Not sure if this is a good idea with respect to extensibility. Today you
only need to create a new block/foo.c and add it to the Makefile in
order to add a new format and protocol. It would be better if the
library could make use of it without changing these enums, e.g. by
referring to formats by string (possibly getting a struct referring to a
format by something like qblk_get_format("raw"))
qblk_get_format("raw") seems a good way solve the option difference,
maybe user call it as:
void *op = qblk_get_format_option("raw");
struct QBlockOption_raw *o_raw = (struct QBlockOption_raw *)op;
o_raw.virt_size = 16 * 1024;
I think this is good, with the cost of user have to deal string
instead of simpler enum integer.
+
+/* block target location info, it include all information about how to find
+ the image */
+struct QBlockLocInfo {
+ int struct_size;
+ const char *filename;
+ enum QBlockProtocol protocol;
+};
This is a relatively simplistic view of the world. It works okay for
local image files (which is probably the most common use case for the
library), but you get into trouble as soon as you want to build some
less trivial structures, like something including blkdebug.
Eventually, this leads straight into -blockdev discussions, so maybe we
should indeed go with a very simplistic approach for the start, but
always be aware that a more general solution is to be expected and
should be possible to fit in naturally.
if blkdebug is used for debug block I/O, maybe we can sort it into
another protocol.
+
+/* how to open the image */
+struct QBlockOptionOpen {
+ int struct_size;
+ struct QBlockLocInfo o_loc; /* how to find */
+ enum QBlockFormat o_fmt_type; /* how to extract */
+ int o_flag; /* how to control */
+};
+
+/*
+ create a new QBlockOptionOpen structure.
+ params:
+ op: out, pointer that will receive created structure.
+ return:
+ 0 on succeed, negative on failure.
+ */
+static inline int qb_oo_new(struct QBlockOptionOpen **op)
+{
+ *op = calloc(1, sizeof(struct QBlockOptionOpen));
+ if (*op == NULL) {
+ return QB_ERR_MEM_ERR;
+ }
+ (*op)->struct_size = sizeof(struct QBlockOptionOpen);
+ (*op)->o_loc.struct_size = sizeof(struct QBlockLocInfo);
+ return 0;
+}
+
+/*
+ free QBlockOptionOpen structure.
+ params:
+ op: in, *op will be set as NULL after called.
+ return:
+ void
+ */
+static inline void qb_oo_free(struct QBlockOptionOpen **op)
+{
+ free(*op);
+ *op = NULL;
+}
+
+/*
+ open a block object.
+ params:
+ qbs: in, pointer to QBlockState.
+ op: in, options for open.
+ return:
+ 0 on success, negative on fail.
+ */
+int qb_open(struct QBlockState *qbs, struct QBlockOptionOpen *op);
No further explanation on what negative values there are allowed? In
practice I guess -errno, but will it be officially undefined?
Will there be a function that allows getting an error message if it has
failed?
Yes, there is a API doint that.
+
+/*
+ close a block object.
+ params:
+ qbs: in, pointer to QBlockState.
+ return:
+ void.
+ */
+void qb_close(struct QBlockState *qbs);
Does it imply a flush? Do we need an error return then?
No flush now, will flush and document it next time.
+
+/* format related options, struct_size must be set. */
+struct QBlockOption_cow {
+ int struct_size;
+ size_t virt_size;
+ const char *backing_file;
+ int backing_flag;
+};
+
+struct QBlockOption_qed {
+ int struct_size;
+ size_t virt_size;
+ const char *backing_file;
+ enum QBlockFormat backing_fmt;
+ int backing_flag;
+ size_t cluster_size; /* unit is bytes */
+ size_t table_size; /* unit is clusters */
+};
+
+struct QBlockOption_qcow {
+ int struct_size;
+ size_t virt_size;
+ const char *backing_file;
+ int backing_flag;
+ bool encrypt;
+};
+
+struct QBlockOption_qcow2 {
+ int struct_size;
+ size_t virt_size;
+ const char *compat_level; /* "Compatibility level (0.10 or 1.1)" */
+ const char *backing_file;
+ enum QBlockFormat backing_fmt;
+ int backing_flag;
+ bool encrypt;
+ size_t cluster_size; /* unit is bytes */
+ const char *prealloc_mode; /* off or metadata */
+};
+
+struct QBlockOption_raw {
+ int struct_size;
+ size_t virt_size;
+};
+
+struct QBlockOption_rbd {
+ int struct_size;
+ size_t virt_size;
+ size_t cluster_size;
+};
+
+struct QBlockOption_sheepdog {
+ int struct_size;
+ size_t virt_size;
+ const char *backing_file;
+ int backing_flag;
+ const char *prealloc_mode; /* off or full */
+};
+
+struct QBlockOption_vdi {
+ int struct_size;
+ size_t virt_size;
+ size_t cluster_size;
+ bool prealloc_mode;
+};
+
+struct QBlockOption_vmdk {
+ int struct_size;
+ size_t virt_size;
+ const char *backing_file;
+ int backing_flag;
+ bool compat_version6;
+ const char *subfmt;
+ /* vmdk flat extent format, values:
+ "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse |
+ twoGbMaxExtentFlat | streamOptimized} */
+};
+
+struct QBlockOption_vpc {
+ int struct_size;
+ size_t virt_size;
+ const char *subfmt; /* "{dynamic (default) | fixed} " */
+};
Ouch. :-)
But maybe it's the best we can do...
This structure brought format specified options to the surface,
user could understand them easier and simply give values to the
members, so this is a friendly API. But with a union implement became
hard, I think the way your suggested as
qblk_format_option_get("raw")
could make thing easier.
+
+union QBlockOption_fmt {
+ struct QBlockOption_cow o_cow;
+ struct QBlockOption_qed o_qed;
+ struct QBlockOption_qcow o_qcow;
+ struct QBlockOption_qcow2 o_qcow2;
+ struct QBlockOption_raw o_raw;
+ struct QBlockOption_rbd o_rbd;
+ struct QBlockOption_sheepdog o_sheepdog;
+ struct QBlockOption_vdi o_vdi;
+ struct QBlockOption_vmdk o_vmdk;
+ struct QBlockOption_vpc o_vpc;
+};
+
+struct QBlockOptionFormat {
+ int struct_size;
+ enum QBlockFormat fmt_type;
+ union QBlockOption_fmt fmt_op;
+};
+
+/* struct_size in o_loc and o_fmt must set. To make this structure extensible,
+ all new member must be added in the tail of each structure. */
+struct QBlockOptionCreate {
+ int struct_size;
size_t?
struct_size will not be big, should I also use size_t?
+ struct QBlockLocInfo o_loc;
+ struct QBlockOptionFormat o_fmt;
+};
+
+/*
+ create a new QBlockOptionCreate structure.
+ params:
+ op: out, pointer that will receive created structure.
+ fmt: format want to use.
+ return:
+ 0 on succeed, negative on failure.
+ */
+static inline int qb_oc_new(struct QBlockOptionCreate **op,
+ enum QBlockFormat fmt)
+{
+ *op = calloc(1, sizeof(struct QBlockOptionCreate));
+ if (*op == NULL) {
+ return QB_ERR_MEM_ERR;
+ }
+ (*op)->struct_size = sizeof(struct QBlockOptionCreate);
+ (*op)->o_loc.struct_size = sizeof(struct QBlockLocInfo);
+ (*op)->o_fmt.struct_size = sizeof(struct QBlockOptionFormat);
+
+ switch (fmt) {
+ case QB_FMT_COW:
+ (*op)->o_fmt.fmt_op.o_cow.struct_size =
+ sizeof(struct QBlockOption_cow);
+ break;
+ case QB_FMT_QED:
+ (*op)->o_fmt.fmt_op.o_qed.struct_size =
+ sizeof(struct QBlockOption_qed);
+ break;
+ case QB_FMT_QCOW:
+ (*op)->o_fmt.fmt_op.o_qcow.struct_size =
+ sizeof(struct QBlockOption_qcow);
+ break;
+ case QB_FMT_QCOW2:
+ (*op)->o_fmt.fmt_op.o_qcow2.struct_size =
+ sizeof(struct QBlockOption_qcow2);
+ break;
+ case QB_FMT_RAW:
+ (*op)->o_fmt.fmt_op.o_raw.struct_size =
+ sizeof(struct QBlockOption_raw);
+ break;
+ case QB_FMT_RBD:
+ (*op)->o_fmt.fmt_op.o_rbd.struct_size =
+ sizeof(struct QBlockOption_rbd);
+ break;
+ case QB_FMT_SHEEPDOG:
+ (*op)->o_fmt.fmt_op.o_sheepdog.struct_size =
+ sizeof(struct QBlockOption_sheepdog);
+ break;
+ case QB_FMT_VDI:
+ (*op)->o_fmt.fmt_op.o_vdi.struct_size =
+ sizeof(struct QBlockOption_vdi);
+ break;
+ case QB_FMT_VMDK:
+ (*op)->o_fmt.fmt_op.o_vmdk.struct_size =
+ sizeof(struct QBlockOption_vmdk);
+ break;
+ case QB_FMT_VPC:
+ (*op)->o_fmt.fmt_op.o_vpc.struct_size =
+ sizeof(struct QBlockOption_vpc);
+ break;
+ default:
+ break;
+ }
+ (*op)->o_fmt.fmt_type = fmt;
+ return 0;
+}
+
This is what make implement hard, with format union, in order to
form a friendly API.
+/*
+ free QBlockOptionCreate structure.
+ params:
+ op: in, *op will be set as NULL after called.
+ return:
+ void
+ */
+static inline void qb_oc_free(struct QBlockOptionCreate **op)
+{
+ free(*op);
+ *op = NULL;
+}
+/*
+ create a block file.
+ params:
+ qbs: in, pointer to QBlockState.
+ op: in, create option.
+ return:
+ negative on fail, 0 on success.
+ */
+int qb_create(struct QBlockState *qbs, struct QBlockOptionCreate *op);
+
+
+/* sync access */
+/*
+ block sync read.
+ params:
+ qbs: in, pointer to QBlockState.
+ buf: out, buffer that receive the content.
+ len: in, length to read.
+ offset: in, offset in the block data.
+ return:
+ negative on fail, 0 on success.
+ */
+int qb_read(struct QBlockState *qbs, const void *buf, size_t len,
+ off_t offset);
+
+/*
+ block sync read.
+ params:
+ qbs: in, pointer to QBlockState.
+ buf: in, buffer that would be wrote.
+ len: in, length to write.
+ offset: in, offset in the block data.
+ return:
+ negative on fail, 0 on success.
+ */
+int qb_write(struct QBlockState *qbs, const void *buf, size_t len,
+ off_t offset);
+
+/*
+ block sync flush.
+ params:
+ qbs: in, pointer to QBlockState.
+ return:
+ negative on fail, 0 on success.
+ */
+int qb_flush(struct QBlockState *qbs);
+
+/* information */
+/* image related info, static information, from user perspective. */
+/* now it is a plain structure, wonder if it could be foldered into embbed one
+ to reflect that format related information better. */
+struct QBlockInfoImage {
+ int struct_size;
+ char *filename;
+ enum QBlockProtocol protocol;
+ enum QBlockFormat format;
+ size_t virt_size;
+ /* advance info */
+ size_t allocated_size;
+ bool encrypt;
+ char *backing_filename;
+};
What about things like cluster size, dirty state etc. that only some
image formats have?
hard to resolve, enum plus sub format structure seems reasonable. My
plan is using embbed structure:
struct QBlockInfoImage {
int struct_size;
struct QBlockInfoLoc *i_loc;
struct QBlockInfoFormat *i_fmt;
};
with format string user got, user do a cast to specified format info.
+
+/* image info */
+/*
+ get image info.
+ params:
+ qbs: in, pointer to QBlockState.
+ info, out, pointer that would receive the information.
+ return:
+ negative on fail, 0 on success.
+ */
+int qb_infoimage_get(struct QBlockState *qbs, struct QBlockInfoImage **info);
+
+/*
+ free image info.
+ params:
+ info, in, pointer, *info would be set to NULL after function called.
+ return:
+ void.
+ */
+void qb_infoimage_free(struct QBlockInfoImage **info);
+
+/* misc */
+bool qb_supports_format(enum QBlockFormat fmt);
+bool qb_supports_protocol(enum QBlockProtocol proto);
+
+const char *qb_error_get_detail(struct QBlockState *qbs, int *err_num);
This needs a more detailed description. I also think that you'll want to
add a const char** parameter for human readable error messages (as
printed with qerror_report or error_set in qemu).
This API try to return human readable strings. Will add document.
Kevin
--
Best Regards
Wenchao Xia