Am 05.05.2015 um 12:28 hat Stefan Hajnoczi geschrieben: > On Mon, May 04, 2015 at 12:58:13PM +0200, Kevin Wolf wrote: > > Am 01.05.2015 um 16:23 hat Stefan Hajnoczi geschrieben: > > > On Thu, Apr 30, 2015 at 01:11:40PM +0300, Alberto Garcia wrote: > > > > Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables) > > > > { > > > > BDRVQcowState *s = bs->opaque; > > > > Qcow2Cache *c; > > > > - int i; > > > > > > > > c = g_new0(Qcow2Cache, 1); > > > > c->size = num_tables; > > > > + c->table_size = s->cluster_size; > > > > > > This assumes c->table_size meets bs' memory alignment requirements. The > > > following would be safer: > > > > > > c->table_size = QEMU_ALIGN_UP(c->table_size, > > > bdrv_opt_mem_align(bs->file)); > > > > You can't just access more than one cluster. You might be caching data > > and later write it back when it's stale. > > I don't mean I/O alignment, just memory alignment (i.e. the start > address of the data buffer in memory).
The start address is already taken care of by qemu_blockalign(). With rounding c->table_size, you'd align the length, and that would be wrong. Though looking at the code again I see now that c->table_size isn't consistently used. The I/O requests still use s->cluster_size. We should either use it everywhere or not introduce it at all. Kevin
pgpKKRQ7DlFlY.pgp
Description: PGP signature