1. Patchew
  2. QEMU
  3. View series

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

Max Reitz posted 6 patches 5 years, 5 months ago
Test asan passed
Test checkpatch failed
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Failed in applying to current master (apply log)
There is a newer version of this series
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
[Qemu-devel] [PATCH for-next 0/6] qemu-img: Add salvaging mode to convert
Posted by Max Reitz 5 years, 5 months ago 
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

-- 
2.19.2
Re: [Qemu-devel] [PATCH for-next 0/6] qemu-img: Add salvaging mode to convert
Posted by no-reply@patchew.org 5 years, 5 months ago 
Patchew URL: https://patchew.org/QEMU/20181203175211.8230-1-mreitz@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH for-next 0/6] qemu-img: Add salvaging mode to convert
Message-id: 20181203175211.8230-1-mreitz@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
fatal: unable to access 'https://github.com/patchew-project/qemu/': Operation timed out after 300008 milliseconds with 0 out of 0 bytes received
error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384
Traceback (most recent call last):
  File "patchew-tester/src/patchew-cli", line 521, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf, True)
  File "patchew-tester/src/patchew-cli", line 48, in git_clone_repo
    stdout=logf, stderr=logf)
  File "/usr/lib64/python3.4/subprocess.py", line 558, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1



The full log is available at
http://patchew.org/logs/20181203175211.8230-1-mreitz@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Patchew-devel] [Qemu-devel] [PATCH for-next 0/6] qemu-img: Add salvaging mode to convert
Posted by Eric Blake 5 years, 5 months ago 
On 12/3/18 9:39 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20181203175211.8230-1-mreitz@redhat.com/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 

> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> fatal: unable to access 'https://github.com/patchew-project/qemu/': Operation timed out after 300008 milliseconds with 0 out of 0 bytes received

False negative. It would be nice to figure out why patchew had a 
connection problem, but it would also be nice if patchew didn't send out 
failure notices when the failure is due to a git clone issue unrelated 
to the patch itself.

> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Qemu-devel] [PATCH for-next 0/6] qemu-img: Add salvaging mode to convert
Posted by Max Reitz 5 years, 3 months ago 
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
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.