The following changes since commit 718d7f4f9cf772e5784093d8e6085680a235acdb:

Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2017-07-18 13:09:51 +0100)

are available in the git repository at:

for you to fetch changes up to 6e6e55f5c2e5b520d6506c2716287ba3b5d1bbc8:

qemu-img: Check for backing image if specified during create (2017-07-18 15:27:37 +0200)

Block layer patches

Hervé Poussineau (4):

vvfat: add constants for special values of name[0]

vvfat: add a constant for bootsector name

vvfat: correctly parse non-ASCII short and long file names

vvfat: initialize memory after allocating it

John Snow (2):

blockdev: move BDRV_O_NO_BACKING option forward

qemu-img: Check for backing image if specified during create

Kevin Wolf (10):

commit: Add NULL check for overlay_bs

block: Make blk_get_attached_dev_id() public

block/qapi: Add qdev device name to query-block

block: Make blk_all_next() public

block/qapi: Use blk_all_next() for query-block

block: List anonymous device BBs in query-block

ide: bdrv_attach_dev() for empty CD-ROM

scsi-disk: bdrv_attach_dev() for empty CD-ROM

qemu-iotests: Test 'info block'

qemu-iotests: Test unplug of -device without drive

Manos Pitsidianakis (2):

block: add clock_type field to ThrottleGroup

block: remove timer canceling in throttle_config()

Max Reitz (1):

block/vvfat: Fix compiler warning with gcc 7

From: John Snow <jsnow@redhat.com>

For both external_snapshot_prepare and qmp_drive_mirror, we eventually

append the option BDRV_O_NO_BACKING. However, we generally do so after

we create the image.

To accommodate image creation wanting to verify that a backing file

exists or not, add this option prior to create to override checking

the existence of the backing file. This prevents QEMU from trying to

re-open a backing file that's already in use (thanks to qcow2 locking).

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

blockdev.c | 11 ++++++-----

1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

}

flags = state->old_bs->open_flags;

- flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);

+ flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_COPY_ON_READ);

+ flags |= BDRV_O_NO_BACKING;

/* create new image w/backing file */

mode = s->has_mode ? s->mode : NEW_IMAGE_MODE_ABSOLUTE_PATHS;

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

qdict_put_str(options, "node-name", snapshot_node_name);

}

qdict_put_str(options, "driver", format);

-

- flags |= BDRV_O_NO_BACKING;

}

state->new_bs = bdrv_open(new_image_file, snapshot_ref, options, flags,

@@ -XXX,XX +XXX,XX @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)

backing_mode = MIRROR_OPEN_BACKING_CHAIN;

}

+ /* Don't open backing image in create() */

+ flags |= BDRV_O_NO_BACKING;

+

if ((arg->sync == MIRROR_SYNC_MODE_FULL || !source)

&& arg->mode != NEW_IMAGE_MODE_EXISTING)

{

@@ -XXX,XX +XXX,XX @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)

/* Mirroring takes care of copy-on-write using the source's backing

* file.

*/

- target_bs = bdrv_open(arg->target, NULL, options,

- flags | BDRV_O_NO_BACKING, errp);

+ target_bs = bdrv_open(arg->target, NULL, options, flags, errp);

if (!target_bs) {

goto out;

}

1.8.3.1

From: John Snow <jsnow@redhat.com>

Or, rather, force the open of a backing image if one was specified

for creation. Using a similar -unsafe option as rebase, allow qemu-img

to ignore the backing file validation if possible.

It may not always be possible, as in the existing case when a filesize

for the new image was not specified.

This is accomplished by shifting around the conditionals in

bdrv_img_create, such that a backing file is always opened unless we

provide BDRV_O_NO_BACKING. qemu-img is adjusted to pass this new flag

when -u is provided to create.

Sorry for the heinous looking diffstat, but it's mostly whitespace.

Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1213786

Signed-off-by: John Snow <jsnow@redhat.com>

block.c | 94 +++++++++++++++++++++++++---------------------

