Re: [Qemu-devel] [PATCH for-next 0/6] qemu-img: Add salvaging mode to convert

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2748 bytes --]

Any opinions?

On 03.12.18 18:52, Max Reitz wrote:
> Hi,
> 
> This series adds a --salvage option to qemu-img convert.  With this,
> qemu-img will not abort when it encounters an I/O error.  Instead, it
> tries to narrow it down and will treat the affected sectors as being
> completely 0 (and print a warning).
> 
> Testing this is not so easy, because while real I/O errors during read
> operations should be treated as described above, errors encountered
> during bdrv_block_status() should just be ignored and the affected
> sectors should be considered allocated.  But blkdebug does not yet have
> a way to intercept this, and:
> 
> (1) Just adding a new block-status event would be silly, because I don't
>     want an event, I want it to fail on a certain kind of operation, on
>     a certain sector range, independently of any events, so why can't we
>     just do that?  See patch 4.
> 
> (2) If we just make blkdebug intercept .bdrv_co_block_status() like all
>     other kinds of operations, at least iotest 041 fails, which does
>     exactly that silly thing: It uses the read_aio event to wait for any
>     read.  But it turns out that there may be a bdrv_*block_status()
>     call in between, so suddenly the wrong operation yields an error.
>     As I said, the real fault here is that it does not really make sense
>     to pray that the operation you want to fail is the one that is
>     immediately executed after some event that you hope will trigger
>     that operation.
>     See patch 3.
> 
> So patch 3 allows blkdebug users to select which kind of I/O operation
> they actually want to make fail, and patch 4 allows them to not use any
> event, but to have a rule active all the time.
> 
> Together, we can then enable error injection for block-status in patch 5
> and make use of event=none iotype=block-status in patch 6.
> 
> 
> Max Reitz (6):
>   qemu-img: Move quiet into ImgConvertState
>   qemu-img: Add salvaging mode to convert
>   blkdebug: Add @iotype error option
>   blkdebug: Add "none" event
>   blkdebug: Inject errors on .bdrv_co_block_status()
>   iotests: Test qemu-img convert --salvage
> 
>  qapi/block-core.json       |  33 +++++++-
>  block/blkdebug.c           |  60 ++++++++++++--
>  qemu-img.c                 |  97 ++++++++++++++++------
>  qemu-img-cmds.hx           |   4 +-
>  qemu-img.texi              |   5 ++
>  tests/qemu-iotests/236     | 164 +++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/236.out |  43 ++++++++++
>  tests/qemu-iotests/group   |   1 +
>  8 files changed, 370 insertions(+), 37 deletions(-)
>  create mode 100755 tests/qemu-iotests/236
>  create mode 100644 tests/qemu-iotests/236.out
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]