* Re: [Qemu-devel] [PATCH] block/vmdk: add basic .bdrv_check support
[not found] <1389612246-9545-1-git-send-email-pl@kamp.de>
@ 2014-01-13 13:39 ` Fam Zheng
2014-01-27 15:37 ` Stefan Hajnoczi
1 sibling, 0 replies; 3+ messages in thread
From: Fam Zheng @ 2014-01-13 13:39 UTC (permalink / raw)
To: Peter Lieven; +Cc: kwolf, qemu-devel, stefanha
On Mon, 01/13 12:24, Peter Lieven wrote:
> this adds a basic vmdk corruption check. it should detect severe
> table corruptions and file truncation.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> block/vmdk.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index c6b60b4..1d66858 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1918,6 +1918,51 @@ static ImageInfo *vmdk_get_extent_info(VmdkExtent *extent)
> return info;
> }
>
> +static int vmdk_check(BlockDriverState *bs, BdrvCheckResult *result,
> + BdrvCheckMode fix)
> +{
> + BDRVVmdkState *s = bs->opaque;
> + VmdkExtent *extent = NULL;
> + int64_t sector_num = 0;
> + int64_t total_sectors = bdrv_getlength(bs) / BDRV_SECTOR_SIZE;
> + int ret;
> + uint64_t cluster_offset;
> +
> + if (fix) {
> + return -ENOTSUP;
> + }
> +
> + for (;;) {
> + if (sector_num >= total_sectors) {
> + return 0;
> + }
> + extent = find_extent(s, sector_num, extent);
> + if (!extent) {
> + fprintf(stderr, "ERROR: could not find extend for sector %ld\n",
> + sector_num);
> + break;
> + }
> + ret = get_cluster_offset(bs, extent, NULL, sector_num << BDRV_SECTOR_BITS,
> + 0, &cluster_offset);
> + if (ret == VMDK_ERROR) {
> + fprintf(stderr,
> + "ERROR: could not get cluster_offset for sector %ld\n",
> + sector_num);
> + break;
> + }
> + if (ret == VMDK_OK && cluster_offset >= bdrv_getlength(extent->file)) {
> + fprintf(stderr,
> + "ERROR: cluster offset for sector %ld points after EOF\n",
> + sector_num);
> + break;
> + }
> + sector_num += extent->cluster_sectors;
> + }
> +
> + result->corruptions++;
> + return 0;
> +}
> +
> static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs)
> {
> int i;
> @@ -1991,6 +2036,7 @@ static BlockDriver bdrv_vmdk = {
> .instance_size = sizeof(BDRVVmdkState),
> .bdrv_probe = vmdk_probe,
> .bdrv_open = vmdk_open,
> + .bdrv_check = vmdk_check,
> .bdrv_reopen_prepare = vmdk_reopen_prepare,
> .bdrv_read = vmdk_co_read,
> .bdrv_write = vmdk_co_write,
Works for me. Thank.
And BTW, is it worth comparing with the "grain_offset" header field? This
should be the first data cluster's offset. When you have a fresh image with no
data cluster allocated, and it's truncated, this patch doesn't catch it. Anyway,
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] block/vmdk: add basic .bdrv_check support
[not found] <1389612246-9545-1-git-send-email-pl@kamp.de>
2014-01-13 13:39 ` [Qemu-devel] [PATCH] block/vmdk: add basic .bdrv_check support Fam Zheng
@ 2014-01-27 15:37 ` Stefan Hajnoczi
2014-01-27 18:30 ` Peter Lieven
1 sibling, 1 reply; 3+ messages in thread
From: Stefan Hajnoczi @ 2014-01-27 15:37 UTC (permalink / raw)
To: Peter Lieven; +Cc: kwolf, famz, qemu-devel
On Mon, Jan 13, 2014 at 12:24:06PM +0100, Peter Lieven wrote:
> + for (;;) {
> + if (sector_num >= total_sectors) {
> + return 0;
> + }
> + extent = find_extent(s, sector_num, extent);
> + if (!extent) {
> + fprintf(stderr, "ERROR: could not find extend for sector %ld\n",
> + sector_num);
s/extend/extent/
Please use PRId64 instead of %ld so this works on 32-bit hosts.
> + break;
> + }
> + ret = get_cluster_offset(bs, extent, NULL, sector_num << BDRV_SECTOR_BITS,
> + 0, &cluster_offset);
> + if (ret == VMDK_ERROR) {
> + fprintf(stderr,
> + "ERROR: could not get cluster_offset for sector %ld\n",
PRId64
> + sector_num);
> + break;
> + }
> + if (ret == VMDK_OK && cluster_offset >= bdrv_getlength(extent->file)) {
> + fprintf(stderr,
> + "ERROR: cluster offset for sector %ld points after EOF\n",
PRId64
^ permalink raw reply [flat|nested] 3+ messages in thread