Commit e4603fe1 authored by Kevin Wolf's avatar Kevin Wolf Committed by Stefan Hajnoczi
Browse files

qcow2: Fix header update with overridden backing file 



In recent qemu versions, it is possible to override the backing file
name and format that is stored in the image file with values given at
runtime. In such cases, the temporary override could end up in the
image header if the qcow2 header was updated, while obviously correct
behaviour would be to leave the on-disk backing file path/format
unchanged.

Fix this and add a test case for it.

Reported-by: default avatarMichael Tokarev <mjt@tls.msk.ru>
Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Message-id: 1428411796-2852-1-git-send-email-kwolf@redhat.com
Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
parent 5a24f20a

block/qcow2.c

+22 −7
Original line number Diff line number Diff line
@@ -140,6 +140,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, 
                return 3;

            }

            bs->backing_format[ext.len] = '\0';

            s->image_backing_format = g_strdup(bs->backing_format);

#ifdef DEBUG_EXT

            printf("Qcow2: Got format extension %s\n", bs->backing_format);

#endif
@@ -884,6 +885,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, 
            goto fail;

        }

        bs->backing_file[len] = '\0';

        s->image_backing_file = g_strdup(bs->backing_file);

    }



    /* Internal snapshots */
@@ -1457,6 +1459,9 @@ static void qcow2_close(BlockDriverState *bs) 
    g_free(s->unknown_header_fields);

    cleanup_unknown_header_ext(bs);



    g_free(s->image_backing_file);

    g_free(s->image_backing_format);



    g_free(s->cluster_cache);

    qemu_vfree(s->cluster_data);

    qcow2_refcount_close(bs);
