From: Alberto Garcia <berto@igalia.com>

The code that tries to reopen a BlockDriverState in stream_start()

when the creation of a new block job fails crashes because it attempts

to dereference a pointer that is known to be NULL.

This is a regression introduced in a170a91fd3eab6155da39e740381867e,

likely because the code was copied from stream_complete().

Cc: qemu-stable@nongnu.org

Reported-by: Kashyap Chamarthy <kchamart@redhat.com>

Signed-off-by: Alberto Garcia <berto@igalia.com>

Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

block/stream.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

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

index XXXXXXX..XXXXXXX 100644

--- a/block/stream.c

+++ b/block/stream.c

@@ -XXX,XX +XXX,XX @@ void stream_start(const char *job_id, BlockDriverState *bs,

fail:

if (orig_bs_flags != bdrv_get_flags(bs)) {

- bdrv_reopen(bs, s->bs_flags, NULL);

+ bdrv_reopen(bs, orig_bs_flags, NULL);

}

}

--

1.8.3.1

This adds a small test for the image streaming error path for failing

block_job_create(), which would have found the null pointer dereference

in commit a170a91f.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Jeff Cody <jcody@redhat.com>

---

tests/qemu-iotests/030 | 4 ++++

tests/qemu-iotests/030.out | 4 ++--

2 files changed, 6 insertions(+), 2 deletions(-)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/030

+++ b/tests/qemu-iotests/030

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

result = self.vm.qmp('block-stream', device='nonexistent')

self.assert_qmp(result, 'error/class', 'GenericError')

+ def test_job_id_missing(self):

+ result = self.vm.qmp('block-stream', device='mid')

+ self.assert_qmp(result, 'error/class', 'GenericError')

+

class TestParallelOps(iotests.QMPTestCase):

num_ops = 4 # Number of parallel block-stream operations

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

index XXXXXXX..XXXXXXX 100644

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

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

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

-......................

+.......................

----------------------------------------------------------------------

-Ran 22 tests

+Ran 23 tests

OK

--

1.8.3.1

From: Fam Zheng <famz@redhat.com>

This is the case in our docker tests, as we use --net=none there. Skip

this method.

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

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

tests/qemu-iotests/147 | 7 +++++++

1 file changed, 7 insertions(+)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/147

+++ b/tests/qemu-iotests/147

@@ -XXX,XX +XXX,XX @@ class BuiltinNBD(NBDBlockdevAddBase):

self._server_down()

def test_inet6(self):

+ try:

+ socket.getaddrinfo("::0", "0", socket.AF_INET6,

+ socket.SOCK_STREAM, socket.IPPROTO_TCP,

+ socket.AI_ADDRCONFIG | socket.AI_CANONNAME)

+ except socket.gaierror:

+ # IPv6 not available, skip

+ return

address = { 'type': 'inet',

'data': {

'host': '::1',

--

1.8.3.1

From: Stephen Bates <sbates@raithlin.com>

Implement NVMe Controller Memory Buffers (CMBs) which were added in

version 1.2 of the NVMe Specification. This patch adds an optional

argument (cmb_size_mb) which indicates the size of the CMB (in

MB). Currently only the Submission Queue Support (SQS) is enabled

which aligns with the current Linux driver for NVMe.

Signed-off-by: Stephen Bates <sbates@raithlin.com>

Acked-by: Keith Busch <keith.busch@intel.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

hw/block/nvme.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---

hw/block/nvme.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++

2 files changed, 144 insertions(+), 4 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/nvme.c

+++ b/hw/block/nvme.c

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

*/

/**

- * Reference Specs: http://www.nvmexpress.org, 1.1, 1.0e

+ * Reference Specs: http://www.nvmexpress.org, 1.2, 1.1, 1.0e

*

* http://www.nvmexpress.org/resources/

*/

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

/**

* Usage: add options:

* -drive file=<file>,if=none,id=<drive_id>

- * -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>

+ * -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \

+ * cmb_size_mb=<cmb_size_mb[optional]>

+ *

+ * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at

+ * offset 0 in BAR2 and supports SQS only for now.

*/

#include "qemu/osdep.h"

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

static void nvme_process_sq(void *opaque);

+static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)

+{

+ if (n->cmbsz && addr >= n->ctrl_mem.addr &&

+ addr < (n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size))) {

+ memcpy(buf, (void *)&n->cmbuf[addr - n->ctrl_mem.addr], size);

+ } else {

+ pci_dma_read(&n->parent_obj, addr, buf, size);

+ }

+}

