[Qemu-devel] [PATCH v5 0/2] qemu-img: Check for backing image if specified during create

John Snow posted 2 patches 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170718003422.4497-1-jsnow@redhat.com
Test FreeBSD passed
Test checkpatch failed
Test docker passed
Test s390x passed
block.c                    | 96 +++++++++++++++++++++++++---------------------
blockdev.c                 | 11 +++---
qemu-img-cmds.hx           |  4 +-
qemu-img.c                 | 16 +++++---
tests/qemu-iotests/082     |  4 +-
tests/qemu-iotests/082.out |  4 +-
tests/qemu-iotests/085     |  2 +-
tests/qemu-iotests/111.out |  1 +
tests/qemu-iotests/139     |  2 +-
tests/qemu-iotests/156     |  2 +-
tests/qemu-iotests/158     |  2 +-
11 files changed, 81 insertions(+), 63 deletions(-)
[Qemu-devel] [PATCH v5 0/2] qemu-img: Check for backing image if specified during create
Posted by John Snow 6 years, 9 months ago
We do not currently guarantee that QEMU will or will not open a
backing file when creating a new overlay file. Presently, QEMU will
not open that file if you provide a filesize, because it has no reason
to want to open it in that case.

This series makes the contract more explicit: if '-u' is provided to
create, we will not open the backing image regardless, erroring out if
a size was not provided.

In the other case, if '-u' is not provided, we now endeavor to open the
backing image if possible to check that it exists. For now, if a size
is provided and the image does not exist, QEMU will only warn to maintain
compatibility with legacy behavior.

In the future, QEMU may treat the operation as a failure if '-u' was not
provided.

Tests are amended primarily to pass the '-u' flag where it makes sense;
which is when creating overlays for objects already open by QEMU. These
will now generally fail to succeed because of image locking. In this
case, they only warn instead of fail, but this keeps the output cleaner.

Test 111 is updated to accommodate a new error message.
082, 085, 139, 156 and 158 add '-u' just to suppress warnings.

John Snow (2):
  blockdev: move BDRV_O_NO_BACKING option forward
  qemu-img: Check for backing image if specified during create

 block.c                    | 96 +++++++++++++++++++++++++---------------------
 blockdev.c                 | 11 +++---
 qemu-img-cmds.hx           |  4 +-
 qemu-img.c                 | 16 +++++---
 tests/qemu-iotests/082     |  4 +-
 tests/qemu-iotests/082.out |  4 +-
 tests/qemu-iotests/085     |  2 +-
 tests/qemu-iotests/111.out |  1 +
 tests/qemu-iotests/139     |  2 +-
 tests/qemu-iotests/156     |  2 +-
 tests/qemu-iotests/158     |  2 +-
 11 files changed, 81 insertions(+), 63 deletions(-)

-- 
2.9.4


Re: [Qemu-devel] [PATCH v5 0/2] qemu-img: Check for backing image if specified during create
Posted by John Snow 6 years, 9 months ago
Kevin: I took a stab at this 'feature', but if there are any fixups or
changes that need to occur and it's important that it happens before I'm
awake, please feel free to steal these patches and do whatever you need
to them, including setting them on fire.

Thanks,
--John

post-script: I think the only thing that I don't really do here is
attempt to force-open an image to see if it exists if a size is already
provided in order to quiet errors related to locking.

That change would just eliminate a little bit of "this image is locked!"
whining in the case that -u was omitted but a size was provided, which
is mostly QOL.

On 07/17/2017 08:34 PM, John Snow wrote:
> We do not currently guarantee that QEMU will or will not open a
> backing file when creating a new overlay file. Presently, QEMU will
> not open that file if you provide a filesize, because it has no reason
> to want to open it in that case.
> 
> This series makes the contract more explicit: if '-u' is provided to
> create, we will not open the backing image regardless, erroring out if
> a size was not provided.
> 
> In the other case, if '-u' is not provided, we now endeavor to open the
> backing image if possible to check that it exists. For now, if a size
> is provided and the image does not exist, QEMU will only warn to maintain
> compatibility with legacy behavior.
> 
> In the future, QEMU may treat the operation as a failure if '-u' was not
> provided.
> 
> Tests are amended primarily to pass the '-u' flag where it makes sense;
> which is when creating overlays for objects already open by QEMU. These
> will now generally fail to succeed because of image locking. In this
> case, they only warn instead of fail, but this keeps the output cleaner.
> 
> Test 111 is updated to accommodate a new error message.
> 082, 085, 139, 156 and 158 add '-u' just to suppress warnings.
> 
> John Snow (2):
>   blockdev: move BDRV_O_NO_BACKING option forward
>   qemu-img: Check for backing image if specified during create
> 
>  block.c                    | 96 +++++++++++++++++++++++++---------------------
>  blockdev.c                 | 11 +++---
>  qemu-img-cmds.hx           |  4 +-
>  qemu-img.c                 | 16 +++++---
>  tests/qemu-iotests/082     |  4 +-
>  tests/qemu-iotests/082.out |  4 +-
>  tests/qemu-iotests/085     |  2 +-
>  tests/qemu-iotests/111.out |  1 +
>  tests/qemu-iotests/139     |  2 +-
>  tests/qemu-iotests/156     |  2 +-
>  tests/qemu-iotests/158     |  2 +-
>  11 files changed, 81 insertions(+), 63 deletions(-)
> 


Re: [Qemu-devel] [PATCH v5 0/2] qemu-img: Check for backing image if specified during create
Posted by no-reply@patchew.org 6 years, 9 months ago
Hi,

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

Subject: [Qemu-devel] [PATCH v5 0/2] qemu-img: Check for backing image if specified during create
Message-id: 20170718003422.4497-1-jsnow@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

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
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20170717145556.14142-1-aurelien@aurel32.net -> patchew/20170717145556.14142-1-aurelien@aurel32.net
Switched to a new branch 'test'
7ebdc98 qemu-img: Check for backing image if specified during create
8bc4293 blockdev: move BDRV_O_NO_BACKING option forward

=== OUTPUT BEGIN ===
Checking PATCH 1/2: blockdev: move BDRV_O_NO_BACKING option forward...
Checking PATCH 2/2: qemu-img: Check for backing image if specified during create...
ERROR: Error messages should not contain newlines
#103: FILE: block.c:4432:
+                              "This may become an error in future versions.\n");

total: 1 errors, 0 warnings, 222 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH v5 0/2] qemu-img: Check for backing image if specified during create
Posted by Kevin Wolf 6 years, 9 months ago
Am 18.07.2017 um 02:34 hat John Snow geschrieben:
> We do not currently guarantee that QEMU will or will not open a
> backing file when creating a new overlay file. Presently, QEMU will
> not open that file if you provide a filesize, because it has no reason
> to want to open it in that case.
> 
> This series makes the contract more explicit: if '-u' is provided to
> create, we will not open the backing image regardless, erroring out if
> a size was not provided.
> 
> In the other case, if '-u' is not provided, we now endeavor to open the
> backing image if possible to check that it exists. For now, if a size
> is provided and the image does not exist, QEMU will only warn to maintain
> compatibility with legacy behavior.
> 
> In the future, QEMU may treat the operation as a failure if '-u' was not
> provided.
> 
> Tests are amended primarily to pass the '-u' flag where it makes sense;
> which is when creating overlays for objects already open by QEMU. These
> will now generally fail to succeed because of image locking. In this
> case, they only warn instead of fail, but this keeps the output cleaner.
> 
> Test 111 is updated to accommodate a new error message.
> 082, 085, 139, 156 and 158 add '-u' just to suppress warnings.

Thanks, applied to the block branch.

Kevin