qemu-img-cmds.hx | 4 +-

qemu-img.c | 16 +++++---

qemu-img.texi | 9 ++++-

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 +-

tests/qemu-iotests/189 | 2 +-

12 files changed, 83 insertions(+), 59 deletions(-)

@@ -XXX,XX +XXX,XX @@ void bdrv_img_create(const char *filename, const char *fmt,

backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);

- // The size for the image must always be specified, with one exception:

- // If we are using a backing file, we can obtain the size from there

+ /* The size for the image must always be specified, unless we have a backing

+ * file and we have not been forbidden from opening it. */

size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);

- if (size == -1) {

- if (backing_file) {

- BlockDriverState *bs;

- char *full_backing = g_new0(char, PATH_MAX);

- int64_t size;

- int back_flags;

- QDict *backing_options = NULL;

-

- bdrv_get_full_backing_filename_from_filename(filename, backing_file,

- full_backing, PATH_MAX,

- &local_err);

- if (local_err) {

- g_free(full_backing);

- goto out;

- }

+ if (backing_file && !(flags & BDRV_O_NO_BACKING)) {

+ BlockDriverState *bs;

+ char *full_backing = g_new0(char, PATH_MAX);

+ int back_flags;

+ QDict *backing_options = NULL;

+ bdrv_get_full_backing_filename_from_filename(filename, backing_file,

+ full_backing, PATH_MAX,

+ &local_err);

+ if (local_err) {

+ g_free(full_backing);

+ goto out;

+ }

- /* backing files always opened read-only */

- back_flags = flags;

- back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);

+ /* backing files always opened read-only */

+ back_flags = flags;

+ back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);

- if (backing_fmt) {

- backing_options = qdict_new();

- qdict_put_str(backing_options, "driver", backing_fmt);

- }

+ if (backing_fmt) {

+ backing_options = qdict_new();

+ qdict_put_str(backing_options, "driver", backing_fmt);

+ }

- bs = bdrv_open(full_backing, NULL, backing_options, back_flags,

- &local_err);

- g_free(full_backing);

- if (!bs) {

- goto out;

- }

- size = bdrv_getlength(bs);

- if (size < 0) {

- error_setg_errno(errp, -size, "Could not get size of '%s'",

- backing_file);

- bdrv_unref(bs);

- goto out;

+ bs = bdrv_open(full_backing, NULL, backing_options, back_flags,

+ &local_err);

+ g_free(full_backing);

+ if (!bs && size != -1) {

+ /* Couldn't open BS, but we have a size, so it's nonfatal */

+ warn_reportf_err(local_err,

+ "Could not verify backing image. "

+ "This may become an error in future versions.\n");

+ local_err = NULL;

+ } else if (!bs) {

+ /* Couldn't open bs, do not have size */

+ error_append_hint(&local_err,

+ "Could not open backing image to determine size.\n");

+ goto out;

+ } else {

+ if (size == -1) {

+ /* Opened BS, have no size */

+ size = bdrv_getlength(bs);

+ if (size < 0) {

+ error_setg_errno(errp, -size, "Could not get size of '%s'",

+ backing_file);

+ bdrv_unref(bs);

+ goto out;

+ }

+ qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort);

}

-

- qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort);

-

bdrv_unref(bs);

- } else {

- error_setg(errp, "Image creation needs a size parameter");

- goto out;

+ } /* (backing_file && !(flags & BDRV_O_NO_BACKING)) */

+ if (size == -1) {

+ error_setg(errp, "Image creation needs a size parameter");

+ goto out;

if (!quiet) {

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img-cmds.hx

+++ b/qemu-img-cmds.hx

@@ -XXX,XX +XXX,XX @@ STEXI

ETEXI

DEF("create", img_create,

- "create [-q] [--object objectdef] [-f fmt] [-b backing_file] [-F backing_fmt] [-o options] filename [size]")

+ "create [-q] [--object objectdef] [-f fmt] [-b backing_file] [-F backing_fmt] [-u] [-o options] filename [size]")

STEXI

-@item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-b @var{backing_file}] [-F @var{backing_fmt}] [-o @var{options}] @var{filename} [@var{size}]