+

static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)

{

return sqid < n->num_queues && n->sq[sqid] != NULL ? 0 : -1;

@@ -XXX,XX +XXX,XX @@ static void nvme_process_sq(void *opaque)

while (!(nvme_sq_empty(sq) || QTAILQ_EMPTY(&sq->req_list))) {

addr = sq->dma_addr + sq->head * n->sqe_size;

- pci_dma_read(&n->parent_obj, addr, (void *)&cmd, sizeof(cmd));

+ nvme_addr_read(n, addr, (void *)&cmd, sizeof(cmd));

nvme_inc_sq_head(sq);

req = QTAILQ_FIRST(&sq->req_list);

@@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps nvme_mmio_ops = {

},

};

+static void nvme_cmb_write(void *opaque, hwaddr addr, uint64_t data,

+ unsigned size)

+{

+ NvmeCtrl *n = (NvmeCtrl *)opaque;

+ memcpy(&n->cmbuf[addr], &data, size);

+}

+

+static uint64_t nvme_cmb_read(void *opaque, hwaddr addr, unsigned size)

+{

+ uint64_t val;

+ NvmeCtrl *n = (NvmeCtrl *)opaque;

+

+ memcpy(&val, &n->cmbuf[addr], size);

+ return val;

+}

+

+static const MemoryRegionOps nvme_cmb_ops = {

+ .read = nvme_cmb_read,

+ .write = nvme_cmb_write,

+ .endianness = DEVICE_LITTLE_ENDIAN,

+ .impl = {

+ .min_access_size = 2,

+ .max_access_size = 8,

+ },

+};

+

static int nvme_init(PCIDevice *pci_dev)

{

NvmeCtrl *n = NVME(pci_dev);

@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)

NVME_CAP_SET_CSS(n->bar.cap, 1);

NVME_CAP_SET_MPSMAX(n->bar.cap, 4);

- n->bar.vs = 0x00010100;

+ n->bar.vs = 0x00010200;

n->bar.intmc = n->bar.intms = 0;

+ if (n->cmb_size_mb) {

+

+ NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2);

+ NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);

+

+ NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);

+ NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);

+ NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);

+ NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 0);

+ NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 0);

+ NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */

+ NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->cmb_size_mb);

+

+ n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));

+ memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,

+ "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));

+ pci_register_bar(&n->parent_obj, NVME_CMBLOC_BIR(n->bar.cmbloc),

+ PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |

+ PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);

+

+ }

+

for (i = 0; i < n->num_namespaces; i++) {

NvmeNamespace *ns = &n->namespaces[i];

NvmeIdNs *id_ns = &ns->id_ns;

@@ -XXX,XX +XXX,XX @@ static void nvme_exit(PCIDevice *pci_dev)

g_free(n->namespaces);

g_free(n->cq);

g_free(n->sq);

+ if (n->cmbsz) {

+ memory_region_unref(&n->ctrl_mem);

+ }

+

msix_uninit_exclusive_bar(pci_dev);

}

static Property nvme_props[] = {

DEFINE_BLOCK_PROPERTIES(NvmeCtrl, conf),

DEFINE_PROP_STRING("serial", NvmeCtrl, serial),

+ DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, cmb_size_mb, 0),

DEFINE_PROP_END_OF_LIST(),

};

diff --git a/hw/block/nvme.h b/hw/block/nvme.h

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/nvme.h

+++ b/hw/block/nvme.h

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

uint32_t aqa;

uint64_t asq;

uint64_t acq;

+ uint32_t cmbloc;

+ uint32_t cmbsz;

} NvmeBar;

enum NvmeCapShift {

@@ -XXX,XX +XXX,XX @@ enum NvmeAqaMask {

#define NVME_AQA_ASQS(aqa) ((aqa >> AQA_ASQS_SHIFT) & AQA_ASQS_MASK)

#define NVME_AQA_ACQS(aqa) ((aqa >> AQA_ACQS_SHIFT) & AQA_ACQS_MASK)

+enum NvmeCmblocShift {

+ CMBLOC_BIR_SHIFT = 0,

+ CMBLOC_OFST_SHIFT = 12,

+};

+

+enum NvmeCmblocMask {

+ CMBLOC_BIR_MASK = 0x7,

+ CMBLOC_OFST_MASK = 0xfffff,

+};

+

+#define NVME_CMBLOC_BIR(cmbloc) ((cmbloc >> CMBLOC_BIR_SHIFT) & \

+ CMBLOC_BIR_MASK)

+#define NVME_CMBLOC_OFST(cmbloc)((cmbloc >> CMBLOC_OFST_SHIFT) & \

+ CMBLOC_OFST_MASK)

