于 2012-8-10 18:48, Paolo Bonzini 写道:
Il 09/08/2012 12:12, Wenchao Xia ha scritto:
+/* copy information and take care the member difference in differect version.
+ Assuming all new member are added in the tail, struct size is the first
+ member, this is old to new version, src have its struct_size set. */
+static void qboc_adjust_o2n(struct QBlockOptionCreate *dest,
+ struct QBlockOptionCreate *src)
+{
+ /* for simple it does memcpy now, need to take care of embbed structure */
+ memcpy(dest, src, src->struct_size);
You need an assertion that src->struct_size < sizeof(*dest).
However, the structure size perhaps wasn't my brightest idea ever. But
still thanks for preparing this prototype! Now that we have a more
complete API to discuss, we can iterate to something better.
+ assert(0 == set_option_parameter_int(param,
+ BLOCK_OPT_SIZE, o_cow->virt_size));
Assertions should not have side effects.
what side effects would this line have?
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 <xiaw...@linux.vnet.ibm.com>
+ *
+ * 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
+
+#define QB_ERR_MEM_ERR (-1)
+#define QB_ERR_INTERNAL_ERR (-2)
+#define QB_ERR_INVALID_PARAM (-3)
+
+/* 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);
+
+
+/* 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
I'd rather avoid exposing this for now.
+/* 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
NATIVE_AIO should be an option for the file protocol. But it's mostly
for performance and since we only support synchronous I/O we can drop it
for now.
+/* 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
I'd rather avoid exposing this for now too.
+/* consistency hint for incoming migration */
+#define LIBQBLOCK_O_INCOMING 0x0800
This is internal to QEMU.
Please add a mask of valid bits and an assertion that unknown bits are zero.
OK to hide this flag now. Do you think this flag belongs to the
control flag or I should sort them to anther option? In long term the
library should provide full capablities as qemu block layer have so
qemu can try migrate to it, so now I need to make sure the API designed
have left room for all features qemu have support.
+
+#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
+};
+
+/* 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;
+};
+
+/* 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 */
+};
You are right that the embedded structs are very complicated. I think
we need to find the right balance between structs for extensibility and
function arguments for ease of use.
For example, we could have
int qb_open(struct QBlockState *qbs,
struct QBlockLocation *loc,
struct QBlockFormatOption *op,
int o_flag);
where:
- QBlockLocation is basically your QBlockLocInfo, but restructured to
use unions for the protocol-specific fields.
- QBlockFormatOption is basically your QBlockOptionFormat struct. Thus
we can plan for specifying format-specific options at open time rather
than just at create time (this can be useful, for example, for things
such as lazy_refcounts). Until support is there in the block layer, we
can simply check that all format-specific options are zero.
Since both QBlockLocation and QBlockFormatOption are basically a
discriminated record (enum + union of structs) we can add a large char
member to the union (e.g. char padding[512]) to keep the ABI stable.
Users must zero out all fields, and future additions must ensure that
the default value is represented by all-zeros.
+ const char *prealloc_mode; /* off or metadata */
Make this an enum.
+ bool prealloc_mode;
Here too.
+};
+
+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} */
Here too.
+/* 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 {
QBlockImageInfo
+ int struct_size;
Here the struct size makes more sense, because it's a returned struct.
+ char *filename;
+ enum QBlockProtocol protocol;
Can this just be a struct QBlockLocation * (using a pointer avoids
problems with embedded structs)?
+ enum QBlockFormat format;
Can this just be a struct QBlockFormatOption * (same as the above)?
Yes, I tried it before but found that structure alignment is hard to
solve, but as you suggest, I think plain structure plus pointer could
make it simpler. Blue Swirl suggest using a API to unify property
setting and solve version difference, not using structure, as:
qb_set_property(s, "filename", "c:\\\\autoexec.bat");
What do you think of that way?
+ size_t virt_size;
+ /* advance info */
+ size_t allocated_size;
+ bool encrypt;
+ char *backing_filename;
+};
+
+/* 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);
qb_get_image_info
OK.
+
+/*
+ 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);
qb_free_image_info
Paolo
+
+/* 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);
+#endif
--
Best Regards
Wenchao Xia