Message ID | 20190415155452.5115-3-kwolf@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | qcow2: Preallocation fixes | expand |
On 4/15/19 10:54 AM, Kevin Wolf wrote: > With an external data file, preallocate_co() must write the final byte > to the external data file, not to the qcow2 image file. > > This is harmless for preallocation of newly created images (only the > qcow2 file size is increased to the virtual disk size while it should be > much smaller), but with preallocated resize, it could in theory cause > visible corruption if the metadata of the image is larger than the data > (e.g. lots of bitmaps). Can we come up with such an image - maybe one with 512-byte cluster sizing and only 1k in guest-visible length? Since each bitmap is cluster-aligned, it seems like you'd only need a couple of bitmaps to easily reach that point. We're awfully late for 4.0, but as we already have -rc4 coming due, and as this is a data-corruption bug in a new feature, I can buy the argument of getting this one into 4.0, particularly if you can design the iotest along the lines of my ideas to prove that yes, indeed, we are accidentally wiping out qcow2 metadata for visible image corruption. Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/block/qcow2.c b/block/qcow2.c index c8400e9712..dfac74c264 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2772,7 +2772,7 @@ static int coroutine_fn preallocate_co(BlockDriverState *bs, uint64_t offset, */ if (host_offset != 0) { uint8_t data = 0; - ret = bdrv_pwrite(bs->file, (host_offset + cur_bytes) - 1, + ret = bdrv_pwrite(s->data_file, (host_offset + cur_bytes) - 1, &data, 1); if (ret < 0) { return ret;
With an external data file, preallocate_co() must write the final byte to the external data file, not to the qcow2 image file. This is harmless for preallocation of newly created images (only the qcow2 file size is increased to the virtual disk size while it should be much smaller), but with preallocated resize, it could in theory cause visible corruption if the metadata of the image is larger than the data (e.g. lots of bitmaps). Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)