* [Qemu-devel] [RFC v2 0/6] block: Error parameter for opening/creating images
@ 2013-09-05 13:55 Max Reitz
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images Max Reitz
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Max Reitz @ 2013-09-05 13:55 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 fifth 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.
v2:
- included all block drivers
- split open/create patches (old 1 into 1/2 and old 2 into 3/4)
- added a patch (6) for printing out error messages from blk_open,
blk_file_open and blk_create in all block drivers (except qcow2, for
which there is a dedicated patch (5))
I intentionally made this an own commit since I think this to be
real functionality, whereas the patches 3 and 4 are more for
introducing Error into block.c and making sure everything compiles
with these changes (without actually adding functionality to any
other code). However, I'm well aware that this in some way breaks
the code between patches 3/4 and 6, since some error messages may be
discarded between these commits. Therefore I wouldn't object too much
to merging patch 6 with 3/4.
Max Reitz (6):
bdrv: Use "Error" for opening images
bdrv: Use "Error" for creating images
block: Error parameter for open functions
block: Error parameter for create functions
qcow2: Use Error parameter
bdrv: No silent error message discarding
block.c | 164 ++++++++++++++++++++++++++++++----------------
block/blkdebug.c | 7 +-
block/blkverify.c | 11 +++-
block/bochs.c | 3 +-
block/cloop.c | 3 +-
block/cow.c | 15 +++--
block/curl.c | 3 +-
block/dmg.c | 3 +-
block/gluster.c | 4 +-
block/iscsi.c | 6 +-
block/mirror.c | 5 +-
block/nbd.c | 3 +-
block/parallels.c | 3 +-
block/qcow.c | 15 +++--
block/qcow2.c | 143 ++++++++++++++++++++++++++--------------
block/qed.c | 18 +++--
block/raw-posix.c | 18 +++--
block/raw-win32.c | 9 ++-
block/raw_bsd.c | 16 ++++-
block/rbd.c | 6 +-
block/sheepdog.c | 14 ++--
block/snapshot.c | 2 +-
block/ssh.c | 6 +-
block/vdi.c | 6 +-
block/vhdx.c | 3 +-
block/vmdk.c | 17 +++--
block/vpc.c | 6 +-
block/vvfat.c | 13 +++-
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 +-
34 files changed, 410 insertions(+), 221 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images
2013-09-05 13:55 [Qemu-devel] [RFC v2 0/6] block: Error parameter for opening/creating images Max Reitz
@ 2013-09-05 13:55 ` Max Reitz
2013-09-06 13:08 ` Kevin Wolf
2013-09-06 13:36 ` Kevin Wolf
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 2/6] bdrv: Use "Error" for creating images Max Reitz
` (4 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: Max Reitz @ 2013-09-05 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add an Error ** parameter to BlockDriver.bdrv_open and
BlockDriver.bdrv_file_open to allow more specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 4 ++--
block/blkdebug.c | 3 ++-
block/blkverify.c | 3 ++-
block/bochs.c | 3 ++-
block/cloop.c | 3 ++-
block/cow.c | 3 ++-
block/curl.c | 3 ++-
block/dmg.c | 3 ++-
block/gluster.c | 2 +-
block/iscsi.c | 3 ++-
block/nbd.c | 3 ++-
block/parallels.c | 3 ++-
block/qcow.c | 3 ++-
block/qcow2.c | 5 +++--
block/qed.c | 5 +++--
block/raw-posix.c | 12 ++++++++----
block/raw-win32.c | 6 ++++--
block/raw_bsd.c | 6 ++++--
block/rbd.c | 3 ++-
block/sheepdog.c | 3 ++-
block/snapshot.c | 2 +-
block/ssh.c | 3 ++-
block/vdi.c | 3 ++-
block/vhdx.c | 3 ++-
block/vmdk.c | 3 ++-
block/vpc.c | 3 ++-
block/vvfat.c | 3 ++-
include/block/block_int.h | 6 ++++--
28 files changed, 68 insertions(+), 37 deletions(-)
diff --git a/block.c b/block.c
index 26639e8..c9cb371 100644
--- a/block.c
+++ b/block.c
@@ -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..4961ca0 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;
diff --git a/block/curl.c b/block/curl.c
index ca2cedc..5a46f97 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -395,7 +395,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int curl_open(BlockDriverState *bs, QDict *options, int flags)
+static int curl_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVCURLState *s = bs->opaque;
CURLState *state = NULL;
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/gluster.c b/block/gluster.c
index dbb03f4..6c7b000 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -288,7 +288,7 @@ static QemuOptsList runtime_opts = {
};
static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
- int bdrv_flags)
+ int bdrv_flags, Error **errp)
{
BDRVGlusterState *s = bs->opaque;
int open_flags = O_BINARY;
diff --git a/block/iscsi.c b/block/iscsi.c
index 2bbee1f..2464f19 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1046,7 +1046,8 @@ static QemuOptsList runtime_opts = {
* We support iscsi url's on the form
* iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
*/
-static int iscsi_open(BlockDriverState *bs, QDict *options, int flags)
+static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
struct iscsi_context *iscsi = NULL;
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..9ac9120 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;
diff --git a/block/qcow2.c b/block/qcow2.c
index 4bc679a..33168b0 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);
diff --git a/block/qed.c b/block/qed.c
index cc904c4..927f30b 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;
@@ -1526,7 +1527,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..19068b1 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;
@@ -1325,7 +1326,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;
@@ -1559,7 +1561,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 +1683,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-win32.c b/block/raw-win32.c
index 9b5b2af..395c912 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -234,7 +234,8 @@ static QemuOptsList raw_runtime_opts = {
},
};
-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;
int access_flags;
@@ -531,7 +532,8 @@ static int hdev_probe_device(const char *filename)
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 access_flags, create_flags;
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/rbd.c b/block/rbd.c
index e798e19..dc06117 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -446,7 +446,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags)
+static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRBDState *s = bs->opaque;
char pool[RBD_MAX_POOL_NAME_SIZE];
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 1ad4d07..b4a86ea 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;
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/ssh.c b/block/ssh.c
index 27691b4..23cd1f0 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -608,7 +608,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
return ret;
}
-static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags)
+static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
+ Error **errp)
{
BDRVSSHState *s = bs->opaque;
int ret;
diff --git a/block/vdi.c b/block/vdi.c
index 8a91525..df0be42 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;
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..bf424b1 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;
diff --git a/block/vpc.c b/block/vpc.c
index fe4f311..ed00370 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;
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..b05f850 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -97,8 +97,10 @@ 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,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [RFC v2 2/6] bdrv: Use "Error" for creating images
2013-09-05 13:55 [Qemu-devel] [RFC v2 0/6] block: Error parameter for opening/creating images Max Reitz
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images Max Reitz
@ 2013-09-05 13:55 ` Max Reitz
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 3/6] block: Error parameter for open functions Max Reitz
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Max Reitz @ 2013-09-05 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add an Error ** parameter to BlockDriver.bdrv_create to allow more
specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 2 +-
block/cow.c | 3 ++-
block/gluster.c | 2 +-
block/iscsi.c | 3 ++-
block/qcow.c | 3 ++-
block/qcow2.c | 3 ++-
block/qed.c | 3 ++-
block/raw-posix.c | 6 ++++--
block/raw-win32.c | 3 ++-
block/rbd.c | 3 ++-
block/sheepdog.c | 3 ++-
block/ssh.c | 3 ++-
block/vdi.c | 3 ++-
block/vmdk.c | 3 ++-
block/vpc.c | 3 ++-
include/block/block_int.h | 3 ++-
16 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/block.c b/block.c
index c9cb371..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,
diff --git a/block/cow.c b/block/cow.c
index 4961ca0..207fea1 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -256,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/gluster.c b/block/gluster.c
index 6c7b000..256de10 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -357,7 +357,7 @@ out:
}
static int qemu_gluster_create(const char *filename,
- QEMUOptionParameter *options)
+ QEMUOptionParameter *options, Error **errp)
{
struct glfs *glfs;
struct glfs_fd *fd;
diff --git a/block/iscsi.c b/block/iscsi.c
index 2464f19..3384704 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1238,7 +1238,8 @@ static int iscsi_has_zero_init(BlockDriverState *bs)
return 0;
}
-static int iscsi_create(const char *filename, QEMUOptionParameter *options)
+static int iscsi_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int ret = 0;
int64_t total_size = 0;
diff --git a/block/qcow.c b/block/qcow.c
index 9ac9120..6f7e73d 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -652,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 33168b0..98d1551 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1457,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 927f30b..1a9d01b 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -604,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;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 19068b1..22c8303 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1041,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;
@@ -1500,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;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 395c912..1b287bc 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -421,7 +421,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
return st.st_size;
}
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd;
int64_t total_size = 0;
diff --git a/block/rbd.c b/block/rbd.c
index dc06117..06cbef7 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -287,7 +287,8 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
return ret;
}
-static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options)
+static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int64_t bytes = 0;
int64_t objsize;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index b4a86ea..b1a0f7a 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1438,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/ssh.c b/block/ssh.c
index 23cd1f0..aa63c9d 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -651,7 +651,8 @@ static QEMUOptionParameter ssh_create_options[] = {
{ NULL }
};
-static int ssh_create(const char *filename, QEMUOptionParameter *options)
+static int ssh_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int r, ret;
Error *local_err = NULL;
diff --git a/block/vdi.c b/block/vdi.c
index df0be42..c22f94c 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -634,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/vmdk.c b/block/vmdk.c
index bf424b1..eb8bba2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1535,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 ed00370..db61274 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -684,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/include/block/block_int.h b/include/block/block_int.h
index b05f850..84001b8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -107,7 +107,8 @@ struct BlockDriver {
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] 13+ messages in thread
* [Qemu-devel] [RFC v2 3/6] block: Error parameter for open functions
2013-09-05 13:55 [Qemu-devel] [RFC v2 0/6] block: Error parameter for opening/creating images Max Reitz
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images Max Reitz
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 2/6] bdrv: Use "Error" for creating images Max Reitz
@ 2013-09-05 13:55 ` Max Reitz
2013-09-06 13:35 ` Kevin Wolf
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 4/6] block: Error parameter for create functions Max Reitz
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Max Reitz @ 2013-09-05 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add an Error ** parameter to bdrv_open, bdrv_file_open and associated
functions to allow more specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 98 ++++++++++++++++++++++++++++++++-------------------
block/blkdebug.c | 2 +-
block/blkverify.c | 4 +--
block/cow.c | 2 +-
block/mirror.c | 5 +--
block/qcow.c | 2 +-
block/qcow2.c | 4 +--
block/qed.c | 3 +-
block/sheepdog.c | 4 +--
block/vmdk.c | 5 +--
block/vvfat.c | 2 +-
blockdev.c | 30 +++++++---------
include/block/block.h | 6 ++--
qemu-img.c | 23 +++++++-----
qemu-io.c | 14 +++++---
qemu-nbd.c | 6 ++--
16 files changed, 125 insertions(+), 85 deletions(-)
diff --git a/block.c b/block.c
index f485906..8da32b2 100644
--- a/block.c
+++ b/block.c
@@ -525,7 +525,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 +536,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 +545,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 +563,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 +684,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 +717,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 +741,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 +793,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 +818,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 +828,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 +901,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 +939,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 +978,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 +986,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 +1005,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 +1016,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 +1027,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 +1036,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;
}
@@ -1038,6 +1056,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
free_option_parameters(create_options);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not create temporary overlay "
+ "'%s'", tmp_filename);
goto fail;
}
@@ -1054,7 +1074,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 +1087,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 +1095,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 +1110,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 +1119,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 +1150,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;
}
@@ -4613,7 +4639,7 @@ 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, NULL);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not open '%s'",
backing_file->value.s);
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..f890db0 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -281,7 +281,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
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..ab3f1ab 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -681,7 +681,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
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..8692931 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1359,7 +1359,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
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..ba1f9d8 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -559,7 +559,8 @@ static int qed_create(const char *filename, uint32_t cluster_size,
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/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..76a30a5 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -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..efafd7a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -127,10 +127,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..607981e 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;
}
@@ -1912,6 +1915,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 +2019,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] 13+ messages in thread
* [Qemu-devel] [RFC v2 4/6] block: Error parameter for create functions
2013-09-05 13:55 [Qemu-devel] [RFC v2 0/6] block: Error parameter for opening/creating images Max Reitz
` (2 preceding siblings ...)
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 3/6] block: Error parameter for open functions Max Reitz
@ 2013-09-05 13:55 ` Max Reitz
2013-09-06 13:45 ` Kevin Wolf
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 5/6] qcow2: Use Error parameter Max Reitz
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 6/6] bdrv: No silent error message discarding Max Reitz
5 siblings, 1 reply; 13+ messages in thread
From: Max Reitz @ 2013-09-05 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add an Error ** parameter to bdrv_create and its associated functions to
allow more specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 72 +++++++++++++++++++++++++++++++++------------------
block/cow.c | 2 +-
block/qcow.c | 2 +-
block/qcow2.c | 2 +-
block/qed.c | 2 +-
block/raw_bsd.c | 2 +-
block/vvfat.c | 2 +-
include/block/block.h | 5 ++--
qemu-img.c | 16 ++++--------
9 files changed, 61 insertions(+), 44 deletions(-)
diff --git a/block.c b/block.c
index 8da32b2..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;
}
/*
@@ -1053,11 +1078,9 @@ 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) {
- error_setg_errno(errp, -ret, "Could not create temporary overlay "
- "'%s'", tmp_filename);
goto fail;
}
@@ -4553,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 */
@@ -4639,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, NULL);
+ 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);
@@ -4661,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:
@@ -4686,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/cow.c b/block/cow.c
index f890db0..1c12996 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -276,7 +276,7 @@ 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;
}
diff --git a/block/qcow.c b/block/qcow.c
index ab3f1ab..50c6657 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -676,7 +676,7 @@ 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;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 8692931..e16d352 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1354,7 +1354,7 @@ 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;
}
diff --git a/block/qed.c b/block/qed.c
index ba1f9d8..3552daf 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -554,7 +554,7 @@ 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;
}
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/vvfat.c b/block/vvfat.c
index 76a30a5..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;
}
diff --git a/include/block/block.h b/include/block/block.h
index efafd7a..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);
diff --git a/qemu-img.c b/qemu-img.c
index 607981e..a0c400b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1136,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";
@@ -1333,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;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [RFC v2 5/6] qcow2: Use Error parameter
2013-09-05 13:55 [Qemu-devel] [RFC v2 0/6] block: Error parameter for opening/creating images Max Reitz
` (3 preceding siblings ...)
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 4/6] block: Error parameter for create functions Max Reitz
@ 2013-09-05 13:55 ` Max Reitz
2013-09-06 14:15 ` Kevin Wolf
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 6/6] bdrv: No silent error message discarding Max Reitz
5 siblings, 1 reply; 13+ messages in thread
From: Max Reitz @ 2013-09-05 13:55 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] 13+ messages in thread
* [Qemu-devel] [RFC v2 6/6] bdrv: No silent error message discarding
2013-09-05 13:55 [Qemu-devel] [RFC v2 0/6] block: Error parameter for opening/creating images Max Reitz
` (4 preceding siblings ...)
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 5/6] qcow2: Use Error parameter Max Reitz
@ 2013-09-05 13:55 ` Max Reitz
2013-09-06 14:22 ` Kevin Wolf
5 siblings, 1 reply; 13+ messages in thread
From: Max Reitz @ 2013-09-05 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Using NULL as errp parameters to bdrv_open, bdrv_file_open and
bdrv_create in all block drivers which do not yet implement proper error
propagation is not a good idea, since this now silently discards error
messages. Fix this by using a proper parameter and printing its content
on error (until proper propagation is implemented).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
Note that many error propagation are actually trivial (such as in
raw_bsd), but I intentionally refrained from implementing the propagation
and tenaciously followed the pattern:
Error *local_err = NULL;
foo(&local_err);
if (/*error condition*/) {
qerror_report_err(local_err):
error_free(local_err);
}
This is because the propagation may sometimes be trivial, however, it is
often still driver-specific, therefore this deserves its own patch for
every driver, in my opinion. Also, I think it is easier to review this
way. ;-)
---
block/blkdebug.c | 4 +++-
block/blkverify.c | 8 ++++++--
block/cow.c | 9 +++++++--
block/qcow.c | 9 +++++++--
block/qed.c | 9 +++++++--
block/raw_bsd.c | 10 +++++++++-
block/sheepdog.c | 8 ++++++--
block/vmdk.c | 10 ++++++++--
block/vvfat.c | 10 ++++++++--
9 files changed, 61 insertions(+), 16 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index e0ba16d..be948b2 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -387,8 +387,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- ret = bdrv_file_open(&bs->file, filename, NULL, flags, NULL);
+ ret = bdrv_file_open(&bs->file, filename, NULL, flags, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto fail;
}
diff --git a/block/blkverify.c b/block/blkverify.c
index d428efd..cceb88f 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -141,8 +141,10 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- ret = bdrv_file_open(&bs->file, raw, NULL, flags, NULL);
+ ret = bdrv_file_open(&bs->file, raw, NULL, flags, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto fail;
}
@@ -154,8 +156,10 @@ 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, NULL);
+ ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
bdrv_delete(s->test_file);
s->test_file = NULL;
goto fail;
diff --git a/block/cow.c b/block/cow.c
index 1c12996..0b93b45 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -263,6 +263,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
struct stat st;
int64_t image_sectors = 0;
const char *image_filename = NULL;
+ Error *local_err = NULL;
int ret;
BlockDriverState *cow_bs;
@@ -276,13 +277,17 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
options++;
}
- ret = bdrv_create_file(filename, options, NULL);
+ ret = bdrv_create_file(filename, options, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
- ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, NULL);
+ ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/qcow.c b/block/qcow.c
index 50c6657..9b4020f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -661,6 +661,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
int64_t total_size = 0;
const char *backing_file = NULL;
int flags = 0;
+ Error *local_err = NULL;
int ret;
BlockDriverState *qcow_bs;
@@ -676,13 +677,17 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
options++;
}
- ret = bdrv_create_file(filename, options, NULL);
+ ret = bdrv_create_file(filename, options, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
- ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, NULL);
+ ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/qed.c b/block/qed.c
index 3552daf..168a68a 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -551,17 +551,22 @@ static int qed_create(const char *filename, uint32_t cluster_size,
QEDHeader le_header;
uint8_t *l1_table = NULL;
size_t l1_size = header.cluster_size * header.table_size;
+ Error *local_err = NULL;
int ret = 0;
BlockDriverState *bs = NULL;
- ret = bdrv_create_file(filename, NULL, NULL);
+ ret = bdrv_create_file(filename, NULL, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB,
- NULL);
+ &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 0f1b677..2effe72 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -133,7 +133,15 @@ 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, NULL);
+ Error *local_err = NULL;
+ int ret;
+
+ ret = bdrv_create_file(filename, options, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ }
+ return ret;
}
static int raw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 6722771..a9d8019 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1401,9 +1401,10 @@ static int sd_prealloc(const char *filename)
uint32_t idx, max_idx;
int64_t vdi_size;
void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
+ Error *local_err = NULL;
int 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) {
goto out;
}
@@ -1449,6 +1450,7 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN];
uint32_t snapid;
bool prealloc = false;
+ Error *local_err = NULL;
s = g_malloc0(sizeof(BDRVSheepdogState));
@@ -1502,8 +1504,10 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
goto out;
}
- ret = bdrv_file_open(&bs, backing_file, NULL, 0, NULL);
+ ret = bdrv_file_open(&bs, backing_file, NULL, 0, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto out;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index 1e1b860..e62be63 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -697,6 +697,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
int64_t flat_offset;
char extent_path[PATH_MAX];
BlockDriverState *extent_file;
+ Error *local_err = NULL;
while (*p) {
/* parse extent line:
@@ -727,8 +728,10 @@ 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,
- NULL);
+ &local_err);
if (ret) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
@@ -1575,6 +1578,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
"ddb.geometry.heads = \"%d\"\n"
"ddb.geometry.sectors = \"63\"\n"
"ddb.adapterType = \"%s\"\n";
+ Error *local_err = NULL;
if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
return -EINVAL;
@@ -1637,8 +1641,10 @@ 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, NULL);
+ ret = bdrv_open(bs, backing_file, NULL, 0, NULL, &local_err);
if (ret != 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
bdrv_delete(bs);
return ret;
}
diff --git a/block/vvfat.c b/block/vvfat.c
index 65e5bd0..fcb97f4 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2909,6 +2909,7 @@ static int enable_write_target(BDRVVVFATState *s)
{
BlockDriver *bdrv_qcow;
QEMUOptionParameter *options;
+ Error *local_err = NULL;
int ret;
int size = sector2cluster(s, s->sector_count);
s->used_clusters = calloc(size, 1);
@@ -2926,16 +2927,21 @@ 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, NULL);
+ ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto err;
}
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, NULL);
+ BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
+ &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
bdrv_delete(s->qcow);
goto err;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images Max Reitz
@ 2013-09-06 13:08 ` Kevin Wolf
2013-09-06 13:36 ` Kevin Wolf
1 sibling, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2013-09-06 13:08 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> Add an Error ** parameter to BlockDriver.bdrv_open and
> BlockDriver.bdrv_file_open to allow more specific error messages.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 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;
raw_create() shouldn't be converted in this patch, this causes a build
failure.
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC v2 3/6] block: Error parameter for open functions
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 3/6] block: Error parameter for open functions Max Reitz
@ 2013-09-06 13:35 ` Kevin Wolf
0 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2013-09-06 13:35 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> Add an Error ** parameter to bdrv_open, bdrv_file_open and associated
> functions to allow more specific error messages.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 98 ++++++++++++++++++++++++++++++++-------------------
> block/blkdebug.c | 2 +-
> block/blkverify.c | 4 +--
> block/cow.c | 2 +-
> block/mirror.c | 5 +--
> block/qcow.c | 2 +-
> block/qcow2.c | 4 +--
> block/qed.c | 3 +-
> block/sheepdog.c | 4 +--
> block/vmdk.c | 5 +--
> block/vvfat.c | 2 +-
> blockdev.c | 30 +++++++---------
> include/block/block.h | 6 ++--
> qemu-img.c | 23 +++++++-----
> qemu-io.c | 14 +++++---
> qemu-nbd.c | 6 ++--
> 16 files changed, 125 insertions(+), 85 deletions(-)
>
> diff --git a/block.c b/block.c
> index f485906..8da32b2 100644
> --- a/block.c
> +++ b/block.c
> @@ -525,7 +525,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 +536,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 +545,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 +563,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 +684,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 +717,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 +741,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);
filename can be NULL. This happens in cases like:
-drive file.driver=nbd,file.host=localhost
> + }
> 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;
> }
>
> diff --git a/qemu-img.c b/qemu-img.c
> index b9a848d..607981e 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));
This is an independent change. :-)
It's probably also better handled with the loc_*() functions that define
an area where some given information (like the filename) is prepended to
all error messages printed by error_report().
> error_free(local_err);
> return 1;
> }
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images Max Reitz
2013-09-06 13:08 ` Kevin Wolf
@ 2013-09-06 13:36 ` Kevin Wolf
1 sibling, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2013-09-06 13:36 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> Add an Error ** parameter to BlockDriver.bdrv_open and
> BlockDriver.bdrv_file_open to allow more specific error messages.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 2bbee1f..2464f19 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1046,7 +1046,8 @@ static QemuOptsList runtime_opts = {
> * We support iscsi url's on the form
> * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> */
> -static int iscsi_open(BlockDriverState *bs, QDict *options, int flags)
> +static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> + Error **errp)
> {
> IscsiLun *iscsilun = bs->opaque;
> struct iscsi_context *iscsi = NULL;
block/iscsi.c: In function 'iscsi_create':
block/iscsi.c:1265:5: error: too few arguments to function 'iscsi_open'
block/iscsi.c:1049:12: note: declared here
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC v2 4/6] block: Error parameter for create functions
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 4/6] block: Error parameter for create functions Max Reitz
@ 2013-09-06 13:45 ` Kevin Wolf
0 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2013-09-06 13:45 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> Add an Error ** parameter to bdrv_create and its associated functions to
> allow more specific error messages.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 72 +++++++++++++++++++++++++++++++++------------------
> block/cow.c | 2 +-
> block/qcow.c | 2 +-
> block/qcow2.c | 2 +-
> block/qed.c | 2 +-
> block/raw_bsd.c | 2 +-
> block/vvfat.c | 2 +-
> include/block/block.h | 5 ++--
> qemu-img.c | 16 ++++--------
> 9 files changed, 61 insertions(+), 44 deletions(-)
> @@ -1053,11 +1078,9 @@ 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) {
> - error_setg_errno(errp, -ret, "Could not create temporary overlay "
> - "'%s'", tmp_filename);
> goto fail;
> }
I think this message adds value, because a message that makes sense for
a caller of bdrv_create(), doesn't necessarily make sense for a
bdrv_open() caller without further context (like that it's about the
temporary overlay).
I guess you should do something along the lines of:
error_setg_errno(errp, -ret, "Could not create temporary overlay "
"'%s': %s", tmp_filename, error_get_pretty(local_err));
error_free(local_err);
local_err = NULL;
> @@ -4553,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 */
> @@ -4639,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, NULL);
> + backing_drv, &local_err);
> if (ret < 0) {
> - error_setg_errno(errp, -ret, "Could not open '%s'",
> - backing_file->value.s);
Same thing about context of the error message here.
> goto out;
> }
> bdrv_get_geometry(bs, &size);
> @@ -4661,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;
> }
This should eventually be moved into the block drivers as well, but okay
with me for now.
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC v2 5/6] qcow2: Use Error parameter
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 5/6] qcow2: Use Error parameter Max Reitz
@ 2013-09-06 14:15 ` Kevin Wolf
0 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2013-09-06 14:15 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> 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
Uhm... No? :-)
> @@ -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);
Huh?
> #endif
> break;
>
> @@ -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;
> }
More context:
ret = qcow2_alloc_clusters(bs, 2 * cluster_size);
if (ret < 0) {
goto out;
Missing error_setg_errno()?
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC v2 6/6] bdrv: No silent error message discarding
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 6/6] bdrv: No silent error message discarding Max Reitz
@ 2013-09-06 14:22 ` Kevin Wolf
0 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2013-09-06 14:22 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> Using NULL as errp parameters to bdrv_open, bdrv_file_open and
> bdrv_create in all block drivers which do not yet implement proper error
> propagation is not a good idea, since this now silently discards error
> messages. Fix this by using a proper parameter and printing its content
> on error (until proper propagation is implemented).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> Note that many error propagation are actually trivial (such as in
> raw_bsd), but I intentionally refrained from implementing the propagation
> and tenaciously followed the pattern:
> Error *local_err = NULL;
> foo(&local_err);
> if (/*error condition*/) {
> qerror_report_err(local_err):
> error_free(local_err);
> }
> This is because the propagation may sometimes be trivial, however, it is
> often still driver-specific, therefore this deserves its own patch for
> every driver, in my opinion. Also, I think it is easier to review this
> way. ;-)
Agreed. But this patch should be squashed into the patches introducing
the NULL argument. Otherwise I'd need to check if this patch catches all
places that were previously introduced.
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 6722771..a9d8019 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1401,9 +1401,10 @@ static int sd_prealloc(const char *filename)
> uint32_t idx, max_idx;
> int64_t vdi_size;
> void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
> + Error *local_err = NULL;
> int 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) {
> goto out;
> }
You didn't feel like printing the message here? :-)
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-09-06 14:22 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-05 13:55 [Qemu-devel] [RFC v2 0/6] block: Error parameter for opening/creating images Max Reitz
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 1/6] bdrv: Use "Error" for opening images Max Reitz
2013-09-06 13:08 ` Kevin Wolf
2013-09-06 13:36 ` Kevin Wolf
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 2/6] bdrv: Use "Error" for creating images Max Reitz
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 3/6] block: Error parameter for open functions Max Reitz
2013-09-06 13:35 ` Kevin Wolf
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 4/6] block: Error parameter for create functions Max Reitz
2013-09-06 13:45 ` Kevin Wolf
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 5/6] qcow2: Use Error parameter Max Reitz
2013-09-06 14:15 ` Kevin Wolf
2013-09-05 13:55 ` [Qemu-devel] [RFC v2 6/6] bdrv: No silent error message discarding Max Reitz
2013-09-06 14:22 ` Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.