+

+#define NVME_CMBLOC_SET_BIR(cmbloc, val) \

+ (cmbloc |= (uint64_t)(val & CMBLOC_BIR_MASK) << CMBLOC_BIR_SHIFT)

+#define NVME_CMBLOC_SET_OFST(cmbloc, val) \

+ (cmbloc |= (uint64_t)(val & CMBLOC_OFST_MASK) << CMBLOC_OFST_SHIFT)

+

+enum NvmeCmbszShift {

+ CMBSZ_SQS_SHIFT = 0,

+ CMBSZ_CQS_SHIFT = 1,

+ CMBSZ_LISTS_SHIFT = 2,

+ CMBSZ_RDS_SHIFT = 3,

+ CMBSZ_WDS_SHIFT = 4,

+ CMBSZ_SZU_SHIFT = 8,

+ CMBSZ_SZ_SHIFT = 12,

+};

+

+enum NvmeCmbszMask {

+ CMBSZ_SQS_MASK = 0x1,

+ CMBSZ_CQS_MASK = 0x1,

+ CMBSZ_LISTS_MASK = 0x1,

+ CMBSZ_RDS_MASK = 0x1,

+ CMBSZ_WDS_MASK = 0x1,

+ CMBSZ_SZU_MASK = 0xf,

+ CMBSZ_SZ_MASK = 0xfffff,

+};

+

+#define NVME_CMBSZ_SQS(cmbsz) ((cmbsz >> CMBSZ_SQS_SHIFT) & CMBSZ_SQS_MASK)

+#define NVME_CMBSZ_CQS(cmbsz) ((cmbsz >> CMBSZ_CQS_SHIFT) & CMBSZ_CQS_MASK)

+#define NVME_CMBSZ_LISTS(cmbsz)((cmbsz >> CMBSZ_LISTS_SHIFT) & CMBSZ_LISTS_MASK)

+#define NVME_CMBSZ_RDS(cmbsz) ((cmbsz >> CMBSZ_RDS_SHIFT) & CMBSZ_RDS_MASK)

+#define NVME_CMBSZ_WDS(cmbsz) ((cmbsz >> CMBSZ_WDS_SHIFT) & CMBSZ_WDS_MASK)

+#define NVME_CMBSZ_SZU(cmbsz) ((cmbsz >> CMBSZ_SZU_SHIFT) & CMBSZ_SZU_MASK)

+#define NVME_CMBSZ_SZ(cmbsz) ((cmbsz >> CMBSZ_SZ_SHIFT) & CMBSZ_SZ_MASK)

+

+#define NVME_CMBSZ_SET_SQS(cmbsz, val) \

+ (cmbsz |= (uint64_t)(val & CMBSZ_SQS_MASK) << CMBSZ_SQS_SHIFT)

+#define NVME_CMBSZ_SET_CQS(cmbsz, val) \

+ (cmbsz |= (uint64_t)(val & CMBSZ_CQS_MASK) << CMBSZ_CQS_SHIFT)

+#define NVME_CMBSZ_SET_LISTS(cmbsz, val) \

+ (cmbsz |= (uint64_t)(val & CMBSZ_LISTS_MASK) << CMBSZ_LISTS_SHIFT)

+#define NVME_CMBSZ_SET_RDS(cmbsz, val) \

+ (cmbsz |= (uint64_t)(val & CMBSZ_RDS_MASK) << CMBSZ_RDS_SHIFT)

+#define NVME_CMBSZ_SET_WDS(cmbsz, val) \

+ (cmbsz |= (uint64_t)(val & CMBSZ_WDS_MASK) << CMBSZ_WDS_SHIFT)

+#define NVME_CMBSZ_SET_SZU(cmbsz, val) \

+ (cmbsz |= (uint64_t)(val & CMBSZ_SZU_MASK) << CMBSZ_SZU_SHIFT)

+#define NVME_CMBSZ_SET_SZ(cmbsz, val) \

+ (cmbsz |= (uint64_t)(val & CMBSZ_SZ_MASK) << CMBSZ_SZ_SHIFT)

+

+#define NVME_CMBSZ_GETSIZE(cmbsz) \