+@item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-b @var{backing_file}] [-F @var{backing_fmt}] [-u] [-o @var{options}] @var{filename} [@var{size}]

ETEXI

DEF("commit", img_commit,

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ static void QEMU_NORETURN help(void)

" 'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"

" instead\n"

" '-c' indicates that target image must be compressed (qcow format only)\n"

- " '-u' enables unsafe rebasing. It is assumed that old and new backing file\n"

- " match exactly. The image doesn't need a working backing file before\n"

- " rebasing in this case (useful for renaming the backing file)\n"

+ " '-u' allows unsafe backing chains. For rebasing, it is assumed that old and\n"

+ " new backing file match exactly. The image doesn't need a working\n"

+ " backing file before rebasing in this case (useful for renaming the\n"

+ " backing file). For image creation, allow creating without attempting\n"

+ " to open the backing file.\n"

" '-h' with or without a command shows this help and lists the supported formats\n"

" '-p' show progress of command (only certain commands)\n"

" '-q' use Quiet mode - do not print any output (except errors)\n"

@@ -XXX,XX +XXX,XX @@ static int img_create(int argc, char **argv)

char *options = NULL;

Error *local_err = NULL;

bool quiet = false;

+ int flags = 0;

for(;;) {

static const struct option long_options[] = {

@@ -XXX,XX +XXX,XX @@ static int img_create(int argc, char **argv)

{"object", required_argument, 0, OPTION_OBJECT},

{0, 0, 0, 0}

};

- c = getopt_long(argc, argv, ":F:b:f:ho:q",

+ c = getopt_long(argc, argv, ":F:b:f:ho:qu",

long_options, NULL);

if (c == -1) {

break;

@@ -XXX,XX +XXX,XX @@ static int img_create(int argc, char **argv)

case 'q':

quiet = true;

break;

+ case 'u':

+ flags |= BDRV_O_NO_BACKING;

+ break;

case OPTION_OBJECT: {

QemuOpts *opts;

opts = qemu_opts_parse_noisily(&qemu_object_opts,

@@ -XXX,XX +XXX,XX @@ static int img_create(int argc, char **argv)

}

bdrv_img_create(filename, fmt, base_filename, base_fmt,

- options, img_size, 0, quiet, &local_err);

+ options, img_size, flags, quiet, &local_err);

if (local_err) {

error_reportf_err(local_err, "%s: ", filename);

goto fail;

diff --git a/qemu-img.texi b/qemu-img.texi

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.texi

+++ b/qemu-img.texi

@@ -XXX,XX +XXX,XX @@ If @code{-r} is specified, exit codes representing the image state refer to the

state after (the attempt at) repairing it. That is, a successful @code{-r all}

will yield the exit code 0, independently of the image state before.

-@item create [-f @var{fmt}] [-b @var{backing_file}] [-F @var{backing_fmt}] [-o @var{options}] @var{filename} [@var{size}]

+@item create [-f @var{fmt}] [-b @var{backing_file}] [-F @var{backing_fmt}] [-u] [-o @var{options}] @var{filename} [@var{size}]

Create the new disk image @var{filename} of size @var{size} and format

@var{fmt}. Depending on the file format, you can add one or more @var{options}

@@ -XXX,XX +XXX,XX @@ only the differences from @var{backing_file}. No size needs to be specified in

this case. @var{backing_file} will never be modified unless you use the

@code{commit} monitor command (or qemu-img commit).

+Note that a given backing file will be opened to check that it is valid. Use

+the @code{-u} option to enable unsafe backing file mode, which means that the

+image will be created even if the associated backing file cannot be opened. A

+matching backing file must be created or additional options be used to make the

+backing file specification valid when you want to use an image created this

+way.

+

The size can also be specified using the @var{size} option with @code{-o},

it doesn't need to be specified separately in this case.

diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/082

+++ b/tests/qemu-iotests/082

@@ -XXX,XX +XXX,XX @@ run_qemu_img create -f $IMGFMT -o cluster_size=4k -o help "$TEST_IMG" $size

run_qemu_img create -f $IMGFMT -o cluster_size=4k -o \? "$TEST_IMG" $size

# Looks like a help option, but is part of the backing file name

-run_qemu_img create -f $IMGFMT -o backing_file="$TEST_IMG",,help "$TEST_IMG" $size

-run_qemu_img create -f $IMGFMT -o backing_file="$TEST_IMG",,\? "$TEST_IMG" $size

+run_qemu_img create -f $IMGFMT -u -o backing_file="$TEST_IMG",,help "$TEST_IMG" $size

+run_qemu_img create -f $IMGFMT -u -o backing_file="$TEST_IMG",,\? "$TEST_IMG" $size

# Try to trick qemu-img into creating escaped commas

run_qemu_img create -f $IMGFMT -o backing_file="$TEST_IMG", -o help "$TEST_IMG" $size

diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/082.out

+++ b/tests/qemu-iotests/082.out

@@ -XXX,XX +XXX,XX @@ lazy_refcounts Postpone refcount updates

refcount_bits Width of a reference count entry in bits

nocow Turn off copy-on-write (valid only on btrfs)

-Testing: create -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2 128M

+Testing: create -f qcow2 -u -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2 128M

Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2,,help cluster_size=65536 lazy_refcounts=off refcount_bits=16

-Testing: create -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,? TEST_DIR/t.qcow2 128M

+Testing: create -f qcow2 -u -o backing_file=TEST_DIR/t.qcow2,,? TEST_DIR/t.qcow2 128M

Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2,,? cluster_size=65536 lazy_refcounts=off refcount_bits=16

Testing: create -f qcow2 -o backing_file=TEST_DIR/t.qcow2, -o help TEST_DIR/t.qcow2 128M

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/085

+++ b/tests/qemu-iotests/085

@@ -XXX,XX +XXX,XX @@ function add_snapshot_image()

{

base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"

snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"

- _make_test_img -b "${base_image}" "$size"

+ _make_test_img -u -b "${base_image}" "$size"

mv "${TEST_IMG}" "${snapshot_file}"

do_blockdev_add "$1" "'backing': '', " "${snapshot_file}"

}

diff --git a/tests/qemu-iotests/111.out b/tests/qemu-iotests/111.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/111.out

+++ b/tests/qemu-iotests/111.out

@@ -XXX,XX +XXX,XX @@

QA output created by 111

qemu-img: TEST_DIR/t.IMGFMT: Could not open 'TEST_DIR/t.IMGFMT.inexistent': No such file or directory

+Could not open backing image to determine size.

*** done

diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/139

+++ b/tests/qemu-iotests/139

@@ -XXX,XX +XXX,XX @@ class TestBlockdevDel(iotests.QMPTestCase):

# Add a BlockDriverState that will be used as overlay for the base_img BDS

def addBlockDriverStateOverlay(self, node):

self.checkBlockDriverState(node, False)

- iotests.qemu_img('create', '-f', iotests.imgfmt,

+ iotests.qemu_img('create', '-u', '-f', iotests.imgfmt,

'-b', base_img, new_img, '1M')

opts = {'driver': iotests.imgfmt,

'node-name': node,

diff --git a/tests/qemu-iotests/156 b/tests/qemu-iotests/156

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/156

+++ b/tests/qemu-iotests/156

@@ -XXX,XX +XXX,XX @@ _send_qemu_cmd $QEMU_HANDLE \

'return'

# Create snapshot

-TEST_IMG="$TEST_IMG.overlay" _make_test_img -b "$TEST_IMG" 1M

+TEST_IMG="$TEST_IMG.overlay" _make_test_img -u -b "$TEST_IMG" 1M

_send_qemu_cmd $QEMU_HANDLE \

"{ 'execute': 'blockdev-snapshot-sync',

'arguments': { 'device': 'source',

diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/158

+++ b/tests/qemu-iotests/158

@@ -XXX,XX +XXX,XX @@ echo "== verify pattern =="

$QEMU_IO --object $SECRET -c "read -P 0xa 0 $size" --image-opts $IMGSPECBASE | _filter_qemu_io | _filter_testdir

echo "== create overlay =="

-_make_test_img --object $SECRET -o "encryption=on,encrypt.key-secret=sec0" -b "$TEST_IMG_BASE" $size

+_make_test_img -u --object $SECRET -o "encryption=on,encrypt.key-secret=sec0" -b "$TEST_IMG_BASE" $size

echo

echo "== writing part of a cluster =="

diff --git a/tests/qemu-iotests/189 b/tests/qemu-iotests/189

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/189

+++ b/tests/qemu-iotests/189

@@ -XXX,XX +XXX,XX @@ echo "== verify pattern =="

$QEMU_IO --object $SECRET0 -c "read -P 0xa 0 $size" --image-opts $IMGSPECBASE | _filter_qemu_io | _filter_testdir

echo "== create overlay =="

-_make_test_img --object $SECRET1 -o "encrypt.format=luks,encrypt.key-secret=sec1,encrypt.iter-time=10" -b "$TEST_IMG_BASE" $size

+_make_test_img --object $SECRET1 -o "encrypt.format=luks,encrypt.key-secret=sec1,encrypt.iter-time=10" -u -b "$TEST_IMG_BASE" $size

echo

echo "== writing part of a cluster =="

1.8.3.1

From: Peter Maydell <peter.maydell@linaro.org>

The function vmdk_read_cid() can fail if the read on the underlying

block device fails, or if there's a format error in the VMDK file.

However its API doesn't provide a mechanism to report these errors,

and in some cases we were returning a CID of 0 and in some cases a

CID of 0xffffffff, either of which might potentially be valid values.

Change the function to return 0 on success or a negative errno, and

return the CID via a uint32_t* argument. Update the callsites to

handle and propagate the error appropriately.

This fixes in passing a Coverity-spotted issue (CID 1350038) where

we weren't checking the return value from sscanf().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Fam Zheng <famz@redhat.com>

block/vmdk.c | 44 ++++++++++++++++++++++++++++++++------------

1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c

--- a/block/vmdk.c

+++ b/block/vmdk.c

@@ -XXX,XX +XXX,XX @@ static void vmdk_free_last_extent(BlockDriverState *bs)

s->extents = g_renew(VmdkExtent, s->extents, s->num_extents);

-static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)

+/* Return -ve errno, or 0 on success and write CID into *pcid. */

+static int vmdk_read_cid(BlockDriverState *bs, int parent, uint32_t *pcid)

{

char *desc;

- uint32_t cid = 0xffffffff;

+ uint32_t cid;

const char *p_name, *cid_str;

size_t cid_str_size;

BDRVVmdkState *s = bs->opaque;

@@ -XXX,XX +XXX,XX @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)

desc = g_malloc0(DESC_SIZE);

ret = bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE);

if (ret < 0) {

- g_free(desc);

- return 0;

+ goto out;

if (parent) {

@@ -XXX,XX +XXX,XX @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)

desc[DESC_SIZE - 1] = '\0';

p_name = strstr(desc, cid_str);

- if (p_name != NULL) {

- p_name += cid_str_size;

- sscanf(p_name, "%" SCNx32, &cid);

+ if (p_name == NULL) {

+ ret = -EINVAL;

+ goto out;

}

+ p_name += cid_str_size;

+ if (sscanf(p_name, "%" SCNx32, &cid) != 1) {

+ ret = -EINVAL;

+ goto out;

+ }

+ *pcid = cid;

+ ret = 0;

+out:

g_free(desc);

- return cid;

+ return ret;

}

static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)

@@ -XXX,XX +XXX,XX @@ static int vmdk_is_cid_valid(BlockDriverState *bs)

if (!s->cid_checked && bs->backing) {

BlockDriverState *p_bs = bs->backing->bs;

- cur_pcid = vmdk_read_cid(p_bs, 0);

+ if (vmdk_read_cid(p_bs, 0, &cur_pcid) != 0) {

+ /* read failure: report as not valid */

+ return 0;

+ }

if (s->parent_cid != cur_pcid) {

/* CID not valid */

return 0;

@@ -XXX,XX +XXX,XX @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,

if (ret) {

goto fail;

}

- s->cid = vmdk_read_cid(bs, 0);

- s->parent_cid = vmdk_read_cid(bs, 1);

+ ret = vmdk_read_cid(bs, 0, &s->cid);

+ if (ret) {

+ goto fail;

+ }

+ ret = vmdk_read_cid(bs, 1, &s->parent_cid);

+ if (ret) {

+ goto fail;

+ }

qemu_co_mutex_init(&s->lock);

/* Disable migration when VMDK images are used */

@@ -XXX,XX +XXX,XX @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)

ret = -EINVAL;

goto exit;

}

- parent_cid = vmdk_read_cid(blk_bs(blk), 0);

+ ret = vmdk_read_cid(blk_bs(blk), 0, &parent_cid);

blk_unref(blk);

+ if (ret) {

+ goto exit;

+ }

snprintf(parent_desc_line, BUF_SIZE,

"parentFileNameHint=\"%s\"", backing_file);

}

1.8.3.1

From: Hervé Poussineau <hpoussin@reactos.org>

This prevents some host to guest memory content leaks.

Fixes: https://bugs.launchpad.net/qemu/+bug/1599539

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

block/vvfat.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/block/vvfat.c b/block/vvfat.c

--- a/block/vvfat.c

+++ b/block/vvfat.c

@@ -XXX,XX +XXX,XX @@ static inline int array_ensure_allocated(array_t* array, int index)

array->pointer = g_realloc(array->pointer, new_size);

if (!array->pointer)

return -1;

+ memset(array->pointer + array->size, 0, new_size - array->size);

array->size = new_size;

array->next = index + 1;

}

1.8.3.1

From: Max Reitz <mreitz@redhat.com>

gcc 7 complains that the sprintf() might write a null byte beyond the

end of the tail buffer. That is wrong, but we can silence it by making

i unsigned (it can never be negative anyway, see the if condition right

before). For some reason, this allows gcc to suddenly accurately

calculate the range of i so we can give the tail[] array the exact size

it needs to have (which is 8 bytes) without gcc complaining.

In addition, let us convert the sprintf() to snprintf(), because that is

always nicer, and add an assertion about the range of the return value

afterwards so we can see that "8 - len" will never be negative and thus

"entry->name + MIN(j, 8 - len)" will never be out of bounds.

Signed-off-by: Max Reitz <mreitz@redhat.com>

block/vvfat.c | 5 +++--

1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c

--- a/block/vvfat.c

+++ b/block/vvfat.c

@@ -XXX,XX +XXX,XX @@ static direntry_t *create_short_filename(BDRVVVFATState *s,

const gchar *p, *last_dot = NULL;

gunichar c;

bool lossy_conversion = false;

- char tail[11];

+ char tail[8];

if (!entry) {

return NULL;

@@ -XXX,XX +XXX,XX @@ static direntry_t *create_short_filename(BDRVVVFATState *s,

for (i = lossy_conversion ? 1 : 0; i < 999999; i++) {

direntry_t *entry1;

if (i > 0) {

- int len = sprintf(tail, "~%d", i);

+ int len = snprintf(tail, sizeof(tail), "~%u", (unsigned)i);

+ assert(len <= 7);

memcpy(entry->name + MIN(j, 8 - len), tail, len);

}

for (entry1 = array_get(&(s->directory), directory_start);

1.8.3.1

From: Hervé Poussineau <hpoussin@reactos.org>

Write support works again when image contains non-ASCII names. It is either the

case when user created a non-ASCII filename, or when initial directory contained

a non-ASCII filename (since 0c36111f57ec2188f679e7fa810291b7386bdca1)

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

block/vvfat.c | 59 +++++++++++++++++++++++++++++++++++++----------------------

1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c

--- a/block/vvfat.c

+++ b/block/vvfat.c

@@ -XXX,XX +XXX,XX @@ typedef struct {

* filename length is 0x3f * 13 bytes.

*/

unsigned char name[0x3f * 13 + 1];

+ gunichar2 name2[0x3f * 13 + 1];

int checksum, len;

int sequence_number;

} long_file_name;

@@ -XXX,XX +XXX,XX @@ static int parse_long_name(long_file_name* lfn,

return 1;

if (pointer[0] & 0x40) {

+ /* first entry; do some initialization */

lfn->sequence_number = pointer[0] & 0x3f;

lfn->checksum = pointer[13];

lfn->name[0] = 0;

lfn->name[lfn->sequence_number * 13] = 0;

- } else if ((pointer[0] & 0x3f) != --lfn->sequence_number)

+ } else if ((pointer[0] & 0x3f) != --lfn->sequence_number) {

+ /* not the expected sequence number */

return -1;

- else if (pointer[13] != lfn->checksum)

+ } else if (pointer[13] != lfn->checksum) {

+ /* not the expected checksum */

return -2;

- else if (pointer[12] || pointer[26] || pointer[27])

+ } else if (pointer[12] || pointer[26] || pointer[27]) {

+ /* invalid zero fields */

return -3;

+ }

offset = 13 * (lfn->sequence_number - 1);

for (i = 0, j = 1; i < 13; i++, j+=2) {

@@ -XXX,XX +XXX,XX @@ static int parse_long_name(long_file_name* lfn,

else if (j == 26)

j = 28;

- if (pointer[j+1] == 0)

- lfn->name[offset + i] = pointer[j];

- else if (pointer[j+1] != 0xff || (pointer[0] & 0x40) == 0)

- return -4;

- else

- lfn->name[offset + i] = 0;

+ if (pointer[j] == 0 && pointer[j + 1] == 0) {

+ /* end of long file name */

+ break;

+ }

+ gunichar2 c = (pointer[j + 1] << 8) + pointer[j];

+ lfn->name2[offset + i] = c;

}

- if (pointer[0] & 0x40)

- lfn->len = offset + strlen((char*)lfn->name + offset);

+ if (pointer[0] & 0x40) {

+ /* first entry; set len */

+ lfn->len = offset + i;

+ }

+ if ((pointer[0] & 0x3f) == 0x01) {

+ /* last entry; finalize entry */

+ glong olen;

+ gchar *utf8 = g_utf16_to_utf8(lfn->name2, lfn->len, NULL, &olen, NULL);

+ if (!utf8) {

+ return -4;

+ }

+ lfn->len = olen;

+ memcpy(lfn->name, utf8, olen + 1);

+ g_free(utf8);

+ }

return 0;

}

@@ -XXX,XX +XXX,XX @@ static int parse_short_name(BDRVVVFATState* s,

for (j = 7; j >= 0 && direntry->name[j] == ' '; j--);

for (i = 0; i <= j; i++) {

- if (direntry->name[i] <= ' ' || direntry->name[i] > 0x7f)

+ uint8_t c = direntry->name[i];

+ if (c != to_valid_short_char(c)) {

return -1;

- else if (s->downcase_short_names)

+ } else if (s->downcase_short_names) {

lfn->name[i] = qemu_tolower(direntry->name[i]);

- else

+ } else {

lfn->name[i] = direntry->name[i];

+ }

}

for (j = 2; j >= 0 && direntry->name[8 + j] == ' '; j--) {

@@ -XXX,XX +XXX,XX @@ static int parse_short_name(BDRVVVFATState* s,

lfn->name[i + j + 1] = '\0';

for (;j >= 0; j--) {

uint8_t c = direntry->name[8 + j];

- if (c <= ' ' || c > 0x7f) {

+ if (c != to_valid_short_char(c)) {

return -2;

} else if (s->downcase_short_names) {

lfn->name[i + j] = qemu_tolower(c);

@@ -XXX,XX +XXX,XX @@ DLOG(checkpoint());

/*

* Some sanity checks:

* - do not allow writing to the boot sector

- * - do not allow to write non-ASCII filenames

*/

if (sector_num < s->offset_to_fat)

@@ -XXX,XX +XXX,XX @@ DLOG(checkpoint());

direntries = (direntry_t*)(buf + 0x200 * (begin - sector_num));

for (k = 0; k < (end - begin) * 0x10; k++) {

- /* do not allow non-ASCII filenames */

- if (parse_long_name(&lfn, direntries + k) < 0) {

- fprintf(stderr, "Warning: non-ASCII filename\n");

- return -1;

- }

/* no access to the direntry of a read-only file */

- else if (is_short_name(direntries+k) &&

+ if (is_short_name(direntries + k) &&

(direntries[k].attributes & 1)) {

if (memcmp(direntries + k,

array_get(&(s->directory), dir_index + k),

1.8.3.1

From: Hervé Poussineau <hpoussin@reactos.org>

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

block/vvfat.c | 17 +++++++++++------

1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c

--- a/block/vvfat.c

+++ b/block/vvfat.c

@@ -XXX,XX +XXX,XX @@ void nonono(const char* file, int line, const char* msg) {

#endif

+#define DIR_DELETED 0xe5

+#define DIR_KANJI DIR_DELETED

+#define DIR_KANJI_FAKE 0x05

+#define DIR_FREE 0x00

+

/* dynamic array functions */

typedef struct array_t {

char* pointer;

@@ -XXX,XX +XXX,XX @@ static direntry_t *create_long_filename(BDRVVVFATState *s, const char *filename)

static char is_free(const direntry_t* direntry)

{

- return direntry->name[0]==0xe5 || direntry->name[0]==0x00;

+ return direntry->name[0] == DIR_DELETED || direntry->name[0] == DIR_FREE;

}

static char is_volume_label(const direntry_t* direntry)

@@ -XXX,XX +XXX,XX @@ static char is_short_name(const direntry_t* direntry)

static char is_directory(const direntry_t* direntry)

{

- return direntry->attributes & 0x10 && direntry->name[0] != 0xe5;

+ return direntry->attributes & 0x10 && direntry->name[0] != DIR_DELETED;

}

static inline char is_dot(const direntry_t* direntry)

@@ -XXX,XX +XXX,XX @@ static direntry_t *create_short_filename(BDRVVVFATState *s,

}

}

- if (entry->name[0] == 0xe5) {

- entry->name[0] = 0x05;

+ if (entry->name[0] == DIR_KANJI) {

+ entry->name[0] = DIR_KANJI_FAKE;

}

/* numeric-tail generation */

@@ -XXX,XX +XXX,XX @@ static int parse_short_name(BDRVVVFATState* s,

} else

lfn->name[i + j + 1] = '\0';

- if (lfn->name[0] == 0x05) {

- lfn->name[0] = 0xe5;

+ if (lfn->name[0] == DIR_KANJI_FAKE) {

+ lfn->name[0] = DIR_KANJI;

}

lfn->len = strlen((char*)lfn->name);

1.8.3.1

From: Hervé Poussineau <hpoussin@reactos.org>

Also add links to related compatibility problems.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

block/vvfat.c | 8 +++++++-

1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/vvfat.c b/block/vvfat.c

--- a/block/vvfat.c

+++ b/block/vvfat.c

@@ -XXX,XX +XXX,XX @@ void nonono(const char* file, int line, const char* msg) {

#endif

+/* bootsector OEM name. see related compatibility problems at:

+ * https://jdebp.eu/FGA/volume-boot-block-oem-name-field.html

+ * http://seasip.info/Misc/oemid.html

+ */

+#define BOOTSECTOR_OEM_NAME "MSWIN4.1"

+

#define DIR_DELETED 0xe5

#define DIR_KANJI DIR_DELETED