On Sat, Jun 04, 2011 at 08:40:22AM +0800, Fam Zheng wrote: > fail: > - qemu_free(s->l1_backup_table); > - qemu_free(s->l1_table); > - qemu_free(s->l2_cache); > + if(s->extents) { > + qemu_free(s->extents[0].l1_backup_table); > + qemu_free(s->extents[0].l1_table); > + qemu_free(s->extents[0].l2_cache); > + }
for (i = 0; i < s->num_extents; i++) { qemu_free(s->extents[i].l1_backup_table); qemu_free(s->extents[i].l1_table); qemu_free(s->extents[i].l2_cache); } qemu_free(s->extents); > +static int find_extent(BDRVVmdkState *s, int64_t sector_num, int start_idx) > +{ > + int ext_idx = start_idx; > + while (ext_idx < s->num_extents > + && sector_num >= s->extents[ext_idx].sectors) { > + sector_num -= s->extents[ext_idx].sectors; > + ext_idx++; > + } > + if (ext_idx == s->num_extents) return -1; > + return ext_idx; > +} Callers don't really need the index, they just want the extent and an optimized way of repeated calls to avoid O(N^2) find_extent() times: static VmdkExtent *find_extent(BDRVVmdkState *s, int64_t sector_num, VmdkExtent *start_extent) { VmdkExtent *extent = start_extent; if (!start_extent) { extent = &s->extent[0]; } while (extent < &s->extents[s->num_extents]) { if (sector_num < extent->end) { return extent; } extent++; } return NULL; } I added the VmdkExtent.end field so that this function can be called repeatedly for an increasing series of sector_num values. It seems like your code would fail when called with a non-0 index since sector_num -= s->extents[ext_idx].sectors is incorrect when starting from an arbitrary extent_idx. > + > static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num, > int nb_sectors, int *pnum) > { > BDRVVmdkState *s = bs->opaque; > - int index_in_cluster, n; > - uint64_t cluster_offset; > > - cluster_offset = get_cluster_offset(bs, NULL, sector_num << 9, 0); > - index_in_cluster = sector_num % s->cluster_sectors; > - n = s->cluster_sectors - index_in_cluster; > + int index_in_cluster, n, ret; > + uint64_t offset; > + VmdkExtent *extent; > + int ext_idx; > + > + ext_idx = find_extent(s, sector_num, 0); > + if (ext_idx == -1) return 0; > + extent = &s->extents[ext_idx]; > + if (s->extents[ext_idx].flat) { > + n = extent->sectors - sector_num; If I have two flat extents: Extent A: 0 - 1.5MB Extent B: 1.5MB - 2MB And I call vmdk_is_allocated(sector_num=1.5MB), then n = 512KB - 1.5MB which is negative! Also n is an int but it should be an int64_t (or uint64_t) which can hold sector units. Stefan