From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52582) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Szkjo-0007lo-Dr for qemu-devel@nongnu.org; Fri, 10 Aug 2012 04:38:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Szkjk-0000TP-MB for qemu-devel@nongnu.org; Fri, 10 Aug 2012 04:38:32 -0400 Received: from e23smtp04.au.ibm.com ([202.81.31.146]:34295) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Szkjj-0000TB-Sf for qemu-devel@nongnu.org; Fri, 10 Aug 2012 04:38:28 -0400 Received: from /spool/local by e23smtp04.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 10 Aug 2012 18:37:28 +1000 Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7A8TtBT23789702 for ; Fri, 10 Aug 2012 18:29:55 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7A8cMYu016832 for ; Fri, 10 Aug 2012 18:38:22 +1000 Message-ID: <5024C876.3040905@linux.vnet.ibm.com> Date: Fri, 10 Aug 2012 16:38:14 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1344507151-11217-1-git-send-email-xiawenc@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [qemu-devel] [PATCH V2 3/3] [RFC] libqblock-test case. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: stefanha@gmail.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, pbonzini@redhat.com 于 2012-8-10 1:49, Blue Swirl 写道: > On Thu, Aug 9, 2012 at 10:12 AM, Wenchao Xia wrote: >> This file simulate the caller to test the library. >> >> Signed-off-by: Wenchao Xia >> --- >> libqblock-test.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > Please move this to test/ and build with other tests. > Once the API is close to stable I will package the library and move the test case to where it should be. >> 1 files changed, 197 insertions(+), 0 deletions(-) >> create mode 100644 libqblock-test.c >> >> diff --git a/libqblock-test.c b/libqblock-test.c >> new file mode 100644 >> index 0000000..6198924 >> --- /dev/null >> +++ b/libqblock-test.c >> @@ -0,0 +1,197 @@ >> +#include "libqblock.h" > > Move below system includes. > OK. Fix are also done to following comments. >> + >> +#include >> +#include >> +#include >> +#include >> + >> +static unsigned char buf0[1024]; >> +static unsigned char buf1[1024] = {4, 0, 0, 2}; > > const > >> + >> + >> +static int qbi_print_test(struct QBlockInfoImage *info) >> +{ >> + printf("name:%s, protocol %d, format %d, virt_size %" PRId64 " " >> + " allocated_size %" PRId64 >> + " encrypt %d, backing file %s.\n", >> + info->filename, info->protocol, info->format, info->virt_size, >> + info->allocated_size, >> + info->encrypt, info->backing_filename); >> + return 0; > > The return value does not seem useful, just make the function return void. > >> +} >> + >> + >> +int main(int argc, char **argv) >> +{ >> + struct QBlockState *qbs = NULL; >> + struct QBlockOptionOpen *qboo = NULL; >> + struct QBlockOptionCreate *qboc = NULL; >> + struct QBlockInfoImage *qbi = NULL; >> + char *filename1, *filename2; >> + int ret, err_no; >> + const char *err_msg = NULL; >> + >> + int i; >> + unsigned long op_size = 512; >> + unsigned long op_start = 1024; >> + >> + if (argc < 3) { >> + printf("usage: libqblock-test [filename1] [filename2].\n"); > > fprintf(stderr, > >> + return 0; > > return 1; > >> + } >> + filename1 = argv[1]; >> + printf("qemu test, file name1 is %s.\n", filename1); >> + filename2 = argv[2]; >> + printf("qemu test, file name2 is %s.\n", filename2); > > QEMU libblock-test, just print both file names together. > >> + >> + libqblock_init(); >> + >> + ret = qb_state_new(&qbs); >> + if (ret < 0) { >> + goto free; >> + } >> + >> + ret = qb_oc_new(&qboc, QB_FMT_QCOW); >> + if (ret < 0) { >> + goto free; >> + } >> + >> + qboc->o_loc.filename = filename1; >> + qboc->o_loc.protocol = QB_PROTO_FILE; >> + qboc->o_fmt.fmt_type = QB_FMT_QCOW; >> + qboc->o_fmt.fmt_op.o_qcow.virt_size = 128 * 1024 * 1024; > > A smaller file should be enough for the tests. > >> + >> + ret = qb_create(qbs, qboc); >> + if (ret < 0) { >> + printf("failed to create image, ret is %d.\n", ret); >> + if (ret == QB_ERR_INTERNAL_ERR) { >> + err_msg = qb_error_get_detail(qbs, &err_no); >> + printf("errno is %d, msg is %s.\n", err_no, err_msg); >> + } > > This snippet is repeated below, please refactor. Error messages should > go to stderr. > >> + goto free; >> + } >> + >> + qboc->o_loc.filename = filename2; >> + qboc->o_loc.protocol = QB_PROTO_FILE; >> + qboc->o_fmt.fmt_type = QB_FMT_QCOW2; >> + qboc->o_fmt.fmt_op.o_qcow2.backing_file = filename1; >> + >> + ret = qb_create(qbs, qboc); >> + if (ret < 0) { >> + printf("failed to create image, ret is %d.\n", ret); >> + if (ret == QB_ERR_INTERNAL_ERR) { >> + err_msg = qb_error_get_detail(qbs, &err_no); >> + printf("errno is %d, msg is %s.\n", err_no, err_msg); >> + } >> + goto free; >> + } >> + >> + ret = qb_oo_new(&qboo); >> + if (ret < 0) { >> + goto unlink; >> + } >> + >> + qboo->o_loc.filename = filename2; >> + qboo->o_loc.protocol = QB_PROTO_FILE; >> + qboo->o_fmt_type = QB_FMT_QCOW2; >> + qboo->o_flag = LIBQBLOCK_O_RDWR; >> + >> + ret = qb_open(qbs, qboo); >> + if (ret < 0) { >> + printf("failed to open image, ret is %d.\n", ret); >> + if (ret == QB_ERR_INTERNAL_ERR) { >> + err_msg = qb_error_get_detail(qbs, &err_no); >> + printf("errno is %d, msg is %s.\n", err_no, err_msg); >> + } >> + goto unlink; >> + } >> + >> + ret = qb_write(qbs, buf1, op_size, op_start); >> + if (ret < 0) { >> + printf("failed to write image, ret is %d.\n", ret); >> + if (ret == QB_ERR_INTERNAL_ERR) { >> + err_msg = qb_error_get_detail(qbs, &err_no); >> + printf("errno is %d, msg is %s.\n", err_no, err_msg); >> + } >> + goto close; >> + } >> + ret = qb_read(qbs, buf0, op_size, op_start); >> + if (ret < 0) { >> + printf("failed to read image, ret is %d.\n", ret); >> + if (ret == QB_ERR_INTERNAL_ERR) { >> + err_msg = qb_error_get_detail(qbs, &err_no); >> + printf("errno is %d, msg is %s.\n", err_no, err_msg); >> + } >> + goto close; >> + } >> + >> + for (i = 0; i < op_size; i++) { >> + if (buf0[i] != buf1[i]) { > > Just use memcmp(). > >> + printf("mismatch found at %d.\n", i); >> + break; >> + } >> + } >> + >> + /* check backing chain */ >> + ret = qb_infoimage_get(qbs, &qbi); >> + if (ret < 0) { >> + printf("failed to get image info, ret is %d.\n", ret); >> + if (ret == QB_ERR_INTERNAL_ERR) { >> + err_msg = qb_error_get_detail(qbs, &err_no); >> + printf("errno is %d, msg is %s.\n", err_no, err_msg); >> + } >> + goto close; >> + } >> + qbi_print_test(qbi); >> + >> + while (qbi->backing_filename != NULL) { >> + qb_close(qbs); >> + qboo->o_loc.filename = qbi->backing_filename; >> + qboo->o_loc.protocol = QB_PROTO_FILE; >> + qboo->o_fmt_type = QB_FMT_NONE; >> + qboo->o_flag = 0; >> + ret = qb_open(qbs, qboo); >> + if (ret < 0) { >> + printf("failed to open back image %s, ret is %d.\n", >> + qbi->backing_filename, ret); >> + if (ret == QB_ERR_INTERNAL_ERR) { >> + err_msg = qb_error_get_detail(qbs, &err_no); >> + printf("errno is %d, msg is %s.\n", err_no, err_msg); >> + } >> + goto close; >> + } >> + qb_infoimage_free(&qbi); >> + ret = qb_infoimage_get(qbs, &qbi); >> + if (ret < 0) { >> + printf("failed to get image info, ret is %d.\n", ret); >> + if (ret == QB_ERR_INTERNAL_ERR) { >> + err_msg = qb_error_get_detail(qbs, &err_no); >> + printf("errno is %d, msg is %s.\n", err_no, err_msg); >> + } >> + goto close; >> + } >> + qbi_print_test(qbi); >> + } >> + >> + printf("test done.\n"); >> + >> + close: >> + qb_close(qbs); >> + unlink: >> + unlink(filename1); >> + unlink(filename2); >> + free: >> + if (qbs != NULL) { >> + qb_state_free(&qbs); >> + } >> + if (qboo != NULL) { >> + qb_oo_free(&qboo); >> + } >> + if (qboc != NULL) { >> + qb_oc_free(&qboc); >> + } >> + if (qbi != NULL) { >> + qb_infoimage_free(&qbi); >> + } >> + return 0; > > Please exit with nonzero value if there has been any errors. > >> +} >> -- >> 1.7.1 >> >> >> > -- Best Regards Wenchao Xia