From patchwork Fri Oct 11 07:43:22 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fam Zheng X-Patchwork-Id: 282544 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 9A9A52C03FD for ; Fri, 11 Oct 2013 18:44:13 +1100 (EST) Received: from localhost ([::1]:52890 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VUXON-0006cO-9e for incoming@patchwork.ozlabs.org; Fri, 11 Oct 2013 03:44:11 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51499) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VUXNp-0006Xf-8u for qemu-devel@nongnu.org; Fri, 11 Oct 2013 03:43:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VUXNj-0006Px-7P for qemu-devel@nongnu.org; Fri, 11 Oct 2013 03:43:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55247) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VUXNi-0006Pr-Si for qemu-devel@nongnu.org; Fri, 11 Oct 2013 03:43:31 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r9B7hU36010012 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 11 Oct 2013 03:43:30 -0400 Received: from T430s.nay.redhat.com ([10.66.6.118]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r9B7hOP9027981; Fri, 11 Oct 2013 03:43:28 -0400 From: Fam Zheng To: qemu-devel@nongnu.org Date: Fri, 11 Oct 2013 15:43:22 +0800 Message-Id: <1381477403-25759-2-git-send-email-famz@redhat.com> In-Reply-To: <1381477403-25759-1-git-send-email-famz@redhat.com> References: <1381477403-25759-1-git-send-email-famz@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, stefanha@redhat.com Subject: [Qemu-devel] [PATCH v3 1/2] vmdk: convert error code to use errp X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Convert "fprintf(stderr,..." and standardize error messages: Remove a few local_error's and use errp. Remove "VMDK:" or "Vmdk:" prefixes in error message and fix to upper case. Signed-off-by: Fam Zheng --- block/vmdk.c | 116 ++++++++++++++++++++++++--------------------- tests/qemu-iotests/059.out | 6 +-- 2 files changed, 63 insertions(+), 59 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 5d56e31..4f8ae77 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -331,8 +331,7 @@ static int vmdk_reopen_prepare(BDRVReopenState *state, assert(state->bs != NULL); if (queue == NULL) { - error_set(errp, ERROR_CLASS_GENERIC_ERROR, - "No reopen queue for VMDK extents"); + error_setg(errp, "No reopen queue for VMDK extents"); goto exit; } @@ -391,22 +390,23 @@ static int vmdk_add_extent(BlockDriverState *bs, int64_t l1_offset, int64_t l1_backup_offset, uint32_t l1_size, int l2_size, uint64_t cluster_sectors, - VmdkExtent **new_extent) + VmdkExtent **new_extent, + Error **errp) { VmdkExtent *extent; BDRVVmdkState *s = bs->opaque; if (cluster_sectors > 0x200000) { /* 0x200000 * 512Bytes = 1GB for one cluster is unrealistic */ - error_report("invalid granularity, image may be corrupt"); - return -EINVAL; + error_setg(errp, "Invalid granularity, image may be corrupt"); + return -EFBIG; } if (l1_size > 512 * 1024 * 1024) { /* Although with big capacity and small l1_entry_sectors, we can get a * big l1_size, we don't want unbounded value to allocate the table. * Limit it to 512M, which is 16PB for default cluster and L2 table * size */ - error_report("L1 size too big"); + error_setg(errp, "L1 size too big"); return -EFBIG; } @@ -438,7 +438,8 @@ static int vmdk_add_extent(BlockDriverState *bs, return 0; } -static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) +static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent, + Error **errp) { int ret; int l1_size, i; @@ -447,10 +448,13 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) l1_size = extent->l1_size * sizeof(uint32_t); extent->l1_table = g_malloc(l1_size); ret = bdrv_pread(extent->file, - extent->l1_table_offset, - extent->l1_table, - l1_size); + extent->l1_table_offset, + extent->l1_table, + l1_size); if (ret < 0) { + error_setg_errno(errp, -ret, + "Could not read l1 table from extent '%s'", + extent->file->filename); goto fail_l1; } for (i = 0; i < extent->l1_size; i++) { @@ -460,10 +464,13 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) if (extent->l1_backup_table_offset) { extent->l1_backup_table = g_malloc(l1_size); ret = bdrv_pread(extent->file, - extent->l1_backup_table_offset, - extent->l1_backup_table, - l1_size); + extent->l1_backup_table_offset, + extent->l1_backup_table, + l1_size); if (ret < 0) { + error_setg_errno(errp, -ret, + "Could not read l1 backup table from extent '%s'", + extent->file->filename); goto fail_l1b; } for (i = 0; i < extent->l1_size; i++) { @@ -483,7 +490,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) static int vmdk_open_vmfs_sparse(BlockDriverState *bs, BlockDriverState *file, - int flags) + int flags, Error **errp) { int ret; uint32_t magic; @@ -492,6 +499,9 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); if (ret < 0) { + error_setg_errno(errp, -ret, + "Could not read header from file '%s'", + file->filename); return ret; } ret = vmdk_add_extent(bs, file, false, @@ -501,11 +511,12 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, le32_to_cpu(header.l1dir_size), 4096, le32_to_cpu(header.granularity), - &extent); + &extent, + errp); if (ret < 0) { return ret; } - ret = vmdk_init_tables(bs, extent); + ret = vmdk_init_tables(bs, extent, errp); if (ret) { /* free extent allocated by vmdk_add_extent */ vmdk_free_last_extent(bs); @@ -514,11 +525,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, - uint64_t desc_offset); + uint64_t desc_offset, Error **errp); static int vmdk_open_vmdk4(BlockDriverState *bs, BlockDriverState *file, - int flags) + int flags, Error **errp) { int ret; uint32_t magic; @@ -529,12 +540,14 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); if (ret < 0) { - return ret; + error_setg_errno(errp, -ret, + "Could not read header from file '%s'", + file->filename); } if (header.capacity == 0) { uint64_t desc_offset = le64_to_cpu(header.desc_offset); if (desc_offset) { - return vmdk_open_desc_file(bs, flags, desc_offset << 9); + return vmdk_open_desc_file(bs, flags, desc_offset << 9, errp); } } @@ -616,7 +629,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, l1_size, le32_to_cpu(header.num_gtes_per_gt), le64_to_cpu(header.granularity), - &extent); + &extent, + errp); if (ret < 0) { return ret; } @@ -625,7 +639,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, extent->has_marker = le32_to_cpu(header.flags) & VMDK4_FLAG_MARKER; extent->version = le32_to_cpu(header.version); extent->has_zero_grain = le32_to_cpu(header.flags) & VMDK4_FLAG_ZERO_GRAIN; - ret = vmdk_init_tables(bs, extent); + ret = vmdk_init_tables(bs, extent, errp); if (ret) { /* free extent allocated by vmdk_add_extent */ vmdk_free_last_extent(bs); @@ -663,7 +677,7 @@ static int vmdk_parse_description(const char *desc, const char *opt_name, /* Open an extent file and append to bs array */ static int vmdk_open_sparse(BlockDriverState *bs, BlockDriverState *file, - int flags) + int flags, Error **errp) { uint32_t magic; @@ -674,10 +688,10 @@ static int vmdk_open_sparse(BlockDriverState *bs, magic = be32_to_cpu(magic); switch (magic) { case VMDK3_MAGIC: - return vmdk_open_vmfs_sparse(bs, file, flags); + return vmdk_open_vmfs_sparse(bs, file, flags, errp); break; case VMDK4_MAGIC: - return vmdk_open_vmdk4(bs, file, flags); + return vmdk_open_vmdk4(bs, file, flags, errp); break; default: return -EMEDIUMTYPE; @@ -686,7 +700,7 @@ static int vmdk_open_sparse(BlockDriverState *bs, } static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, - const char *desc_file_path) + const char *desc_file_path, Error **errp) { int ret; char access[11]; @@ -697,7 +711,6 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, int64_t flat_offset; char extent_path[PATH_MAX]; BlockDriverState *extent_file; - Error *local_err = NULL; while (*p) { /* parse extent line: @@ -712,9 +725,11 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, goto next_line; } else if (!strcmp(type, "FLAT")) { if (ret != 5 || flat_offset < 0) { + error_setg(errp, "Invalid extent lines: \n%s", p); return -EINVAL; } } else if (ret != 4) { + error_setg(errp, "Invalid extent lines: \n%s", p); return -EINVAL; } @@ -728,10 +743,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, path_combine(extent_path, sizeof(extent_path), desc_file_path, fname); ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags, - &local_err); + errp); if (ret) { - qerror_report_err(local_err); - error_free(local_err); return ret; } @@ -741,21 +754,20 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, VmdkExtent *extent; ret = vmdk_add_extent(bs, extent_file, true, sectors, - 0, 0, 0, 0, 0, &extent); + 0, 0, 0, 0, 0, &extent, errp); if (ret < 0) { return ret; } extent->flat_start_offset = flat_offset << 9; } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) { /* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse file*/ - ret = vmdk_open_sparse(bs, extent_file, bs->open_flags); + ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, errp); if (ret) { bdrv_unref(extent_file); return ret; } } else { - fprintf(stderr, - "VMDK: Not supported extent type \"%s\""".\n", type); + error_setg(errp, "Unsupported extent type '%s'", type); return -ENOTSUP; } next_line: @@ -769,7 +781,7 @@ next_line: } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, - uint64_t desc_offset) + uint64_t desc_offset, Error **errp) { int ret; char *buf = NULL; @@ -798,13 +810,12 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, strcmp(ct, "vmfsSparse") && strcmp(ct, "twoGbMaxExtentSparse") && strcmp(ct, "twoGbMaxExtentFlat")) { - fprintf(stderr, - "VMDK: Not supported image type \"%s\""".\n", ct); + error_setg(errp, "Unsupported image type '%s'", ct); ret = -ENOTSUP; goto exit; } s->desc_offset = 0; - ret = vmdk_parse_extents(buf, bs, bs->file->filename); + ret = vmdk_parse_extents(buf, bs, bs->file->filename, errp); exit: g_free(buf); return ret; @@ -816,10 +827,10 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, int ret; BDRVVmdkState *s = bs->opaque; - if (vmdk_open_sparse(bs, bs->file, flags) == 0) { + if (vmdk_open_sparse(bs, bs->file, flags, errp) == 0) { s->desc_offset = 0x200; } else { - ret = vmdk_open_desc_file(bs, flags, 0); + ret = vmdk_open_desc_file(bs, flags, 0, errp); if (ret) { goto fail; } @@ -1286,8 +1297,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num, VmdkMetaData m_data; if (sector_num > bs->total_sectors) { - fprintf(stderr, - "(VMDK) Wrong offset: sector_num=0x%" PRIx64 + error_report("Wrong offset: sector_num=0x%" PRIx64 " total_sectors=0x%" PRIx64 "\n", sector_num, bs->total_sectors); return -EIO; @@ -1307,9 +1317,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num, if (extent->compressed) { if (ret == VMDK_OK) { /* Refuse write to allocated cluster for streamOptimized */ - fprintf(stderr, - "VMDK: can't write to allocated cluster" - " for streamOptimized\n"); + error_report("Could not write to allocated cluster" + " for streamOptimized"); return -EIO; } else { /* allocate */ @@ -1517,12 +1526,12 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, } static int filename_decompose(const char *filename, char *path, char *prefix, - char *postfix, size_t buf_len) + char *postfix, size_t buf_len, Error **errp) { const char *p, *q; if (filename == NULL || !strlen(filename)) { - fprintf(stderr, "Vmdk: no filename provided.\n"); + error_setg(errp, "Vmdk: no filename provided"); return VMDK_ERROR; } p = strrchr(filename, '/'); @@ -1595,9 +1604,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, "ddb.geometry.heads = \"%d\"\n" "ddb.geometry.sectors = \"63\"\n" "ddb.adapterType = \"%s\"\n"; - Error *local_err = NULL; - if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) { + if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp)) { return -EINVAL; } /* Read out options */ @@ -1623,7 +1631,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, strcmp(adapter_type, "buslogic") && strcmp(adapter_type, "lsilogic") && strcmp(adapter_type, "legacyESX")) { - fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type); + error_setg(errp, "Unknown adapter type: '%s'", adapter_type); return -EINVAL; } if (strcmp(adapter_type, "ide") != 0) { @@ -1639,7 +1647,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, strcmp(fmt, "twoGbMaxExtentSparse") && strcmp(fmt, "twoGbMaxExtentFlat") && strcmp(fmt, "streamOptimized")) { - fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt); + error_setg(errp, "Unknown subformat: '%s'", fmt); return -EINVAL; } split = !(strcmp(fmt, "twoGbMaxExtentFlat") && @@ -1653,15 +1661,13 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, desc_extent_line = "RW %lld SPARSE \"%s\"\n"; } if (flat && backing_file) { - /* not supporting backing file for flat image */ + error_setg(errp, "Flat image can't have backing file"); return -ENOTSUP; } if (backing_file) { BlockDriverState *bs = bdrv_new(""); - ret = bdrv_open(bs, backing_file, NULL, 0, NULL, &local_err); + ret = bdrv_open(bs, backing_file, NULL, 0, NULL, errp); if (ret != 0) { - qerror_report_err(local_err); - error_free(local_err); bdrv_unref(bs); return ret; } diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 9159dbe..dc4f024 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -2,8 +2,7 @@ QA output created by 059 === Testing invalid granularity === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 -invalid granularity, image may be corrupt -qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk': Wrong medium type +qemu-io: can't open device TEST_DIR/t.vmdk: Invalid granularity, image may be corrupt no file open, try 'help open' === Testing too big L2 table size === @@ -14,8 +13,7 @@ no file open, try 'help open' === Testing too big L1 table size === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 -L1 size too big -qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk': Wrong medium type +qemu-io: can't open device TEST_DIR/t.vmdk: L1 size too big no file open, try 'help open' === Testing monolithicFlat creation and opening ===