On Thu, 2011-04-14 at 20:23 +0100, Prasad Joshi wrote:
> Changed the function names from sect_to_l1_offset(), sect_to_l2_offset() to
> get_l1_index(), get_l2_index() as they return index into their respective
> table.
This patch does a lot more than what's described above. Please split the
changes in two separate patches.
> + uint32_t length;
> + uint32_t tmp;
> + char *buf = dst;
> +
> + clust_size = 1 << header->cluster_bits;
> + length = 0;
> +
> + while (length < dst_len) {
> + offset = sector << SECTOR_SHIFT;
> + if (offset >= header->size)
> + goto out_error;
> +
> + l1_idx = get_l1_index(self->priv, offset);
> + if (l1_idx >= q->table.table_size)
> + goto out_error;
> +
> + l2_table_offset = be64_to_cpu(q->table.l1_table[l1_idx]);
> + if (!l2_table_offset) {
> + tmp = clust_size;
> + memset(buf, 0, tmp);
> + goto next_cluster;
> + }
> +
> + l2_table_size = 1 << header->l2_bits;
> +
> + l2_table = calloc(l2_table_size, sizeof(uint64_t));
> + if (!l2_table)
> + goto out_error;
> +
> + if (pread_in_full(q->fd, l2_table, sizeof(uint64_t) *
> l2_table_size, l2_table_offset) < 0)
> + goto out_error_free_l2;
> +
> + l2_idx = get_l2_index(self->priv, offset);
> + if (l2_idx >= l2_table_size)
> + goto out_error_free_l2;
> +
> + clust_start = be64_to_cpu(l2_table[l2_idx]);
> + free(l2_table);
> + if (!clust_start) {
> + tmp = clust_size;
> + memset(buf, 0, tmp);
> + } else {
> + clust_offset = sect_to_cluster_offset(self->priv,
> offset);
> + tmp = clust_size - clust_offset;
> +
> + if (pread_in_full(q->fd, buf, tmp, clust_start +
> clust_offset) < 0)
> + goto out_error;
> + }
> +
> +next_cluster:
> + buf += tmp;
> + sector += (tmp >> SECTOR_SHIFT);
> + length += tmp;
> + }
Well, the loop is not exactly making the code any better. If you're
worried about reads that cross over a single cluster, it's probably
better to rename the current function to qcow1_read_cluster() or
something and use that in a loop that makes sure we never cross over a
cluster.
Btw, I think our ->read_sector and ->write_sector functions need
renaming to ->read_sectors and ->write_sectors with little bit
commentary on what they can expect to receive.
Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
