* [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images
@ 2013-09-05 8:10 Max Reitz
2013-09-05 8:10 ` [Qemu-devel] [RFC 1/3] bdrv: Use "Error" for opening images Max Reitz
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Max Reitz @ 2013-09-05 8:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
This RFC adds an Error ** parameter to bdrv_open, bdrv_file_open,
bdrv_create and the respective functions provided by a block driver.
This results in more specific error information than just -errno provided
to the user when opening or creating images (disregarding the fact that
block drivers often already use error_report, which is generally changed
to error_setg through this patch).
The last patch in this series changes the qcow2 block driver to set an
example of usage in a block driver.
Note that several I/O tests break by applying this RFC since they expect
different error messages (generally, previously, an error message on
image opening/creation consisted of two lines; the first of which would be
generated by the driver through error_report, the second by the block
layer itself through strerror(-ret); this patch is designed to merge these
two lines into a single one). This applies to the tests 49, 51, 54 and 60.
Max Reitz (3):
bdrv: Use "Error" for opening images
block: Error parameter for opening functions
qcow2: Use Error parameter
block.c | 164 ++++++++++++++++++++++++++++++----------------
block/blkdebug.c | 5 +-
block/blkverify.c | 7 +-
block/bochs.c | 3 +-
block/cloop.c | 3 +-
block/cow.c | 10 +--
block/dmg.c | 3 +-
block/mirror.c | 5 +-
block/nbd.c | 3 +-
block/parallels.c | 3 +-
block/qcow.c | 10 +--
block/qcow2.c | 143 ++++++++++++++++++++++++++--------------
block/qed.c | 13 ++--
block/raw-posix.c | 18 +++--
block/raw_bsd.c | 8 ++-
block/sheepdog.c | 10 +--
block/snapshot.c | 2 +-
block/vdi.c | 6 +-
block/vhdx.c | 3 +-
block/vmdk.c | 11 ++--
block/vpc.c | 6 +-
block/vvfat.c | 7 +-
blockdev.c | 30 ++++-----
include/block/block.h | 11 ++--
include/block/block_int.h | 9 ++-
qemu-img.c | 39 +++++------
qemu-io.c | 14 ++--
qemu-nbd.c | 6 +-
28 files changed, 343 insertions(+), 209 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [RFC 1/3] bdrv: Use "Error" for opening images
2013-09-05 8:10 [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images Max Reitz
@ 2013-09-05 8:10 ` Max Reitz
2013-09-05 8:42 ` Kevin Wolf
2013-09-05 8:10 ` [Qemu-devel] [RFC 2/3] block: Error parameter for opening functions Max Reitz
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Max Reitz @ 2013-09-05 8:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add an Error ** parameter to bdrv_open, bdrv_file_open and bdrv_create
to allow more specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 6 +++---
block/blkdebug.c | 3 ++-
block/blkverify.c | 3 ++-
block/bochs.c | 3 ++-
block/cloop.c | 3 ++-
block/cow.c | 6 ++++--
block/dmg.c | 3 ++-
block/nbd.c | 3 ++-
block/parallels.c | 3 ++-
block/qcow.c | 6 ++++--
block/qcow2.c | 8 +++++---
block/qed.c | 8 +++++---
block/raw-posix.c | 18 ++++++++++++------
block/raw_bsd.c | 6 ++++--
block/sheepdog.c | 6 ++++--
block/snapshot.c | 2 +-
block/vdi.c | 6 ++++--
block/vhdx.c | 3 ++-
block/vmdk.c | 6 ++++--
block/vpc.c | 6 ++++--
block/vvfat.c | 3 ++-
include/block/block_int.h | 9 ++++++---
22 files changed, 78 insertions(+), 42 deletions(-)
diff --git a/block.c b/block.c
index 26639e8..f485906 100644
--- a/block.c
+++ b/block.c
@@ -374,7 +374,7 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
CreateCo *cco = opaque;
assert(cco->drv);
- cco->ret = cco->drv->bdrv_create(cco->filename, cco->options);
+ cco->ret = cco->drv->bdrv_create(cco->filename, cco->options, NULL);
}
int bdrv_create(BlockDriver *drv, const char* filename,
@@ -734,7 +734,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
if (drv->bdrv_file_open) {
assert(file == NULL);
assert(drv->bdrv_parse_filename || filename != NULL);
- ret = drv->bdrv_file_open(bs, options, open_flags);
+ ret = drv->bdrv_file_open(bs, options, open_flags, NULL);
} else {
if (file == NULL) {
qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
@@ -744,7 +744,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
goto free_and_fail;
}
bs->file = file;
- ret = drv->bdrv_open(bs, options, open_flags);
+ ret = drv->bdrv_open(bs, options, open_flags, NULL);
}
if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 5d33e03..52d65ff 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -350,7 +350,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags)
+static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVBlkdebugState *s = bs->opaque;
QemuOpts *opts;
diff --git a/block/blkverify.c b/block/blkverify.c
index 1d58cc3..5d716bb 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -116,7 +116,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int blkverify_open(BlockDriverState *bs, QDict *options, int flags)
+static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVBlkverifyState *s = bs->opaque;
QemuOpts *opts;
diff --git a/block/bochs.c b/block/bochs.c
index d7078c0..51d9a90 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -108,7 +108,8 @@ static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int bochs_open(BlockDriverState *bs, QDict *options, int flags)
+static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVBochsState *s = bs->opaque;
int i;
diff --git a/block/cloop.c b/block/cloop.c
index 6ea7cf4..b907023 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -53,7 +53,8 @@ static int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int cloop_open(BlockDriverState *bs, QDict *options, int flags)
+static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVCloopState *s = bs->opaque;
uint32_t offsets_size, max_compressed_block_size = 1, i;
diff --git a/block/cow.c b/block/cow.c
index 1cc2e89..207fea1 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -58,7 +58,8 @@ static int cow_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int cow_open(BlockDriverState *bs, QDict *options, int flags)
+static int cow_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVCowState *s = bs->opaque;
struct cow_header_v2 cow_header;
@@ -255,7 +256,8 @@ static void cow_close(BlockDriverState *bs)
{
}
-static int cow_create(const char *filename, QEMUOptionParameter *options)
+static int cow_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
struct cow_header_v2 cow_header;
struct stat st;
diff --git a/block/dmg.c b/block/dmg.c
index 3141cb5..d5e9b1f 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -92,7 +92,8 @@ static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
return 0;
}
-static int dmg_open(BlockDriverState *bs, QDict *options, int flags)
+static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVDMGState *s = bs->opaque;
uint64_t info_begin,info_end,last_in_offset,last_out_offset;
diff --git a/block/nbd.c b/block/nbd.c
index 691066f..c8deeee 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -453,7 +453,8 @@ static void nbd_teardown_connection(BlockDriverState *bs)
closesocket(s->sock);
}
-static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
+static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVNBDState *s = bs->opaque;
int result;
diff --git a/block/parallels.c b/block/parallels.c
index 18b3ac0..2121e43 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -68,7 +68,8 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam
return 0;
}
-static int parallels_open(BlockDriverState *bs, QDict *options, int flags)
+static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVParallelsState *s = bs->opaque;
int i;
diff --git a/block/qcow.c b/block/qcow.c
index 5239bd6..6f7e73d 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -92,7 +92,8 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int qcow_open(BlockDriverState *bs, QDict *options, int flags)
+static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVQcowState *s = bs->opaque;
int len, i, shift, ret;
@@ -651,7 +652,8 @@ static void qcow_close(BlockDriverState *bs)
error_free(s->migration_blocker);
}
-static int qcow_create(const char *filename, QEMUOptionParameter *options)
+static int qcow_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int header_size, backing_filename_len, l1_size, shift, i;
QCowHeader header;
diff --git a/block/qcow2.c b/block/qcow2.c
index 4bc679a..98d1551 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -350,7 +350,8 @@ static QemuOptsList qcow2_runtime_opts = {
},
};
-static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
+static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVQcowState *s = bs->opaque;
int len, i, ret = 0;
@@ -1049,7 +1050,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs)
qbool_from_int(s->use_lazy_refcounts));
memset(s, 0, sizeof(BDRVQcowState));
- qcow2_open(bs, options, flags);
+ qcow2_open(bs, options, flags, NULL);
QDECREF(options);
@@ -1456,7 +1457,8 @@ out:
return ret;
}
-static int qcow2_create(const char *filename, QEMUOptionParameter *options)
+static int qcow2_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
const char *backing_file = NULL;
const char *backing_fmt = NULL;
diff --git a/block/qed.c b/block/qed.c
index cc904c4..1a9d01b 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -373,7 +373,8 @@ static void bdrv_qed_rebind(BlockDriverState *bs)
s->bs = bs;
}
-static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags)
+static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVQEDState *s = bs->opaque;
QEDHeader le_header;
@@ -603,7 +604,8 @@ out:
return ret;
}
-static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options)
+static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
uint64_t image_size = 0;
uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
@@ -1526,7 +1528,7 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs)
bdrv_qed_close(bs);
memset(s, 0, sizeof(BDRVQEDState));
- bdrv_qed_open(bs, NULL, bs->open_flags);
+ bdrv_qed_open(bs, NULL, bs->open_flags, NULL);
}
static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index ba721d3..22c8303 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -335,7 +335,8 @@ fail:
return ret;
}
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
@@ -1040,7 +1041,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
return (int64_t)st.st_blocks * 512;
}
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd;
int result = 0;
@@ -1325,7 +1327,8 @@ static int check_hdev_writable(BDRVRawState *s)
return 0;
}
-static int hdev_open(BlockDriverState *bs, QDict *options, int flags)
+static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
int ret;
@@ -1498,7 +1501,8 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs,
cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV);
}
-static int hdev_create(const char *filename, QEMUOptionParameter *options)
+static int hdev_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd;
int ret = 0;
@@ -1559,7 +1563,8 @@ static BlockDriver bdrv_host_device = {
};
#ifdef __linux__
-static int floppy_open(BlockDriverState *bs, QDict *options, int flags)
+static int floppy_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
int ret;
@@ -1680,7 +1685,8 @@ static BlockDriver bdrv_host_floppy = {
.bdrv_eject = floppy_eject,
};
-static int cdrom_open(BlockDriverState *bs, QDict *options, int flags)
+static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index ab2b0fd..793121a 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -130,12 +130,14 @@ static int raw_has_zero_init(BlockDriverState *bs)
return bdrv_has_zero_init(bs->file);
}
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
return bdrv_create_file(filename, options);
}
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
bs->sg = bs->file->sg;
return 0;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 1ad4d07..b1a0f7a 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1242,7 +1242,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int sd_open(BlockDriverState *bs, QDict *options, int flags)
+static int sd_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
int ret, fd;
uint32_t vid = 0;
@@ -1437,7 +1438,8 @@ out:
return ret;
}
-static int sd_create(const char *filename, QEMUOptionParameter *options)
+static int sd_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int ret = 0;
uint32_t vid = 0, base_vid = 0;
diff --git a/block/snapshot.c b/block/snapshot.c
index 6c6d9de..51b4b96 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -97,7 +97,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
if (bs->file) {
drv->bdrv_close(bs);
ret = bdrv_snapshot_goto(bs->file, snapshot_id);
- open_ret = drv->bdrv_open(bs, NULL, bs->open_flags);
+ open_ret = drv->bdrv_open(bs, NULL, bs->open_flags, NULL);
if (open_ret < 0) {
bdrv_delete(bs->file);
bs->drv = NULL;
diff --git a/block/vdi.c b/block/vdi.c
index 8a91525..c22f94c 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -364,7 +364,8 @@ static int vdi_probe(const uint8_t *buf, int buf_size, const char *filename)
return result;
}
-static int vdi_open(BlockDriverState *bs, QDict *options, int flags)
+static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVVdiState *s = bs->opaque;
VdiHeader header;
@@ -633,7 +634,8 @@ static int vdi_co_write(BlockDriverState *bs,
return ret;
}
-static int vdi_create(const char *filename, QEMUOptionParameter *options)
+static int vdi_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd;
int result = 0;
diff --git a/block/vhdx.c b/block/vhdx.c
index e9704b1..b8aa49c 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -715,7 +715,8 @@ exit:
}
-static int vhdx_open(BlockDriverState *bs, QDict *options, int flags)
+static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVVHDXState *s = bs->opaque;
int ret = 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index 63b489d..eb8bba2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -806,7 +806,8 @@ exit:
return ret;
}
-static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
+static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
int ret;
BDRVVmdkState *s = bs->opaque;
@@ -1534,7 +1535,8 @@ static int filename_decompose(const char *filename, char *path, char *prefix,
return VMDK_OK;
}
-static int vmdk_create(const char *filename, QEMUOptionParameter *options)
+static int vmdk_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd, idx = 0;
char desc[BUF_SIZE];
diff --git a/block/vpc.c b/block/vpc.c
index fe4f311..db61274 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -155,7 +155,8 @@ static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int vpc_open(BlockDriverState *bs, QDict *options, int flags)
+static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVVPCState *s = bs->opaque;
int i;
@@ -683,7 +684,8 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size)
return ret;
}
-static int vpc_create(const char *filename, QEMUOptionParameter *options)
+static int vpc_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
uint8_t buf[1024];
struct vhd_footer *footer = (struct vhd_footer *) buf;
diff --git a/block/vvfat.c b/block/vvfat.c
index cd3b8ed..d10159d 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1065,7 +1065,8 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
qdict_put(options, "rw", qbool_from_int(rw));
}
-static int vvfat_open(BlockDriverState *bs, QDict *options, int flags)
+static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVVVFATState *s = bs->opaque;
int cyls, heads, secs;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8012e25..84001b8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -97,15 +97,18 @@ struct BlockDriver {
void (*bdrv_reopen_commit)(BDRVReopenState *reopen_state);
void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
- int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags);
- int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags);
+ int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp);
+ int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp);
int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
void (*bdrv_close)(BlockDriverState *bs);
void (*bdrv_rebind)(BlockDriverState *bs);
- int (*bdrv_create)(const char *filename, QEMUOptionParameter *options);
+ int (*bdrv_create)(const char *filename, QEMUOptionParameter *options,
+ Error **errp);
int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
int (*bdrv_make_empty)(BlockDriverState *bs);
/* aio */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [RFC 2/3] block: Error parameter for opening functions
2013-09-05 8:10 [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images Max Reitz
2013-09-05 8:10 ` [Qemu-devel] [RFC 1/3] bdrv: Use "Error" for opening images Max Reitz
@ 2013-09-05 8:10 ` Max Reitz
2013-09-05 8:10 ` [Qemu-devel] [RFC 3/3] qcow2: Use Error parameter Max Reitz
2013-09-05 10:40 ` [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images Kevin Wolf
3 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2013-09-05 8:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add an Error ** parameter to bdrv_open, bdrv_file_open, bdrv_create and
associated functions to allow more specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 164 ++++++++++++++++++++++++++++++++------------------
block/blkdebug.c | 2 +-
block/blkverify.c | 4 +-
block/cow.c | 4 +-
block/mirror.c | 5 +-
block/qcow.c | 4 +-
block/qcow2.c | 6 +-
block/qed.c | 5 +-
block/raw_bsd.c | 2 +-
block/sheepdog.c | 4 +-
block/vmdk.c | 5 +-
block/vvfat.c | 4 +-
blockdev.c | 30 ++++-----
include/block/block.h | 11 ++--
qemu-img.c | 39 ++++++------
qemu-io.c | 14 +++--
qemu-nbd.c | 6 +-
17 files changed, 183 insertions(+), 126 deletions(-)
diff --git a/block.c b/block.c
index f485906..d734265 100644
--- a/block.c
+++ b/block.c
@@ -367,18 +367,26 @@ typedef struct CreateCo {
char *filename;
QEMUOptionParameter *options;
int ret;
+ Error *err;
} CreateCo;
static void coroutine_fn bdrv_create_co_entry(void *opaque)
{
+ Error *local_err = NULL;
+ int ret;
+
CreateCo *cco = opaque;
assert(cco->drv);
- cco->ret = cco->drv->bdrv_create(cco->filename, cco->options, NULL);
+ ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(&cco->err, local_err);
+ }
+ cco->ret = ret;
}
int bdrv_create(BlockDriver *drv, const char* filename,
- QEMUOptionParameter *options)
+ QEMUOptionParameter *options, Error **errp)
{
int ret;
@@ -388,9 +396,11 @@ int bdrv_create(BlockDriver *drv, const char* filename,
.filename = g_strdup(filename),
.options = options,
.ret = NOT_DONE,
+ .err = NULL,
};
if (!drv->bdrv_create) {
+ error_setg(errp, "Driver '%s' does not support image creation", drv->format_name);
ret = -ENOTSUP;
goto out;
}
@@ -407,22 +417,37 @@ int bdrv_create(BlockDriver *drv, const char* filename,
}
ret = cco.ret;
+ if (ret < 0) {
+ if (error_is_set(&cco.err)) {
+ error_propagate(errp, cco.err);
+ } else {
+ error_setg_errno(errp, -ret, "Could not create image");
+ }
+ }
out:
g_free(cco.filename);
return ret;
}
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
+int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
+ Error **errp)
{
BlockDriver *drv;
+ Error *local_err = NULL;
+ int ret;
drv = bdrv_find_protocol(filename, true);
if (drv == NULL) {
+ error_setg(errp, "Could not find protocol for file '%s'", filename);
return -ENOENT;
}
- return bdrv_create(drv, filename, options);
+ ret = bdrv_create(drv, filename, options, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
+ return ret;
}
/*
@@ -525,7 +550,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
}
static int find_image_format(BlockDriverState *bs, const char *filename,
- BlockDriver **pdrv)
+ BlockDriver **pdrv, Error **errp)
{
int score, score_max;
BlockDriver *drv1, *drv;
@@ -536,6 +561,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
drv = bdrv_find_format("raw");
if (!drv) {
+ error_setg(errp, "Could not find raw image format");
ret = -ENOENT;
}
*pdrv = drv;
@@ -544,6 +570,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
ret = bdrv_pread(bs, 0, buf, sizeof(buf));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read image for determining its "
+ "format");
*pdrv = NULL;
return ret;
}
@@ -560,6 +588,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
}
}
if (!drv) {
+ error_setg(errp, "Could not determine image format: No compatible "
+ "driver found");
ret = -ENOENT;
}
*pdrv = drv;
@@ -679,10 +709,11 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
* Removes all processed options from *options.
*/
static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
- QDict *options, int flags, BlockDriver *drv)
+ QDict *options, int flags, BlockDriver *drv, Error **errp)
{
int ret, open_flags;
const char *filename;
+ Error *local_err = NULL;
assert(drv != NULL);
assert(bs->file == NULL);
@@ -711,6 +742,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
bs->read_only = !(open_flags & BDRV_O_RDWR);
if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
+ error_setg(errp, "Driver '%s' is not whitelisted", drv->format_name);
return -ENOTSUP;
}
@@ -734,25 +766,30 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
if (drv->bdrv_file_open) {
assert(file == NULL);
assert(drv->bdrv_parse_filename || filename != NULL);
- ret = drv->bdrv_file_open(bs, options, open_flags, NULL);
+ ret = drv->bdrv_file_open(bs, options, open_flags, &local_err);
} else {
if (file == NULL) {
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
- "block driver for the protocol level",
- drv->format_name);
+ error_setg(errp, "Can't use '%s' as a block driver for the "
+ "protocol level", drv->format_name);
ret = -EINVAL;
goto free_and_fail;
}
bs->file = file;
- ret = drv->bdrv_open(bs, options, open_flags, NULL);
+ ret = drv->bdrv_open(bs, options, open_flags, &local_err);
}
if (ret < 0) {
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ } else {
+ error_setg_errno(errp, -ret, "Could not open '%s'", filename);
+ }
goto free_and_fail;
}
ret = refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not refresh total sector count");
goto free_and_fail;
}
@@ -781,12 +818,13 @@ free_and_fail:
* dictionary, it needs to use QINCREF() before calling bdrv_file_open.
*/
int bdrv_file_open(BlockDriverState **pbs, const char *filename,
- QDict *options, int flags)
+ QDict *options, int flags, Error **errp)
{
BlockDriverState *bs;
BlockDriver *drv;
const char *drvname;
bool allow_protocol_prefix = false;
+ Error *local_err = NULL;
int ret;
/* NULL means an empty set of options */
@@ -805,8 +843,8 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
qdict_put(options, "filename", qstring_from_str(filename));
allow_protocol_prefix = true;
} else {
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't specify 'file' and "
- "'filename' options at the same time");
+ error_setg(errp, "Can't specify 'file' and 'filename' options at the "
+ "same time");
ret = -EINVAL;
goto fail;
}
@@ -815,53 +853,53 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
drvname = qdict_get_try_str(options, "driver");
if (drvname) {
drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR));
+ if (!drv) {
+ error_setg(errp, "Unknown driver '%s'", drvname);
+ }
qdict_del(options, "driver");
} else if (filename) {
drv = bdrv_find_protocol(filename, allow_protocol_prefix);
if (!drv) {
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Unknown protocol");
+ error_setg(errp, "Unknown protocol");
}
} else {
- qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "Must specify either driver or file");
+ error_setg(errp, "Must specify either driver or file");
drv = NULL;
}
if (!drv) {
+ /* errp has been set already */
ret = -ENOENT;
goto fail;
}
/* Parse the filename and open it */
if (drv->bdrv_parse_filename && filename) {
- Error *local_err = NULL;
drv->bdrv_parse_filename(filename, options, &local_err);
if (error_is_set(&local_err)) {
- qerror_report_err(local_err);
- error_free(local_err);
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto fail;
}
qdict_del(options, "filename");
} else if (!drv->bdrv_parse_filename && !filename) {
- qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "The '%s' block driver requires a file name",
- drv->format_name);
+ error_setg(errp, "The '%s' block driver requires a file name",
+ drv->format_name);
ret = -EINVAL;
goto fail;
}
- ret = bdrv_open_common(bs, NULL, options, flags, drv);
+ ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
goto fail;
}
/* Check if any unknown options were used */
if (qdict_size(options) != 0) {
const QDictEntry *entry = qdict_first(options);
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block protocol '%s' doesn't "
- "support the option '%s'",
- drv->format_name, entry->key);
+ error_setg(errp, "Block protocol '%s' doesn't support the option '%s'",
+ drv->format_name, entry->key);
ret = -EINVAL;
goto fail;
}
@@ -888,11 +926,12 @@ fail:
* function (even on failure), so if the caller intends to reuse the dictionary,
* it needs to use QINCREF() before calling bdrv_file_open.
*/
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
{
char backing_filename[PATH_MAX];
int back_flags, ret;
BlockDriver *back_drv = NULL;
+ Error *local_err = NULL;
if (bs->backing_hd != NULL) {
QDECREF(options);
@@ -925,11 +964,12 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
ret = bdrv_open(bs->backing_hd,
*backing_filename ? backing_filename : NULL, options,
- back_flags, back_drv);
+ back_flags, back_drv, &local_err);
if (ret < 0) {
bdrv_delete(bs->backing_hd);
bs->backing_hd = NULL;
bs->open_flags |= BDRV_O_NO_BACKING;
+ error_propagate(errp, local_err);
return ret;
}
return 0;
@@ -963,7 +1003,7 @@ static void extract_subqdict(QDict *src, QDict **dst, const char *start)
* dictionary, it needs to use QINCREF() before calling bdrv_open.
*/
int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
- int flags, BlockDriver *drv)
+ int flags, BlockDriver *drv, Error **errp)
{
int ret;
/* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
@@ -971,6 +1011,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
BlockDriverState *file = NULL;
QDict *file_options = NULL;
const char *drvname;
+ Error *local_err = NULL;
/* NULL means an empty set of options */
if (options == NULL) {
@@ -989,7 +1030,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
char backing_filename[PATH_MAX];
if (qdict_size(options) != 0) {
- error_report("Can't use snapshot=on with driver-specific options");
+ error_setg(errp, "Can't use snapshot=on with driver-specific options");
ret = -EINVAL;
goto fail;
}
@@ -1000,7 +1041,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
/* if there is a backing file, use it */
bs1 = bdrv_new("");
- ret = bdrv_open(bs1, filename, NULL, 0, drv);
+ ret = bdrv_open(bs1, filename, NULL, 0, drv, &local_err);
if (ret < 0) {
bdrv_delete(bs1);
goto fail;
@@ -1011,6 +1052,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not get temporary filename");
goto fail;
}
@@ -1019,6 +1061,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
snprintf(backing_filename, sizeof(backing_filename),
"%s", filename);
} else if (!realpath(filename, backing_filename)) {
+ error_setg_errno(errp, errno, "Could not resolve path '%s'", filename);
ret = -errno;
goto fail;
}
@@ -1035,7 +1078,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
drv->format_name);
}
- ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
+ ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
free_option_parameters(create_options);
if (ret < 0) {
goto fail;
@@ -1054,7 +1097,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
extract_subqdict(options, &file_options, "file.");
ret = bdrv_file_open(&file, filename, file_options,
- bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
+ bdrv_open_flags(bs, flags | BDRV_O_UNMAP), &local_err);
if (ret < 0) {
goto fail;
}
@@ -1067,7 +1110,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
}
if (!drv) {
- ret = find_image_format(file, filename, &drv);
+ ret = find_image_format(file, filename, &drv, &local_err);
}
if (!drv) {
@@ -1075,7 +1118,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
}
/* Open the image */
- ret = bdrv_open_common(bs, file, options, flags, drv);
+ ret = bdrv_open_common(bs, file, options, flags, drv, &local_err);
if (ret < 0) {
goto unlink_and_fail;
}
@@ -1090,7 +1133,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
QDict *backing_options;
extract_subqdict(options, &backing_options, "backing.");
- ret = bdrv_open_backing_file(bs, backing_options);
+ ret = bdrv_open_backing_file(bs, backing_options, &local_err);
if (ret < 0) {
goto close_and_fail;
}
@@ -1099,9 +1142,9 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
/* Check if any unknown options were used */
if (qdict_size(options) != 0) {
const QDictEntry *entry = qdict_first(options);
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by "
- "device '%s' doesn't support the option '%s'",
- drv->format_name, bs->device_name, entry->key);
+ error_setg(errp, "Block format '%s' used by device '%s' doesn't "
+ "support the option '%s'", drv->format_name, bs->device_name,
+ entry->key);
ret = -EINVAL;
goto close_and_fail;
@@ -1130,11 +1173,17 @@ fail:
QDECREF(bs->options);
QDECREF(options);
bs->options = NULL;
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
return ret;
close_and_fail:
bdrv_close(bs);
QDECREF(options);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
return ret;
}
@@ -4527,6 +4576,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
BlockDriverState *bs = NULL;
BlockDriver *drv, *proto_drv;
BlockDriver *backing_drv = NULL;
+ Error *local_err = NULL;
int ret = 0;
/* Find driver and parse its options */
@@ -4613,10 +4663,8 @@ void bdrv_img_create(const char *filename, const char *fmt,
bs = bdrv_new("");
ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
- backing_drv);
+ backing_drv, &local_err);
if (ret < 0) {
- error_setg_errno(errp, -ret, "Could not open '%s'",
- backing_file->value.s);
goto out;
}
bdrv_get_geometry(bs, &size);
@@ -4635,22 +4683,19 @@ void bdrv_img_create(const char *filename, const char *fmt,
print_option_parameters(param);
puts("");
}
- ret = bdrv_create(drv, filename, param);
- if (ret < 0) {
- if (ret == -ENOTSUP) {
- error_setg(errp,"Formatting or formatting option not supported for "
- "file format '%s'", fmt);
- } else if (ret == -EFBIG) {
- const char *cluster_size_hint = "";
- if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
- cluster_size_hint = " (try using a larger cluster size)";
- }
- error_setg(errp, "The image size is too large for file format '%s'%s",
- fmt, cluster_size_hint);
- } else {
- error_setg(errp, "%s: error while creating %s: %s", filename, fmt,
- strerror(-ret));
+ ret = bdrv_create(drv, filename, param, &local_err);
+ if (ret == -EFBIG) {
+ /* This is generally a better message than whatever the driver would
+ * deliver (especially because of the cluster_size_hint), since that
+ * is most probably not much different from "image too large". */
+ const char *cluster_size_hint = "";
+ if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
+ cluster_size_hint = " (try using a larger cluster size)";
}
+ error_setg(errp, "The image size is too large for file format '%s'"
+ "%s", fmt, cluster_size_hint);
+ error_free(local_err);
+ local_err = NULL;
}
out:
@@ -4660,6 +4705,9 @@ out:
if (bs) {
bdrv_delete(bs);
}
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
}
AioContext *bdrv_get_aio_context(BlockDriverState *bs)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 52d65ff..e0ba16d 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -387,7 +387,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- ret = bdrv_file_open(&bs->file, filename, NULL, flags);
+ ret = bdrv_file_open(&bs->file, filename, NULL, flags, NULL);
if (ret < 0) {
goto fail;
}
diff --git a/block/blkverify.c b/block/blkverify.c
index 5d716bb..d428efd 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -141,7 +141,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- ret = bdrv_file_open(&bs->file, raw, NULL, flags);
+ ret = bdrv_file_open(&bs->file, raw, NULL, flags, NULL);
if (ret < 0) {
goto fail;
}
@@ -154,7 +154,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
}
s->test_file = bdrv_new("");
- ret = bdrv_open(s->test_file, filename, NULL, flags, NULL);
+ ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, NULL);
if (ret < 0) {
bdrv_delete(s->test_file);
s->test_file = NULL;
diff --git a/block/cow.c b/block/cow.c
index 207fea1..1c12996 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -276,12 +276,12 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
options++;
}
- ret = bdrv_create_file(filename, options);
+ ret = bdrv_create_file(filename, options, NULL);
if (ret < 0) {
return ret;
}
- ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR);
+ ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, NULL);
if (ret < 0) {
return ret;
}
diff --git a/block/mirror.c b/block/mirror.c
index 86de458..b85326b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -505,14 +505,15 @@ static void mirror_iostatus_reset(BlockJob *job)
static void mirror_complete(BlockJob *job, Error **errp)
{
MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+ Error *local_err = NULL;
int ret;
- ret = bdrv_open_backing_file(s->target, NULL);
+ ret = bdrv_open_backing_file(s->target, NULL, &local_err);
if (ret < 0) {
char backing_filename[PATH_MAX];
bdrv_get_full_backing_filename(s->target, backing_filename,
sizeof(backing_filename));
- error_setg_file_open(errp, -ret, backing_filename);
+ error_propagate(errp, local_err);
return;
}
if (!s->synced) {
diff --git a/block/qcow.c b/block/qcow.c
index 6f7e73d..50c6657 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -676,12 +676,12 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
options++;
}
- ret = bdrv_create_file(filename, options);
+ ret = bdrv_create_file(filename, options, NULL);
if (ret < 0) {
return ret;
}
- ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR);
+ ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, NULL);
if (ret < 0) {
return ret;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 98d1551..e16d352 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1354,12 +1354,12 @@ static int qcow2_create2(const char *filename, int64_t total_size,
uint8_t* refcount_table;
int ret;
- ret = bdrv_create_file(filename, options);
+ ret = bdrv_create_file(filename, options, NULL);
if (ret < 0) {
return ret;
}
- ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR);
+ ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
if (ret < 0) {
return ret;
}
@@ -1412,7 +1412,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
BlockDriver* drv = bdrv_find_format("qcow2");
assert(drv != NULL);
ret = bdrv_open(bs, filename, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv);
+ BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, NULL);
if (ret < 0) {
goto out;
}
diff --git a/block/qed.c b/block/qed.c
index 1a9d01b..3552daf 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -554,12 +554,13 @@ static int qed_create(const char *filename, uint32_t cluster_size,
int ret = 0;
BlockDriverState *bs = NULL;
- ret = bdrv_create_file(filename, NULL);
+ ret = bdrv_create_file(filename, NULL, NULL);
if (ret < 0) {
return ret;
}
- ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB);
+ ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB,
+ NULL);
if (ret < 0) {
return ret;
}
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 793121a..0f1b677 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -133,7 +133,7 @@ static int raw_has_zero_init(BlockDriverState *bs)
static int raw_create(const char *filename, QEMUOptionParameter *options,
Error **errp)
{
- return bdrv_create_file(filename, options);
+ return bdrv_create_file(filename, options, NULL);
}
static int raw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index b1a0f7a..6722771 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1403,7 +1403,7 @@ static int sd_prealloc(const char *filename)
void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
int ret;
- ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR);
+ ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
if (ret < 0) {
goto out;
}
@@ -1502,7 +1502,7 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
goto out;
}
- ret = bdrv_file_open(&bs, backing_file, NULL, 0);
+ ret = bdrv_file_open(&bs, backing_file, NULL, 0, NULL);
if (ret < 0) {
goto out;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index eb8bba2..1e1b860 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -726,7 +726,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
path_combine(extent_path, sizeof(extent_path),
desc_file_path, fname);
- ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags);
+ ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags,
+ NULL);
if (ret) {
return ret;
}
@@ -1636,7 +1637,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
}
if (backing_file) {
BlockDriverState *bs = bdrv_new("");
- ret = bdrv_open(bs, backing_file, NULL, 0, NULL);
+ ret = bdrv_open(bs, backing_file, NULL, 0, NULL, NULL);
if (ret != 0) {
bdrv_delete(bs);
return ret;
diff --git a/block/vvfat.c b/block/vvfat.c
index d10159d..65e5bd0 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2926,7 +2926,7 @@ static int enable_write_target(BDRVVVFATState *s)
set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512);
set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:");
- ret = bdrv_create(bdrv_qcow, s->qcow_filename, options);
+ ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, NULL);
if (ret < 0) {
goto err;
}
@@ -2934,7 +2934,7 @@ static int enable_write_target(BDRVVVFATState *s)
s->qcow = bdrv_new("");
ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow);
+ BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow, NULL);
if (ret < 0) {
bdrv_delete(s->qcow);
goto err;
diff --git a/blockdev.c b/blockdev.c
index e70e16e..b1b95fb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -707,17 +707,11 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
}
QINCREF(bs_opts);
- ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv);
+ ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
if (ret < 0) {
- if (ret == -EMEDIUMTYPE) {
- error_report("could not open disk image %s: not in %s format",
- file ?: dinfo->id, drv ? drv->format_name :
- qdict_get_str(bs_opts, "driver"));
- } else {
- error_report("could not open disk image %s: %s",
- file ?: dinfo->id, strerror(-ret));
- }
+ error_report("could not open disk image %s: %s",
+ file ?: dinfo->id, error_get_pretty(error));
goto err;
}
@@ -957,9 +951,9 @@ static void external_snapshot_prepare(BlkTransactionState *common,
/* TODO Inherit bs->options or only take explicit options with an
* extended QMP command? */
ret = bdrv_open(state->new_bs, new_image_file, NULL,
- flags | BDRV_O_NO_BACKING, drv);
+ flags | BDRV_O_NO_BACKING, drv, &local_err);
if (ret != 0) {
- error_setg_file_open(errp, -ret, new_image_file);
+ error_propagate(errp, local_err);
}
}
@@ -1189,11 +1183,12 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
int bdrv_flags, BlockDriver *drv,
const char *password, Error **errp)
{
+ Error *local_err = NULL;
int ret;
- ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv);
+ ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv, &local_err);
if (ret < 0) {
- error_setg_file_open(errp, -ret, filename);
+ error_propagate(errp, local_err);
return;
}
@@ -1583,10 +1578,10 @@ void qmp_drive_backup(const char *device, const char *target,
}
target_bs = bdrv_new("");
- ret = bdrv_open(target_bs, target, NULL, flags, drv);
+ ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err);
if (ret < 0) {
bdrv_delete(target_bs);
- error_setg_file_open(errp, -ret, target);
+ error_propagate(errp, local_err);
return;
}
@@ -1723,10 +1718,11 @@ void qmp_drive_mirror(const char *device, const char *target,
* file.
*/
target_bs = bdrv_new("");
- ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv);
+ ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
+ &local_err);
if (ret < 0) {
bdrv_delete(target_bs);
- error_setg_file_open(errp, -ret, target);
+ error_propagate(errp, local_err);
return;
}
diff --git a/include/block/block.h b/include/block/block.h
index e6b391c..15e226d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -117,8 +117,9 @@ BlockDriver *bdrv_find_format(const char *format_name);
BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
bool readonly);
int bdrv_create(BlockDriver *drv, const char* filename,
- QEMUOptionParameter *options);
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options);
+ QEMUOptionParameter *options, Error **errp);
+int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
+ Error **errp);
BlockDriverState *bdrv_new(const char *device_name);
void bdrv_make_anon(BlockDriverState *bs);
void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
@@ -127,10 +128,10 @@ void bdrv_delete(BlockDriverState *bs);
int bdrv_parse_cache_flags(const char *mode, int *flags);
int bdrv_parse_discard_flags(const char *mode, int *flags);
int bdrv_file_open(BlockDriverState **pbs, const char *filename,
- QDict *options, int flags);
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
+ QDict *options, int flags, Error **errp);
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
- int flags, BlockDriver *drv);
+ int flags, BlockDriver *drv, Error **errp);
BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
BlockDriverState *bs, int flags);
int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..a0c400b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -264,6 +264,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
BlockDriverState *bs;
BlockDriver *drv;
char password[256];
+ Error *local_err = NULL;
int ret;
bs = bdrv_new("image");
@@ -278,9 +279,11 @@ static BlockDriverState *bdrv_new_open(const char *filename,
drv = NULL;
}
- ret = bdrv_open(bs, filename, NULL, flags, drv);
+ ret = bdrv_open(bs, filename, NULL, flags, drv, &local_err);
if (ret < 0) {
- error_report("Could not open '%s': %s", filename, strerror(-ret));
+ error_report("Could not open '%s': %s", filename,
+ error_get_pretty(local_err));
+ error_free(local_err);
goto fail;
}
@@ -407,7 +410,7 @@ static int img_create(int argc, char **argv)
bdrv_img_create(filename, fmt, base_filename, base_fmt,
options, img_size, BDRV_O_FLAGS, &local_err, quiet);
if (error_is_set(&local_err)) {
- error_report("%s", error_get_pretty(local_err));
+ error_report("%s: %s", filename, error_get_pretty(local_err));
error_free(local_err);
return 1;
}
@@ -1133,6 +1136,7 @@ static int img_convert(int argc, char **argv)
float local_progress = 0;
int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
bool quiet = false;
+ Error *local_err = NULL;
fmt = NULL;
out_fmt = "raw";
@@ -1330,18 +1334,11 @@ static int img_convert(int argc, char **argv)
}
/* Create the new image */
- ret = bdrv_create(drv, out_filename, param);
+ ret = bdrv_create(drv, out_filename, param, &local_err);
if (ret < 0) {
- if (ret == -ENOTSUP) {
- error_report("Formatting not supported for file format '%s'",
- out_fmt);
- } else if (ret == -EFBIG) {
- error_report("The image size is too large for file format '%s'",
- out_fmt);
- } else {
- error_report("%s: error while converting %s: %s",
- out_filename, out_fmt, strerror(-ret));
- }
+ error_report("%s: error while converting %s: %s",
+ out_filename, out_fmt, error_get_pretty(local_err));
+ error_free(local_err);
goto out;
}
@@ -1912,6 +1909,7 @@ static int img_rebase(int argc, char **argv)
int unsafe = 0;
int progress = 0;
bool quiet = false;
+ Error *local_err = NULL;
/* Parse commandline parameters */
fmt = NULL;
@@ -2015,18 +2013,21 @@ static int img_rebase(int argc, char **argv)
bs_old_backing = bdrv_new("old_backing");
bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
ret = bdrv_open(bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
- old_backing_drv);
+ old_backing_drv, &local_err);
if (ret) {
- error_report("Could not open old backing file '%s'", backing_name);
+ error_report("Could not open old backing file '%s': %s",
+ backing_name, error_get_pretty(local_err));
+ error_free(local_err);
goto out;
}
if (out_baseimg[0]) {
bs_new_backing = bdrv_new("new_backing");
ret = bdrv_open(bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
- new_backing_drv);
+ new_backing_drv, &local_err);
if (ret) {
- error_report("Could not open new backing file '%s'",
- out_baseimg);
+ error_report("Could not open new backing file '%s': %s",
+ out_baseimg, error_get_pretty(local_err));
+ error_free(local_err);
goto out;
}
}
diff --git a/qemu-io.c b/qemu-io.c
index d54dc86..88f74b2 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -46,21 +46,27 @@ static const cmdinfo_t close_cmd = {
static int openfile(char *name, int flags, int growable)
{
+ Error *local_err = NULL;
+
if (qemuio_bs) {
fprintf(stderr, "file open already, try 'help close'\n");
return 1;
}
if (growable) {
- if (bdrv_file_open(&qemuio_bs, name, NULL, flags)) {
- fprintf(stderr, "%s: can't open device %s\n", progname, name);
+ if (bdrv_file_open(&qemuio_bs, name, NULL, flags, &local_err)) {
+ fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
+ error_get_pretty(local_err));
+ error_free(local_err);
return 1;
}
} else {
qemuio_bs = bdrv_new("hda");
- if (bdrv_open(qemuio_bs, name, NULL, flags, NULL) < 0) {
- fprintf(stderr, "%s: can't open device %s\n", progname, name);
+ if (bdrv_open(qemuio_bs, name, NULL, flags, NULL, &local_err) < 0) {
+ fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
+ error_get_pretty(local_err));
+ error_free(local_err);
bdrv_delete(qemuio_bs);
qemuio_bs = NULL;
return 1;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index f044546..c26c98e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -355,6 +355,7 @@ int main(int argc, char **argv)
#endif
pthread_t client_thread;
const char *fmt = NULL;
+ Error *local_err = NULL;
/* The client thread uses SIGTERM to interrupt the server. A signal
* handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -573,10 +574,11 @@ int main(int argc, char **argv)
bs = bdrv_new("hda");
srcpath = argv[optind];
- ret = bdrv_open(bs, srcpath, NULL, flags, drv);
+ ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err);
if (ret < 0) {
errno = -ret;
- err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
+ err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind],
+ error_get_pretty(local_err));
}
fd_size = bdrv_getlength(bs);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [RFC 3/3] qcow2: Use Error parameter
2013-09-05 8:10 [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images Max Reitz
2013-09-05 8:10 ` [Qemu-devel] [RFC 1/3] bdrv: Use "Error" for opening images Max Reitz
2013-09-05 8:10 ` [Qemu-devel] [RFC 2/3] block: Error parameter for opening functions Max Reitz
@ 2013-09-05 8:10 ` Max Reitz
2013-09-05 10:40 ` [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images Kevin Wolf
3 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2013-09-05 8:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Employ usage of the new Error ** parameter in qcow2_open, qcow2_create
and associated functions.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 135 ++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 88 insertions(+), 47 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index e16d352..9a96188 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1,4 +1,5 @@
/*
+ * error_setg(errp, "
* Block driver for the QCOW version 2 format
*
* Copyright (c) 2004-2006 Fabrice Bellard
@@ -79,7 +80,8 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
* return 0 upon success, non-0 otherwise
*/
static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
- uint64_t end_offset, void **p_feature_table)
+ uint64_t end_offset, void **p_feature_table,
+ Error **errp)
{
BDRVQcowState *s = bs->opaque;
QCowExtension ext;
@@ -100,10 +102,10 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
printf("attempting to read extended header in offset %lu\n", offset);
#endif
- if (bdrv_pread(bs->file, offset, &ext, sizeof(ext)) != sizeof(ext)) {
- fprintf(stderr, "qcow2_read_extension: ERROR: "
- "pread fail from offset %" PRIu64 "\n",
- offset);
+ ret = bdrv_pread(bs->file, offset, &ext, sizeof(ext));
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "qcow2_read_extension: ERROR: "
+ "pread fail from offset %" PRIu64, offset);
return 1;
}
be32_to_cpus(&ext.magic);
@@ -113,7 +115,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
printf("ext.magic = 0x%x\n", ext.magic);
#endif
if (ext.len > end_offset - offset) {
- error_report("Header extension too large");
+ error_setg(errp, "Header extension too large");
return -EINVAL;
}
@@ -123,17 +125,19 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
case QCOW2_EXT_MAGIC_BACKING_FORMAT:
if (ext.len >= sizeof(bs->backing_format)) {
- fprintf(stderr, "ERROR: ext_backing_format: len=%u too large"
- " (>=%zu)\n",
- ext.len, sizeof(bs->backing_format));
+ error_setg(errp, "ERROR: ext_backing_format: len=%u too large"
+ " (>=%zu)", ext.len, sizeof(bs->backing_format));
return 2;
}
- if (bdrv_pread(bs->file, offset , bs->backing_format,
- ext.len) != ext.len)
+ ret = bdrv_pread(bs->file, offset, bs->backing_format, ext.len);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "ERROR: ext_backing_format: "
+ "Could not read format name");
return 3;
+ }
bs->backing_format[ext.len] = '\0';
#ifdef DEBUG_EXT
- printf("Qcow2: Got format extension %s\n", bs->backing_format);
+ printf("Qcow2: Got format extension %s", bs->backing_format);
#endif
break;
@@ -142,6 +146,8 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
void* feature_table = g_malloc0(ext.len + 2 * sizeof(Qcow2Feature));
ret = bdrv_pread(bs->file, offset , feature_table, ext.len);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "ERROR: ext_feature_table: "
+ "Could not read table");
return ret;
}
@@ -161,6 +167,8 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
ret = bdrv_pread(bs->file, offset , uext->data, uext->len);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "ERROR: unknown extension: "
+ "Could not read data");
return ret;
}
}
@@ -184,8 +192,8 @@ static void cleanup_unknown_header_ext(BlockDriverState *bs)
}
}
-static void GCC_FMT_ATTR(2, 3) report_unsupported(BlockDriverState *bs,
- const char *fmt, ...)
+static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
+ Error **errp, const char *fmt, ...)
{
char msg[64];
va_list ap;
@@ -194,17 +202,17 @@ static void GCC_FMT_ATTR(2, 3) report_unsupported(BlockDriverState *bs,
vsnprintf(msg, sizeof(msg), fmt, ap);
va_end(ap);
- qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bs->device_name, "qcow2", msg);
+ error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bs->device_name, "qcow2",
+ msg);
}
static void report_unsupported_feature(BlockDriverState *bs,
- Qcow2Feature *table, uint64_t mask)
+ Error **errp, Qcow2Feature *table, uint64_t mask)
{
while (table && table->name[0] != '\0') {
if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
if (mask & (1 << table->bit)) {
- report_unsupported(bs, "%.46s",table->name);
+ report_unsupported(bs, errp, "%.46s", table->name);
mask &= ~(1 << table->bit);
}
}
@@ -212,7 +220,8 @@ static void report_unsupported_feature(BlockDriverState *bs,
}
if (mask) {
- report_unsupported(bs, "Unknown incompatible feature: %" PRIx64, mask);
+ report_unsupported(bs, errp, "Unknown incompatible feature: %" PRIx64,
+ mask);
}
}
@@ -363,6 +372,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read qcow2 header");
goto fail;
}
be32_to_cpus(&header.magic);
@@ -380,11 +390,12 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
be32_to_cpus(&header.nb_snapshots);
if (header.magic != QCOW_MAGIC) {
+ error_setg(errp, "Image is not in qcow2 format");
ret = -EMEDIUMTYPE;
goto fail;
}
if (header.version < 2 || header.version > 3) {
- report_unsupported(bs, "QCOW version %d", header.version);
+ report_unsupported(bs, errp, "QCOW version %d", header.version);
ret = -ENOTSUP;
goto fail;
}
@@ -412,6 +423,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = bdrv_pread(bs->file, sizeof(header), s->unknown_header_fields,
s->unknown_header_fields_size);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read unknown qcow2 header "
+ "fields");
goto fail;
}
}
@@ -430,8 +443,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
if (s->incompatible_features & ~QCOW2_INCOMPAT_MASK) {
void *feature_table = NULL;
qcow2_read_extensions(bs, header.header_length, ext_end,
- &feature_table);
- report_unsupported_feature(bs, feature_table,
+ &feature_table, NULL);
+ report_unsupported_feature(bs, errp, feature_table,
s->incompatible_features &
~QCOW2_INCOMPAT_MASK);
ret = -ENOTSUP;
@@ -442,8 +455,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
/* Corrupt images may not be written to unless they are being repaired
*/
if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_CHECK)) {
- error_report("qcow2: Image is corrupt; cannot be opened "
- "read/write.");
+ error_setg(errp, "qcow2: Image is corrupt; cannot be opened "
+ "read/write");
ret = -EACCES;
goto fail;
}
@@ -451,7 +464,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
/* Check support for various header values */
if (header.refcount_order != 4) {
- report_unsupported(bs, "%d bit reference counts",
+ report_unsupported(bs, errp, "%d bit reference counts",
1 << header.refcount_order);
ret = -ENOTSUP;
goto fail;
@@ -459,10 +472,13 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
if (header.cluster_bits < MIN_CLUSTER_BITS ||
header.cluster_bits > MAX_CLUSTER_BITS) {
+ error_setg(errp, "Unsupported cluster size: 2^%i", header.cluster_bits);
ret = -EINVAL;
goto fail;
}
if (header.crypt_method > QCOW_CRYPT_AES) {
+ error_setg(errp, "Unsupported encryption method: %i",
+ header.crypt_method);
ret = -EINVAL;
goto fail;
}
@@ -491,6 +507,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
l1_vm_state_index = size_to_l1(s, header.size);
if (l1_vm_state_index > INT_MAX) {
+ error_setg(errp, "Image is too big");
ret = -EFBIG;
goto fail;
}
@@ -499,6 +516,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
/* the L1 table must contain at least enough entries to put
header.size bytes */
if (s->l1_size < s->l1_vm_state_index) {
+ error_setg(errp, "L1 table is too small");
ret = -EINVAL;
goto fail;
}
@@ -509,6 +527,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table,
s->l1_size * sizeof(uint64_t));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read L1 table");
goto fail;
}
for(i = 0;i < s->l1_size; i++) {
@@ -529,6 +548,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = qcow2_refcount_init(bs);
if (ret != 0) {
+ error_setg_errno(errp, -ret, "Could not initialize refcount handling");
goto fail;
}
@@ -536,7 +556,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
QTAILQ_INIT(&s->discards);
/* read qcow2 extensions */
- if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL)) {
+ if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL,
+ &local_err)) {
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto fail;
}
@@ -550,6 +572,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = bdrv_pread(bs->file, header.backing_file_offset,
bs->backing_file, len);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read backing file name");
goto fail;
}
bs->backing_file[len] = '\0';
@@ -557,6 +580,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = qcow2_read_snapshots(bs);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read snapshots");
goto fail;
}
@@ -565,6 +589,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
s->autoclear_features = 0;
ret = qcow2_update_header(bs);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not update qcow2 header");
goto fail;
}
}
@@ -579,6 +604,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not repair dirty image");
goto fail;
}
}
@@ -587,8 +613,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
- qerror_report_err(local_err);
- error_free(local_err);
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto fail;
}
@@ -609,8 +634,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
qemu_opts_del(opts);
if (s->use_lazy_refcounts && s->qcow_version < 3) {
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Lazy refcounts require "
- "a qcow2 image with at least qemu 1.1 compatibility level");
+ error_setg(errp, "Lazy refcounts require a qcow2 image with at least "
+ "qemu 1.1 compatibility level");
ret = -EINVAL;
goto fail;
}
@@ -1323,7 +1348,8 @@ static int preallocate(BlockDriverState *bs)
static int qcow2_create2(const char *filename, int64_t total_size,
const char *backing_file, const char *backing_format,
int flags, size_t cluster_size, int prealloc,
- QEMUOptionParameter *options, int version)
+ QEMUOptionParameter *options, int version,
+ Error **errp)
{
/* Calculate cluster_bits */
int cluster_bits;
@@ -1331,9 +1357,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits > MAX_CLUSTER_BITS ||
(1 << cluster_bits) != cluster_size)
{
- error_report(
- "Cluster size must be a power of two between %d and %dk",
- 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10));
+ error_setg(errp, "Cluster size must be a power of two between %d and "
+ "%dk", 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10));
return -EINVAL;
}
@@ -1352,15 +1377,18 @@ static int qcow2_create2(const char *filename, int64_t total_size,
BlockDriverState* bs;
QCowHeader header;
uint8_t* refcount_table;
+ Error *local_err = NULL;
int ret;
- ret = bdrv_create_file(filename, options, NULL);
+ ret = bdrv_create_file(filename, options, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
return ret;
}
- ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
+ ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
return ret;
}
@@ -1390,6 +1418,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
ret = bdrv_pwrite(bs, 0, &header, sizeof(header));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not write qcow2 header");
goto out;
}
@@ -1399,6 +1428,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
g_free(refcount_table);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not write refcount table");
goto out;
}
@@ -1412,8 +1442,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
BlockDriver* drv = bdrv_find_format("qcow2");
assert(drv != NULL);
ret = bdrv_open(bs, filename, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, NULL);
+ BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
goto out;
}
@@ -1429,6 +1460,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
/* Okay, now that we have a valid image, let's give it the right size */
ret = bdrv_truncate(bs, total_size * BDRV_SECTOR_SIZE);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not resize image");
goto out;
}
@@ -1436,6 +1468,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
if (backing_file) {
ret = bdrv_change_backing_file(bs, backing_file, backing_format);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not assign backing file '%s' "
+ "with format '%s'", backing_file, backing_format);
goto out;
}
}
@@ -1447,6 +1481,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
ret = preallocate(bs);
qemu_co_mutex_unlock(&s->lock);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not preallocate metadata");
goto out;
}
}
@@ -1467,6 +1502,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
size_t cluster_size = DEFAULT_CLUSTER_SIZE;
int prealloc = 0;
int version = 3;
+ Error *local_err = NULL;
+ int ret;
/* Read out options */
while (options && options->name) {
@@ -1488,8 +1525,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
} else if (!strcmp(options->value.s, "metadata")) {
prealloc = 1;
} else {
- fprintf(stderr, "Invalid preallocation mode: '%s'\n",
- options->value.s);
+ error_setg(errp, "Invalid preallocation mode: '%s'",
+ options->value.s);
return -EINVAL;
}
} else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) {
@@ -1500,8 +1537,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
} else if (!strcmp(options->value.s, "1.1")) {
version = 3;
} else {
- fprintf(stderr, "Invalid compatibility level: '%s'\n",
- options->value.s);
+ error_setg(errp, "Invalid compatibility level: '%s'",
+ options->value.s);
return -EINVAL;
}
} else if (!strcmp(options->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
@@ -1511,19 +1548,23 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
}
if (backing_file && prealloc) {
- fprintf(stderr, "Backing file and preallocation cannot be used at "
- "the same time\n");
+ error_setg(errp, "Backing file and preallocation cannot be used at "
+ "the same time");
return -EINVAL;
}
if (version < 3 && (flags & BLOCK_FLAG_LAZY_REFCOUNTS)) {
- fprintf(stderr, "Lazy refcounts only supported with compatibility "
- "level 1.1 and above (use compat=1.1 or greater)\n");
+ error_setg(errp, "Lazy refcounts only supported with compatibility "
+ "level 1.1 and above (use compat=1.1 or greater)");
return -EINVAL;
}
- return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
- cluster_size, prealloc, options, version);
+ ret = qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
+ cluster_size, prealloc, options, version, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
+ return ret;
}
static int qcow2_make_empty(BlockDriverState *bs)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RFC 1/3] bdrv: Use "Error" for opening images
2013-09-05 8:10 ` [Qemu-devel] [RFC 1/3] bdrv: Use "Error" for opening images Max Reitz
@ 2013-09-05 8:42 ` Kevin Wolf
0 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2013-09-05 8:42 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.09.2013 um 10:10 hat Max Reitz geschrieben:
> Add an Error ** parameter to bdrv_open, bdrv_file_open and bdrv_create
> to allow more specific error messages.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 6 +++---
> block/blkdebug.c | 3 ++-
> block/blkverify.c | 3 ++-
> block/bochs.c | 3 ++-
> block/cloop.c | 3 ++-
> block/cow.c | 6 ++++--
> block/dmg.c | 3 ++-
> block/nbd.c | 3 ++-
> block/parallels.c | 3 ++-
> block/qcow.c | 6 ++++--
> block/qcow2.c | 8 +++++---
> block/qed.c | 8 +++++---
> block/raw-posix.c | 18 ++++++++++++------
> block/raw_bsd.c | 6 ++++--
> block/sheepdog.c | 6 ++++--
> block/snapshot.c | 2 +-
> block/vdi.c | 6 ++++--
> block/vhdx.c | 3 ++-
> block/vmdk.c | 6 ++++--
> block/vpc.c | 6 ++++--
> block/vvfat.c | 3 ++-
> include/block/block_int.h | 9 ++++++---
> 22 files changed, 78 insertions(+), 42 deletions(-)
Don't rely on compiler errors for changing the prototypes: There are
more block drivers which just aren't compiled on your system, either
because they are for a different platform (like raw-win32.c) or they
require headers that you don't have installed, so configure disabled
them.
Kevin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images
2013-09-05 8:10 [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images Max Reitz
` (2 preceding siblings ...)
2013-09-05 8:10 ` [Qemu-devel] [RFC 3/3] qcow2: Use Error parameter Max Reitz
@ 2013-09-05 10:40 ` Kevin Wolf
3 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2013-09-05 10:40 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.09.2013 um 10:10 hat Max Reitz geschrieben:
> This RFC adds an Error ** parameter to bdrv_open, bdrv_file_open,
> bdrv_create and the respective functions provided by a block driver.
>
> This results in more specific error information than just -errno provided
> to the user when opening or creating images (disregarding the fact that
> block drivers often already use error_report, which is generally changed
> to error_setg through this patch).
>
> The last patch in this series changes the qcow2 block driver to set an
> example of usage in a block driver.
>
> Note that several I/O tests break by applying this RFC since they expect
> different error messages (generally, previously, an error message on
> image opening/creation consisted of two lines; the first of which would be
> generated by the driver through error_report, the second by the block
> layer itself through strerror(-ret); this patch is designed to merge these
> two lines into a single one). This applies to the tests 49, 51, 54 and 60.
>
> Max Reitz (3):
> bdrv: Use "Error" for opening images
> block: Error parameter for opening functions
> qcow2: Use Error parameter
One thing I'd like to suggest is to keep the conversions of
bdrv_(file_)open and bdrv_create separate. I don't think there are
dependencies between them, so two logical changes should come in two
separate patches.
The other point is that you do a lot of "dummy" conversions where you
pass NULL as errp. This is generally not what we want to have in the end
because it swallows error messages that would previously be printed by
error_report(). I guess most of them are changed into real error
handling, but it's hard to keep track of the places that need to be
fixed in later patches.
In order to address this, I think it would be better to use code like
this when you can't forward the errors yet:
Error *local_err;
foo(&local_err);
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
error_free(local_err);
}
So that the output isn't lost in any intermediate commit of your series.
Kevin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-05 10:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-05 8:10 [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images Max Reitz
2013-09-05 8:10 ` [Qemu-devel] [RFC 1/3] bdrv: Use "Error" for opening images Max Reitz
2013-09-05 8:42 ` Kevin Wolf
2013-09-05 8:10 ` [Qemu-devel] [RFC 2/3] block: Error parameter for opening functions Max Reitz
2013-09-05 8:10 ` [Qemu-devel] [RFC 3/3] qcow2: Use Error parameter Max Reitz
2013-09-05 10:40 ` [Qemu-devel] [RFC 0/3] block: Error parameter for opening/creating images Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).