+ (NVME_CMBSZ_SZ(cmbsz) * (1 << (12 + 4 * NVME_CMBSZ_SZU(cmbsz))))

+

typedef struct NvmeCmd {

uint8_t opcode;

uint8_t fuse;

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

typedef struct NvmeCtrl {

PCIDevice parent_obj;

MemoryRegion iomem;

+ MemoryRegion ctrl_mem;

NvmeBar bar;

BlockConf conf;

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

uint32_t num_queues;

uint32_t max_q_ents;

uint64_t ns_size;

+ uint32_t cmb_size_mb;

+ uint32_t cmbsz;

+ uint32_t cmbloc;

+ uint8_t *cmbuf;

char *serial;

NvmeNamespace *namespaces;

--

1.8.3.1

This fixes an assertion failure that was triggered by qemu-iotests 129

on some CI host, while the same test case didn't seem to fail on other

hosts.

Essentially the problem is that the blk_unref(s->target) in

mirror_exit() doesn't necessarily mean that the BlockBackend goes away

immediately. It is possible that the job completion was triggered nested

in mirror_drain(), which looks like this:

BlockBackend *target = s->target;

blk_ref(target);

blk_drain(target);

blk_unref(target);

In this case, the write permissions for s->target are retained until

after blk_drain(), which makes removing mirror_top_bs fail for the

active commit case (can't have a writable backing file in the chain

without the filter driver).

Explicitly dropping the permissions first means that the additional

reference doesn't hurt and the job can complete successfully even if

called from the nested blk_drain().

Cc: qemu-stable@nongnu.org

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

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

---

block/mirror.c | 7 ++++++-

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

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

index XXXXXXX..XXXXXXX 100644

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ static void mirror_exit(BlockJob *job, void *opaque)

/* Remove target parent that still uses BLK_PERM_WRITE/RESIZE before

* inserting target_bs at s->to_replace, where we might not be able to get

- * these permissions. */

+ * these permissions.

+ *

+ * Note that blk_unref() alone doesn't necessarily drop permissions because

+ * we might be running nested inside mirror_drain(), which takes an extra

+ * reference, so use an explicit blk_set_perm() first. */

+ blk_set_perm(s->target, 0, BLK_PERM_ALL, &error_abort);

blk_unref(s->target);

s->target = NULL;

--

1.8.3.1

From: Alberto Garcia <berto@igalia.com>

Commit d7086422b1c1e75e320519cfe26176db6ec97a37 added a local_err

variable global to the qcow2_amend_options() function, so there's no

need to have this other one.

Signed-off-by: Alberto Garcia <berto@igalia.com>

Message-id: 20170511150337.21470-1-berto@igalia.com

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

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

---

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

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

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

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,

if (s->refcount_bits != refcount_bits) {

int refcount_order = ctz32(refcount_bits);

- Error *local_error = NULL;

if (new_version < 3 && refcount_bits != 16) {

error_report("Different refcount widths than 16 bits require "

@@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,

helper_cb_info.current_operation = QCOW2_CHANGING_REFCOUNT_ORDER;

ret = qcow2_change_refcount_order(bs, refcount_order,

&qcow2_amend_helper_cb,

- &helper_cb_info, &local_error);

+ &helper_cb_info, &local_err);

if (ret < 0) {

- error_report_err(local_error);

+ error_report_err(local_err);

return ret;

}

}

--

1.8.3.1

From: Fam Zheng <famz@redhat.com>

It got lost in commit a8d16f9ca "qemu-img: Update documentation for -U".

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

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

Message-id: 20170515103551.31313-1-famz@redhat.com

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

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

---

qemu-img-cmds.hx | 4 ++--

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

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("convert", img_convert,

- "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")

+ "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")

STEXI

-@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}

+@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}

ETEXI

DEF("dd", img_dd,

--

1.8.3.1

From: "Daniel P. Berrange" <berrange@redhat.com>

The qemu-img dd command added --image-opts support, but missed

the corresponding --object support. This prevented passing

secrets (eg auth passwords) needed by certain disk images.

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

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

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

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Message-id: 20170515164712.6643-2-berrange@redhat.com

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

---

qemu-img.c | 18 ++++++++++++++++++

1 file changed, 18 insertions(+)

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 int img_dd(int argc, char **argv)

};

