Re: [PATCH v3 4/4] qemu-img: Deprecate use of -b without -F

[Kashyap Chamarthy]

On Tue, Mar 10, 2020 at 07:19:25AM -0500, Eric Blake wrote:
> On 3/10/20 5:57 AM, Kashyap Chamarthy wrote:

(Slightly long e-mail, as it contains a bunch of tests and their
results; please bear with me.)

[...]

> >      $> ~/build/qemu/qemu-img amend -o compat=v3 overlay1.qcow2
> 
> This particular amend is not changing the backing file, and since I did not
> add warnings on the opening of an existing unsafe image, it is silent.

I see; okay, that's expected.

> Instead, you need to test a case where amend provokes a path that would
> change the backing file (perhaps as simple as '-o backing_file=./base.raw'),
> while omitting a format for the new backing file string.

I couldn't work out the black magic to change the backing file via
'qemu-img amend'.  

It is surely not this:

    $> ~/build/qemu/qemu-img amend -o 'backing_file=./bar.qcow2' -o 
another_base.qcow2 
    qemu-img: Expecting one image file name

Let's try something else: give a *non-existent* "bar.qcow2" to '-o':

    $> ~/build/qemu/qemu-img amend -o 'backing_file=./bar.qcow2' 
another_base.qcow2 
    qemu-img: Could not open 'another_base.qcow2': Could not open backing file: 
Failed to get shared "write" lock
    Is another process using the image [./another_base.qcow2]?

That's strange; there is no live QEMU process on this host (let alone
one that is using another_base.qcow2):

    $> pgrep qemu-system-x86
    $> echo $?
    1

Probably it is just complaning about the non-existent bar.qcow2 file?
Then I'd expect it to say as much.


On IRC you pointed out iotest 082 to look for help.  There I don't see a
way to change the backing file.  But only a combination of 'amend' +
'rebase':

    run_qemu_img amend -f $IMGFMT \
        -o backing_fmt=$IMGFMT,backing_file="$TEST_IMG",,\? "$TEST_IMG"
    run_qemu_img rebase -u -b "" -f $IMGFMT "$TEST_IMG"

(I know you can use 'rebase' alone to change the backing file format.)

Note to self: we really need to document 'amend' much better, in which
scenarios it is useful, and contrast it with 'rebase'.

            - - -

Meanwhile, I've done a bunch of tests with 'amend'.  Here are the
results.

Scenario: base.raw <-- overlay1.qcow2
-------------------------------------

Without "-f raw", the warning is provoked when trying to amend the
backing file (let's ignore for a moment that you can't seem to amend a
raw file):

    $> ~/build/qemu/qemu-img amend -o compat=v3 base.raw
    WARNING: Image format was not specified for 'base.raw' and probing guessed 
raw.
             Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
             Specify the 'raw' format explicitly to remove the restrictions.
    qemu-img: Format driver 'raw' does not support option amendment

With "-f raw", the warning is not triggerred (correctly so?):

    $> ~/build/qemu/qemu-img amend -o compat=v3 -f raw base.raw
    qemu-img: Format driver 'raw' does not support option amendment


And these two tests (one with relative path; the other with absolute
path) don't trigger the warning either (on IRC you said the following is
_supposed_ to trigger a warning):


    $> ~/build/qemu/qemu-img amend \
            -o 'backing_file=base.raw' -f qcow2 overlay1.qcow2

    $> ~/build/qemu/qemu-img amend \
            -o 'backing_file=./base.raw' -f qcow2 overlay1.qcow2


'qemu-img info' of the above disk image chain:

    $> ~/build/qemu/qemu-img info --backing-chain overlay1.qcow2
    image: overlay1.qcow2
    file format: qcow2
    virtual size: 4 GiB (4294967296 bytes)
    disk size: 196 KiB
    cluster_size: 65536
    backing file: ./base.raw
    backing file format: raw
    Format specific information:
        compat: 1.1
        lazy refcounts: false
        refcount bits: 16
        corrupt: false
    
    image: ./base.raw
    file format: raw
    virtual size: 4 GiB (4294967296 bytes)
    disk size: 778 MiB


Scenario: another_base.qcow2 <-- overlay1_of_ab.qcow2
-----------------------------------------------------

With and w/o specifying the aAmend the backing file (none of these
provoke the warning -- expected?):

    $> ~/build/qemu/qemu-img amend another_base.qcow2

    $> ~/build/qemu/qemu-img amend -f qcow2 another_base.qcow2

Tests to amend the overlay file (none of these provoke the warning --
expected, per your previous reply):

    $> ~/build/qemu/qemu-img amend overlay1_of_ab.qcow2  

    $> ~/build/qemu/qemu-img amend -f qcow2 overlay1_of_ab.qcow2  


'qemu-img info' of the above disk image chain:

    $> ~/build/qemu/qemu-img info --backing-chain overlay1_of_ab.qcow2 
    image: overlay1_of_ab.qcow2
    file format: qcow2
    virtual size: 4 GiB (4294967296 bytes)
    disk size: 196 KiB
    cluster_size: 65536
    backing file: ./another_base.qcow2
    Format specific information:
        compat: 1.1
        lazy refcounts: false
        refcount bits: 16
        corrupt: false
    
    image: ./another_base.qcow2
    file format: qcow2
    virtual size: 4 GiB (4294967296 bytes)
    disk size: 293 MiB
    cluster_size: 65536
    Format specific information:
        compat: 1.1
        lazy refcounts: false
        refcount bits: 16
        corrupt: false


Strange command-line
--------------------

Does this make sense?  What is this even trying to do ...

    $> ~/build/qemu/qemu-img amend \
            -o 'backing_file=./another_base.qcow2' another_base.qcow2 
    
    $> echo $?
    0

> Look at patch 2/4 - that patch was written AFTER this patch in order to
> silence every warning that was introduced because of this patch, then
> rebased to occur first.  My experience in writing 2/4 was that I indeed hit
> warnings through all four sub-commands. 

Will look.

Thanks.

-- 
/kashyap