Re: [Qemu-block] [PATCH v19 09/10] tests: add unit test case for replication
On 05/31/2016 01:34 AM, Stefan Hajnoczi wrote:
On Fri, May 20, 2016 at 03:36:19PM +0800, Changlong Xie wrote:
+/* primary */
+#define P_LOCAL_DISK "/tmp/p_local_disk.XX"
+#define P_COMMAND "driver=replication,mode=primary,node-name=xxx,"\
+ "file.driver=qcow2,file.file.filename="P_LOCAL_DISK
+
+/* secondary */
+#define S_LOCAL_DISK "/tmp/s_local_disk.XX"
+#define S_ACTIVE_DISK "/tmp/s_active_disk.XX"
+#define S_HIDDEN_DISK "/tmp/s_hidden_disk.XX"
Please use unique filenames so that multiple instances of the test can
run in parallel on a single machine. mkstemp(3) can be used to do this.
will fix in next version.
+static void io_read(BlockDriverState *bs, long pattern, int64_t pattern_offset,
+int64_t pattern_count, int64_t offset, int64_t count,
+bool expect_failed)
+{
+char *buf;
+void *cmp_buf;
+int ret;
+
+/* 1. alloc pattern buffer */
+if (pattern) {
+cmp_buf = g_malloc(pattern_count);
+memset(cmp_buf, pattern, pattern_count);
+}
+
+/* 2. alloc read buffer */
+buf = qemu_blockalign(bs, count);
+memset(buf, 0xab, count);
+
+/* 3. do read */
+ret = bdrv_read(bs, offset >> 9, (uint8_t *)buf, count >> 9);
+
+/* 4. assert and compare buf */
+if (expect_failed) {
+g_assert(ret < 0);
+} else {
+g_assert(ret >= 0);
+if (pattern) {
+g_assert(memcmp(buf + pattern_offset, cmp_buf, pattern_count) <=
0);
+g_free(cmp_buf);
if pattern && expect_failed then cmp_buf is leaked. Probably best to
initialize cmp_buf = NULL and have an unconditional g_free(cmp_buf) at
the end of the function to avoid leaks.
Yes, you are right.
+}
+}
+g_free(buf);
qemu_blockalign() memory is freed with qemu_vfree(), not g_free().
will fix
+static void test_primary_do_checkpoint(void)
+{
+BlockDriverState *bs;
+Error *local_err = NULL;
+
+bs = start_primary();
+
+replication_do_checkpoint_all(&local_err);
+g_assert(!local_err);
+
+teardown_primary(bs);
+}
Shouldn't replication_start_all() be called before
replication_do_checkpoint_all()?
It seems i missed it.
+int main(int argc, char **argv)
+{
+int ret;
+qemu_init_main_loop(&error_fatal);
+bdrv_init();
+
+do {} while (g_main_context_iteration(NULL, false));
Why is this necessary?
Will remove it.
Re: [Qemu-block] [PATCH v19 09/10] tests: add unit test case for replication
On Fri, May 20, 2016 at 03:36:19PM +0800, Changlong Xie wrote:
> +/* primary */
> +#define P_LOCAL_DISK "/tmp/p_local_disk.XX"
> +#define P_COMMAND "driver=replication,mode=primary,node-name=xxx,"\
> + "file.driver=qcow2,file.file.filename="P_LOCAL_DISK
> +
> +/* secondary */
> +#define S_LOCAL_DISK "/tmp/s_local_disk.XX"
> +#define S_ACTIVE_DISK "/tmp/s_active_disk.XX"
> +#define S_HIDDEN_DISK "/tmp/s_hidden_disk.XX"
Please use unique filenames so that multiple instances of the test can
run in parallel on a single machine. mkstemp(3) can be used to do this.
> +static void io_read(BlockDriverState *bs, long pattern, int64_t
> pattern_offset,
> +int64_t pattern_count, int64_t offset, int64_t count,
> +bool expect_failed)
> +{
> +char *buf;
> +void *cmp_buf;
> +int ret;
> +
> +/* 1. alloc pattern buffer */
> +if (pattern) {
> +cmp_buf = g_malloc(pattern_count);
> +memset(cmp_buf, pattern, pattern_count);
> +}
> +
> +/* 2. alloc read buffer */
> +buf = qemu_blockalign(bs, count);
> +memset(buf, 0xab, count);
> +
> +/* 3. do read */
> +ret = bdrv_read(bs, offset >> 9, (uint8_t *)buf, count >> 9);
> +
> +/* 4. assert and compare buf */
> +if (expect_failed) {
> +g_assert(ret < 0);
> +} else {
> +g_assert(ret >= 0);
> +if (pattern) {
> +g_assert(memcmp(buf + pattern_offset, cmp_buf, pattern_count) <=
> 0);
> +g_free(cmp_buf);
if pattern && expect_failed then cmp_buf is leaked. Probably best to
initialize cmp_buf = NULL and have an unconditional g_free(cmp_buf) at
the end of the function to avoid leaks.
> +}
> +}
> +g_free(buf);
qemu_blockalign() memory is freed with qemu_vfree(), not g_free().
> +static void test_primary_do_checkpoint(void)
> +{
> +BlockDriverState *bs;
> +Error *local_err = NULL;
> +
> +bs = start_primary();
> +
> +replication_do_checkpoint_all(&local_err);
> +g_assert(!local_err);
> +
> +teardown_primary(bs);
> +}
Shouldn't replication_start_all() be called before
replication_do_checkpoint_all()?
> +int main(int argc, char **argv)
> +{
> +int ret;
> +qemu_init_main_loop(&error_fatal);
> +bdrv_init();
> +
> +do {} while (g_main_context_iteration(NULL, false));
Why is this necessary?
signature.asc
Description: PGP signature
Re: [Qemu-block] [PATCH v19 09/10] tests: add unit test case for replication
On 05/20/2016 03:36 PM, Changlong Xie wrote:
+static void io_write(BlockDriverState *bs, long pattern, int64_t pattern_count,
+ int64_t offset, int64_t count, bool expect_failed)
+{
+void *pattern_buf;
Should initialize as NULL to avoid below warnning:
tests/test-replication.c:104:15: error: ‘pattern_buf’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
g_free(pattern_buf);
Will fix in next version.
+int ret;
+
+/* 1. alloc pattern buffer */
+if (pattern) {
+pattern_buf = g_malloc(pattern_count);
+memset(pattern_buf, pattern, pattern_count);
+}
+
+/* 2. do write */
+if (pattern) {
+ret = bdrv_write(bs, offset >> 9, (uint8_t *)pattern_buf, count >> 9);
+} else {
+ret = bdrv_write_zeroes(bs, offset >> 9, count >> 9, 0);
+}
+
+/* 3. assert */
+if (expect_failed) {
+g_assert(ret < 0);
+} else {
+g_assert(ret >= 0);
+g_free(pattern_buf);
+}
+}