const struct option long_options[] = {

{ "help", no_argument, 0, 'h'},

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

{ "image-opts", no_argument, 0, OPTION_IMAGE_OPTS},

{ "force-share", no_argument, 0, 'U'},

{ 0, 0, 0, 0 }

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

case 'U':

force_share = true;

break;

+ case OPTION_OBJECT: {

+ QemuOpts *opts;

+ opts = qemu_opts_parse_noisily(&qemu_object_opts,

+ optarg, true);

+ if (!opts) {

+ ret = -1;

+ goto out;

+ }

+ } break;

case OPTION_IMAGE_OPTS:

image_opts = true;

break;

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

ret = -1;

goto out;

}

+

+ if (qemu_opts_foreach(&qemu_object_opts,

+ user_creatable_add_opts_foreach,

+ NULL, NULL)) {

+ ret = -1;

+ goto out;

+ }

+

blk1 = img_open(image_opts, in.filename, fmt, 0, false, false,

force_share);

--

1.8.3.1

From: "Daniel P. Berrange" <berrange@redhat.com>

The --image-opts flag can only be used to affect the parsing

of the source image. The target image has to be specified in

the traditional style regardless, since it needs to be passed

to the bdrv_create() API which does not support the new style

opts.

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

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

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Message-id: 20170515164712.6643-3-berrange@redhat.com

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

---

qemu-img.c | 9 +++++++--

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

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 int img_dd(int argc, char **argv)

goto out;

}

- blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,

- false, false, false);

+ /* TODO, we can't honour --image-opts for the target,

+ * since it needs to be given in a format compatible

+ * with the bdrv_create() call above which does not

+ * support image-opts style.

+ */

+ blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,

+ false, false, false);

if (!blk2) {

ret = -1;

--

1.8.3.1

From: "Daniel P. Berrange" <berrange@redhat.com>

The '--image-opts' flag indicates whether the source filename

includes options. The target filename has to remain in the

plain filename format though, since it needs to be passed to

bdrv_create(). When using --skip-create though, it would be

possible to use image-opts syntax. This adds --target-image-opts

to indicate that the target filename includes options. Currently

this mandates use of the --skip-create flag too.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Message-id: 20170515164712.6643-4-berrange@redhat.com

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

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

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

---

qemu-img-cmds.hx | 4 +--

qemu-img.c | 84 ++++++++++++++++++++++++++++++++++++++------------------

qemu-img.texi | 12 ++++++--

3 files changed, 69 insertions(+), 31 deletions(-)

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("convert", img_convert,

- "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")

+ "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")

STEXI

-@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}

+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}

ETEXI

DEF("dd", img_dd,

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 @@ enum {

OPTION_PATTERN = 260,

OPTION_FLUSH_INTERVAL = 261,

OPTION_NO_DRAIN = 262,

+ OPTION_TARGET_IMAGE_OPTS = 263,

};

typedef enum OutputFormat {

@@ -XXX,XX +XXX,XX @@ static int convert_do_copy(ImgConvertState *s)

static int img_convert(int argc, char **argv)

{

int c, bs_i, flags, src_flags = 0;

- const char *fmt = NULL, *out_fmt = "raw", *cache = "unsafe",

+ const char *fmt = NULL, *out_fmt = NULL, *cache = "unsafe",

*src_cache = BDRV_DEFAULT_CACHE, *out_baseimg = NULL,

*out_filename, *out_baseimg_param, *snapshot_name = NULL;

- BlockDriver *drv, *proto_drv;

+ BlockDriver *drv = NULL, *proto_drv = NULL;

BlockDriverInfo bdi;

BlockDriverState *out_bs;

QemuOpts *opts = NULL, *sn_opts = NULL;

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

char *options = NULL;

Error *local_err = NULL;

bool writethrough, src_writethrough, quiet = false, image_opts = false,

- skip_create = false, progress = false;

+ skip_create = false, progress = false, tgt_image_opts = false;

int64_t ret = -EINVAL;

bool force_share = false;

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

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

{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},

{"force-share", no_argument, 0, 'U'},

+ {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},

{0, 0, 0, 0}

};

c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:WU",

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

case OPTION_IMAGE_OPTS:

image_opts = true;

break;

+ case OPTION_TARGET_IMAGE_OPTS:

+ tgt_image_opts = true;

+ break;

}

}

+ if (!out_fmt && !tgt_image_opts) {

+ out_fmt = "raw";

+ }

+

if (qemu_opts_foreach(&qemu_object_opts,

user_creatable_add_opts_foreach,

NULL, NULL)) {

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

goto fail_getopt;

}