@@ -1622,9 +1627,10 @@ int qcow2_update_header(BlockDriverState *bs) 
    }



    /* Backing file format header extension */

    if (*bs->backing_format) {

    if (s->image_backing_format) {

        ret = header_ext_add(buf, QCOW2_EXT_MAGIC_BACKING_FORMAT,

                             bs->backing_format, strlen(bs->backing_format),

                             s->image_backing_format,

                             strlen(s->image_backing_format),

                             buflen);

        if (ret < 0) {

            goto fail;
@@ -1682,8 +1688,8 @@ int qcow2_update_header(BlockDriverState *bs) 
    buflen -= ret;



    /* Backing file name */

    if (*bs->backing_file) {

        size_t backing_file_len = strlen(bs->backing_file);

    if (s->image_backing_file) {

        size_t backing_file_len = strlen(s->image_backing_file);



        if (buflen < backing_file_len) {

            ret = -ENOSPC;
@@ -1691,7 +1697,7 @@ int qcow2_update_header(BlockDriverState *bs) 
        }



        /* Using strncpy is ok here, since buf is not NUL-terminated. */

        strncpy(buf, bs->backing_file, buflen);

        strncpy(buf, s->image_backing_file, buflen);



        header->backing_file_offset = cpu_to_be64(buf - ((char*) header));

        header->backing_file_size   = cpu_to_be32(backing_file_len);
@@ -1712,9 +1718,17 @@ fail: 
static int qcow2_change_backing_file(BlockDriverState *bs,

    const char *backing_file, const char *backing_fmt)

{

    BDRVQcowState *s = bs->opaque;



    pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_file ?: "");

    pstrcpy(bs->backing_format, sizeof(bs->backing_format), backing_fmt ?: "");



    g_free(s->image_backing_file);

    g_free(s->image_backing_format);



    s->image_backing_file = backing_file ? g_strdup(bs->backing_file) : NULL;

    s->image_backing_format = backing_fmt ? g_strdup(bs->backing_format) : NULL;



    return qcow2_update_header(bs);

}
@@ -2751,8 +2765,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, 
    }



    if (backing_file || backing_format) {

        ret = qcow2_change_backing_file(bs, backing_file ?: bs->backing_file,

                                        backing_format ?: bs->backing_format);

        ret = qcow2_change_backing_file(bs,

                    backing_file ?: s->image_backing_file,

                    backing_format ?: s->image_backing_format);

        if (ret < 0) {

            return ret;

        }

block/qcow2.h

+6 −0
Original line number Diff line number Diff line
@@ -283,6 +283,12 @@ typedef struct BDRVQcowState { 
    QLIST_HEAD(, Qcow2UnknownHeaderExtension) unknown_header_ext;

    QTAILQ_HEAD (, Qcow2DiscardRegion) discards;

    bool cache_discards;



    /* Backing file path and format as stored in the image (this is not the

     * effective path/format, which may be the result of a runtime option

     * override) */

    char *image_backing_file;

    char *image_backing_format;

} BDRVQcowState;



struct QCowAIOCB;

tests/qemu-iotests/130

0 → 100755
+95 −0
Original line number Diff line number Diff line 
#!/bin/bash

#

# Test that temporary backing file overrides (on the command line or in

# blockdev-add) don't replace the original path stored in the image during

# header updates.

#

# Copyright (C) 2015 Red Hat, Inc.

#

# This program is free software; you can redistribute it and/or modify

# it under the terms of the GNU General Public License as published by

# the Free Software Foundation; either version 2 of the License, or

# (at your option) any later version.

#

# This program is distributed in the hope that it will be useful,

# but WITHOUT ANY WARRANTY; without even the implied warranty of

# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

# GNU General Public License for more details.

#

# You should have received a copy of the GNU General Public License

# along with this program.  If not, see <http://www.gnu.org/licenses/>.

#



# creator

owner=kwolf@redhat.com



seq="$(basename $0)"

echo "QA output created by $seq"



here="$PWD"

tmp=/tmp/$$

status=1	# failure is the default!



_cleanup()

{

    _cleanup_test_img

}

trap "_cleanup; exit \$status" 0 1 2 3 15



# get standard environment, filters and checks

. ./common.rc

. ./common.filter

. ./common.qemu



_supported_fmt qcow2

_supported_proto generic

_supported_os Linux



qemu_comm_method="monitor"





TEST_IMG="$TEST_IMG.orig" _make_test_img 64M

TEST_IMG="$TEST_IMG.base" _make_test_img 64M

_make_test_img 64M

_img_info | _filter_img_info



echo

echo "=== HMP commit ==="

echo

# bdrv_make_empty() involves a header update for qcow2



# Test that a backing file isn't written

_launch_qemu -drive file="$TEST_IMG",backing.file.filename="$TEST_IMG.base"

_send_qemu_cmd $QEMU_HANDLE "commit ide0-hd0" "(qemu)"

_send_qemu_cmd $QEMU_HANDLE '' '(qemu)'

_cleanup_qemu

_img_info | _filter_img_info



# Make sure that if there was a backing file that was just overridden on the

# command line, that backing file is retained, with the right format

_make_test_img -F raw -b "$TEST_IMG.orig" 64M

_launch_qemu -drive file="$TEST_IMG",backing.file.filename="$TEST_IMG.base",backing.driver=$IMGFMT

_send_qemu_cmd $QEMU_HANDLE "commit ide0-hd0" "(qemu)"

_send_qemu_cmd $QEMU_HANDLE '' '(qemu)'

_cleanup_qemu

_img_info | _filter_img_info



echo

echo "=== Marking image dirty (lazy refcounts) ==="

echo



# Test that a backing file isn't written

_make_test_img 64M

$QEMU_IO -c "open -o backing.file.filename=$TEST_IMG.base,lazy-refcounts=on $TEST_IMG" -c "write 0 4k" | _filter_qemu_io

_img_info | _filter_img_info



# Make sure that if there was a backing file that was just overridden on the

# command line, that backing file is retained, with the right format

_make_test_img -F raw -b "$TEST_IMG.orig" 64M

$QEMU_IO -c "open -o backing.file.filename=$TEST_IMG.base,backing.driver=$IMGFMT,lazy-refcounts=on $TEST_IMG" -c "write 0 4k" | _filter_qemu_io

_img_info | _filter_img_info



# success, all done

echo '*** done'

rm -f $seq.full

status=0

tests/qemu-iotests/130.out

0 → 100644
+43 −0
Original line number Diff line number Diff line 
QA output created by 130

Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=67108864

Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864

image: TEST_DIR/t.IMGFMT

file format: IMGFMT

virtual size: 64M (67108864 bytes)



=== HMP commit ===



QEMU X.Y.Z monitor - type 'help' for more information

(qemu) ccocomcommcommicommitcommit commit icommit idcommit idecommit ide0commit ide0-commit ide0-hcommit ide0-hdcommit ide0-hd0

(qemu) 

image: TEST_DIR/t.IMGFMT

file format: IMGFMT

virtual size: 64M (67108864 bytes)

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.orig' backing_fmt='raw'

QEMU X.Y.Z monitor - type 'help' for more information

(qemu) ccocomcommcommicommitcommit commit icommit idcommit idecommit ide0commit ide0-commit ide0-hcommit ide0-hdcommit ide0-hd0

(qemu) 

image: TEST_DIR/t.IMGFMT

file format: IMGFMT

virtual size: 64M (67108864 bytes)

backing file: TEST_DIR/t.IMGFMT.orig

backing file format: raw



=== Marking image dirty (lazy refcounts) ===



Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864

wrote 4096/4096 bytes at offset 0

4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

image: TEST_DIR/t.IMGFMT

file format: IMGFMT

virtual size: 64M (67108864 bytes)

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.orig' backing_fmt='raw'

wrote 4096/4096 bytes at offset 0

4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

image: TEST_DIR/t.IMGFMT

file format: IMGFMT

virtual size: 64M (67108864 bytes)

backing file: TEST_DIR/t.IMGFMT.orig

backing file format: raw

*** done

tests/qemu-iotests/group

+1 −0
Original line number Diff line number Diff line
@@ -124,3 +124,4 @@ 
121 rw auto

123 rw auto quick

128 rw auto quick

130 rw auto quick