Re: [Qemu-devel] [PATCH] qcow2: Unset zero_beyond_eof in save_vmstate
Am 21.10.2013 um 22:37 hat Eric Blake geschrieben: On 10/20/2013 08:52 PM, Max Reitz wrote: Saving the VM state is done using bdrv_pwrite. This function may perform a read-modify-write, which in this case results in data being read from beyond the end of the virtual disk. Since we are actually trying to access an area which is not a part of the virtual disk, zero_beyond_eof has to be set to false before performing the partial write, otherwise the VM state may become corrupted. Signed-off-by: Max Reitz mre...@redhat.com --- Follow-up to (depends on): - qcow2: Restore total_sectors value in save_vmstate Reviewed-by: Eric Blake ebl...@redhat.com Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] qcow2: Unset zero_beyond_eof in save_vmstate
On 2013-10-21 22:37, Eric Blake wrote: On 10/20/2013 08:52 PM, Max Reitz wrote: Saving the VM state is done using bdrv_pwrite. This function may perform a read-modify-write, which in this case results in data being read from beyond the end of the virtual disk. Since we are actually trying to access an area which is not a part of the virtual disk, zero_beyond_eof has to be set to false before performing the partial write, otherwise the VM state may become corrupted. Signed-off-by: Max Reitz mre...@redhat.com --- Follow-up to (depends on): - qcow2: Restore total_sectors value in save_vmstate Reviewed-by: Eric Blake ebl...@redhat.com Do you have test cases that demonstrate the corruption pre-patch? I could write a test case for the other patch, but for this one it would probably be rather difficult. What I did to bisect the bug was just starting a VM over and over while saving a snapshot at some time during boot-up and trying to load that snapshot again. Sometimes, qemu itself would report a corrupted VM state, but most of the time, the guest simply hang or paniced. This is something I can detect interactively, but I don't know if I could write a test for this (at least not for hanging). Max
Re: [Qemu-devel] [PATCH] qcow2: Unset zero_beyond_eof in save_vmstate
On 2013-10-21 22:37, Eric Blake wrote: On 10/20/2013 08:52 PM, Max Reitz wrote: Saving the VM state is done using bdrv_pwrite. This function may perform a read-modify-write, which in this case results in data being read from beyond the end of the virtual disk. Since we are actually trying to access an area which is not a part of the virtual disk, zero_beyond_eof has to be set to false before performing the partial write, otherwise the VM state may become corrupted. Signed-off-by: Max Reitz mre...@redhat.com --- Follow-up to (depends on): - qcow2: Restore total_sectors value in save_vmstate Reviewed-by: Eric Blake ebl...@redhat.com Do you have test cases that demonstrate the corruption pre-patch? Okay, so it is possible to test for this after all; I've just sent a follow-up adding such a test. Thanks for pointing that out. ;-) Max
Re: [Qemu-devel] [PATCH] qcow2: Unset zero_beyond_eof in save_vmstate
On 10/20/2013 08:52 PM, Max Reitz wrote: Saving the VM state is done using bdrv_pwrite. This function may perform a read-modify-write, which in this case results in data being read from beyond the end of the virtual disk. Since we are actually trying to access an area which is not a part of the virtual disk, zero_beyond_eof has to be set to false before performing the partial write, otherwise the VM state may become corrupted. Signed-off-by: Max Reitz mre...@redhat.com --- Follow-up to (depends on): - qcow2: Restore total_sectors value in save_vmstate Reviewed-by: Eric Blake ebl...@redhat.com Do you have test cases that demonstrate the corruption pre-patch? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] qcow2: Unset zero_beyond_eof in save_vmstate
Saving the VM state is done using bdrv_pwrite. This function may perform a read-modify-write, which in this case results in data being read from beyond the end of the virtual disk. Since we are actually trying to access an area which is not a part of the virtual disk, zero_beyond_eof has to be set to false before performing the partial write, otherwise the VM state may become corrupted. Signed-off-by: Max Reitz mre...@redhat.com --- Follow-up to (depends on): - qcow2: Restore total_sectors value in save_vmstate --- block/qcow2.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 5c05bb5..3e11f25 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1941,12 +1941,15 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, BDRVQcowState *s = bs-opaque; int64_t total_sectors = bs-total_sectors; int growable = bs-growable; +bool zero_beyond_eof = bs-zero_beyond_eof; int ret; BLKDBG_EVENT(bs-file, BLKDBG_VMSTATE_SAVE); bs-growable = 1; +bs-zero_beyond_eof = false; ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov); bs-growable = growable; +bs-zero_beyond_eof = zero_beyond_eof; // bdrv_co_do_writev will have increased the total_sectors value to include // the VM state - the VM state is however not an actual part of the block // device, therefore, we need to restore the old value. -- 1.8.3.1