+ if (tgt_image_opts && !skip_create) {

+ error_report("--target-image-opts requires use of -n flag");

+ goto fail_getopt;

+ }

+

s.src_num = argc - optind - 1;

out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;

if (options && has_help_option(options)) {

- ret = print_block_option_help(out_filename, out_fmt);

- goto fail_getopt;

+ if (out_fmt) {

+ ret = print_block_option_help(out_filename, out_fmt);

+ goto fail_getopt;

+ } else {

+ error_report("Option help requires a format be specified");

+ goto fail_getopt;

+ }

}

if (s.src_num < 1) {

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

goto out;

}

- /* Find driver and parse its options */

- drv = bdrv_find_format(out_fmt);

- if (!drv) {

- error_report("Unknown file format '%s'", out_fmt);

- ret = -1;

- goto out;

- }

+ if (!skip_create) {

+ /* Find driver and parse its options */

+ drv = bdrv_find_format(out_fmt);

+ if (!drv) {

+ error_report("Unknown file format '%s'", out_fmt);

+ ret = -1;

+ goto out;

+ }

- proto_drv = bdrv_find_protocol(out_filename, true, &local_err);

- if (!proto_drv) {

- error_report_err(local_err);

- ret = -1;

- goto out;

- }

+ proto_drv = bdrv_find_protocol(out_filename, true, &local_err);

+ if (!proto_drv) {

+ error_report_err(local_err);

+ ret = -1;

+ goto out;

+ }

- if (!skip_create) {

if (!drv->create_opts) {

error_report("Format driver '%s' does not support image creation",

drv->format_name);

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

const char *preallocation =

qemu_opt_get(opts, BLOCK_OPT_PREALLOC);

- if (!drv->bdrv_co_pwritev_compressed) {

+ if (drv && !drv->bdrv_co_pwritev_compressed) {

error_report("Compression not supported for this file format");

ret = -1;

goto out;

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

goto out;

}

- /* XXX we should allow --image-opts to trigger use of

- * img_open() here, but then we have trouble with

- * the bdrv_create() call which takes different params.

- * Not critical right now, so fix can wait...

- */

- s.target = img_open_file(out_filename, out_fmt, flags, writethrough, quiet,

- false);

+ if (skip_create) {

+ s.target = img_open(tgt_image_opts, out_filename, out_fmt,

+ flags, writethrough, quiet, false);

+ } else {

+ /* TODO ultimately we should allow --target-image-opts

+ * to be used even when -n is not given.

+ * That has to wait for bdrv_create to be improved

+ * to allow filenames in option syntax

+ */

+ s.target = img_open_file(out_filename, out_fmt, flags,

+ writethrough, quiet, false);

+ }

if (!s.target) {

ret = -1;

goto out;

}

out_bs = blk_bs(s.target);

+ if (s.compressed && !out_bs->drv->bdrv_co_pwritev_compressed) {

+ error_report("Compression not supported for this file format");

+ ret = -1;

+ goto out;

+ }

+

/* increase bufsectors from the default 4096 (2M) if opt_transfer

* or discard_alignment of the out_bs is greater. Limit to 32768 (16MB)

* as maximum. */

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 @@ keys.

@item --image-opts

-Indicates that the @var{filename} parameter is to be interpreted as a

+Indicates that the source @var{filename} parameter is to be interpreted as a

full option string, not a plain filename. This parameter is mutually

-exclusive with the @var{-f} and @var{-F} parameters.

+exclusive with the @var{-f} parameter.

+

+@item --target-image-opts

+

+Indicates that the @var{output_filename} parameter(s) are to be interpreted as

+a full option string, not a plain filename. This parameter is mutually

+exclusive with the @var{-O} parameters. It is currently required to also use

+the @var{-n} parameter to skip image creation. This restriction may be relaxed

+in a future release.

@item fmt

is the disk image format. It is guessed automatically in most cases. See below

--

1.8.3.1

From: "Daniel P. Berrange" <berrange@redhat.com>

The qemu-img dd/convert commands will create an image file and

then try to open it. Historically it has been possible to open

new files without passing any options. With encrypted files

though, the *key-secret options are mandatory, so we need to

provide those options when opening the newly created file.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Message-id: 20170515164712.6643-5-berrange@redhat.com

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

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

---

qemu-img.c | 42 +++++++++++++++++++++++++++++++++++++-----

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

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 BlockBackend *img_open_opts(const char *optstr,

}

static BlockBackend *img_open_file(const char *filename,

+ QDict *options,

const char *fmt, int flags,

bool writethrough, bool quiet,

bool force_share)

{

BlockBackend *blk;

Error *local_err = NULL;

- QDict *options = qdict_new();

+ if (!options) {

+ options = qdict_new();

+ }

if (fmt) {

qdict_put_str(options, "driver", fmt);

}

@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_file(const char *filename,

}

+static int img_add_key_secrets(void *opaque,

+ const char *name, const char *value,

+ Error **errp)

+{

+ QDict *options = opaque;

+

+ if (g_str_has_suffix(name, "key-secret")) {

+ qdict_put(options, name, qstring_from_str(value));

+ }

+

+ return 0;

+}

+

+static BlockBackend *img_open_new_file(const char *filename,

+ QemuOpts *create_opts,

+ const char *fmt, int flags,

+ bool writethrough, bool quiet,

+ bool force_share)

+{

+ QDict *options = NULL;

+

+ options = qdict_new();

+ qemu_opt_foreach(create_opts, img_add_key_secrets, options, &error_abort);

+

+ return img_open_file(filename, options, fmt, flags, writethrough, quiet,

+ force_share);

+}

+

+

static BlockBackend *img_open(bool image_opts,

const char *filename,

const char *fmt, int flags, bool writethrough,

@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open(bool image_opts,

blk = img_open_opts(filename, opts, flags, writethrough, quiet,

force_share);

} else {

- blk = img_open_file(filename, fmt, flags, writethrough, quiet,

+ blk = img_open_file(filename, NULL, fmt, flags, writethrough, quiet,

force_share);

}

return blk;

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

* That has to wait for bdrv_create to be improved

* to allow filenames in option syntax

*/

- s.target = img_open_file(out_filename, out_fmt, flags,

- writethrough, quiet, false);

+ s.target = img_open_new_file(out_filename, opts, out_fmt,

+ flags, writethrough, quiet, false);

}

if (!s.target) {

ret = -1;

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

* with the bdrv_create() call above which does not

* support image-opts style.

*/

- blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,

+ blk2 = img_open_file(out.filename, NULL, out_fmt, BDRV_O_RDWR,

false, false, false);

if (!blk2) {

--

1.8.3.1

From: Fam Zheng <famz@redhat.com>

Reported by Coverity.

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

Message-id: 20170515141014.25793-1-famz@redhat.com

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

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

---

qemu-img.c | 1 +

1 file changed, 1 insertion(+)

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 BlockBackend *img_open_opts(const char *optstr,

if (qdict_haskey(options, BDRV_OPT_FORCE_SHARE)

&& !qdict_get_bool(options, BDRV_OPT_FORCE_SHARE)) {

error_report("--force-share/-U conflicts with image options");

+ QDECREF(options);

return NULL;

}

qdict_put(options, BDRV_OPT_FORCE_SHARE, qbool_from_bool(true));

--

1.8.3.1

From: Eric Blake <eblake@redhat.com>

When converting a 1.1 image down to 0.10, qemu-iotests 060 forces

a contrived failure where allocating a cluster used to replace a

zero cluster reads unaligned data. Since it is a zero cluster

rather than a data cluster being converted, changing the error

message to match our earlier change in 'qcow2: Make distinction

between zero cluster types obvious' is worthwhile.

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

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

Message-id: 20170508171302.17805-1-eblake@redhat.com

[mreitz: Commit message fixes]

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

---

block/qcow2-cluster.c | 3 ++-

tests/qemu-iotests/060.out | 2 +-

2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-cluster.c

+++ b/block/qcow2-cluster.c

@@ -XXX,XX +XXX,XX @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,

}

if (offset_into_cluster(s, offset)) {

- qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset "

+ qcow2_signal_corruption(bs, true, -1, -1,

+ "Cluster allocation offset "

"%#" PRIx64 " unaligned (L2 offset: %#"

PRIx64 ", L2 index: %#x)", offset,

l2_offset, j);

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ read failed: Input/output error

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

wrote 65536/65536 bytes at offset 0

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

-qcow2: Marking image as corrupt: Data cluster offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed

+qcow2: Marking image as corrupt: Cluster allocation offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed

qemu-img: Error while amending options: Input/output error

=== Testing unaligned reftable entry ===

--

1.8.3.1

From: Max Reitz <mreitz@redhat.com>

path_combine() naturally tries to preserve a protocol prefix. However,

it recognizes such a prefix by scanning for the first colon; which is

different from what path_has_protocol() does: There only is a protocol

prefix if there is a colon before the first slash.

A protocol prefix that is not recognized by path_has_protocol() is none,

and should thus not be taken as one.

Case in point, before this patch:

$ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2

qemu-img: ./top:image.qcow2: Could not open './top:backing.qcow2':

No such file or directory

Afterwards:

$ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2

qemu-img: ./top:image.qcow2: Could not open './backing.qcow2':

No such file or directory

Reported-by: yangyang <yangyang@redhat.com>

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

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

Message-id: 20170522195217.12991-2-mreitz@redhat.com

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

---

block.c | 15 ++++++++++-----

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

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

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ void path_combine(char *dest, int dest_size,

if (path_is_absolute(filename)) {

pstrcpy(dest, dest_size, filename);

} else {

- p = strchr(base_path, ':');

- if (p)

- p++;

- else

- p = base_path;

+ const char *protocol_stripped = NULL;

+

+ if (path_has_protocol(base_path)) {

+ protocol_stripped = strchr(base_path, ':');

+ if (protocol_stripped) {

+ protocol_stripped++;

+ }

+ }

+ p = protocol_stripped ?: base_path;

+

p1 = strrchr(base_path, '/');

#ifdef _WIN32

{

--

1.8.3.1

From: Max Reitz <mreitz@redhat.com>

The file drivers' *_parse_filename() implementations just strip the

optional protocol prefix off the filename. However, for e.g.

"file:foo:bar", this would lead to "foo:bar" being stored as the BDS's

filename which looks like it should be managed using the "foo" protocol.

This is especially troublesome if you then try to resolve a backing

filename based on "foo:bar".

This issue can only occur if the stripped part is a relative filename

("file:/foo:bar" will be shortened to "/foo:bar" and having a slash

before the first colon means that "/foo" is not recognized as a protocol

part). Therefore, we can easily fix it by prepending "./" to such

filenames.

Before this patch:

$ ./qemu-img create -f qcow2 backing.qcow2 64M

Formatting 'backing.qcow2', fmt=qcow2 size=67108864 encryption=off

cluster_size=65536 lazy_refcounts=off refcount_bits=16

$ ./qemu-img create -f qcow2 -b backing.qcow2 file:top:image.qcow2

Formatting 'file:top:image.qcow2', fmt=qcow2 size=67108864

backing_file=backing.qcow2 encryption=off cluster_size=65536

lazy_refcounts=off refcount_bits=16

$ ./qemu-io file:top:image.qcow2

can't open device file:top:image.qcow2: Could not open backing file:

Unknown protocol 'top'

After this patch:

$ ./qemu-io file:top:image.qcow2

[no error]

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

Message-id: 20170522195217.12991-3-mreitz@redhat.com

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

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

---

block.c | 35 +++++++++++++++++++++++++++++++++++

block/file-posix.c | 17 +++--------------

block/file-win32.c | 12 ++----------

include/block/block_int.h | 3 +++

4 files changed, 43 insertions(+), 24 deletions(-)

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

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ void path_combine(char *dest, int dest_size,

}

}

+/*

+ * Helper function for bdrv_parse_filename() implementations to remove optional

+ * protocol prefixes (especially "file:") from a filename and for putting the

+ * stripped filename into the options QDict if there is such a prefix.

+ */

+void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,

+ QDict *options)

+{

+ if (strstart(filename, prefix, &filename)) {

+ /* Stripping the explicit protocol prefix may result in a protocol

+ * prefix being (wrongly) detected (if the filename contains a colon) */

+ if (path_has_protocol(filename)) {

+ QString *fat_filename;

+

+ /* This means there is some colon before the first slash; therefore,

+ * this cannot be an absolute path */

+ assert(!path_is_absolute(filename));

+

+ /* And we can thus fix the protocol detection issue by prefixing it

+ * by "./" */

+ fat_filename = qstring_from_str("./");

+ qstring_append(fat_filename, filename);

+

+ assert(!path_has_protocol(qstring_get_str(fat_filename)));

+

+ qdict_put(options, "filename", fat_filename);

+ } else {

+ /* If no protocol prefix was detected, we can use the shortened

+ * filename as-is */

+ qdict_put_str(options, "filename", filename);

+ }

+ }

+}

+

+

/* Returns whether the image file is opened as read-only. Note that this can