* [Qemu-devel] [PATCH 01/18] blockdev: Don't use qerror_report_err() in drive_init()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-13 17:04 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 02/18] blockdev: Don't use qerror_report() in do_drive_del() Markus Armbruster
` (16 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
qerror_report_err() is a transitional interface to help with
converting existing HMP commands to QMP. It should not be used
elsewhere.
drive_init() is not meant to be used by QMP commands. It uses both
qerror_report_err() and error_report(). Convert the former to the
latter.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 1a12e24..24b1c3b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -691,7 +691,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
&error_abort);
qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err);
if (local_err) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));
error_free(local_err);
goto fail;
}
@@ -903,7 +903,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
dinfo = blockdev_init(filename, bs_opts, &local_err);
if (dinfo == NULL) {
if (local_err) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));
error_free(local_err);
}
goto fail;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 02/18] blockdev: Don't use qerror_report() in do_drive_del()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
2014-05-13 16:02 ` [Qemu-devel] [PATCH 01/18] blockdev: Don't use qerror_report_err() in drive_init() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-13 19:38 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 03/18] qemu-nbd: Don't use qerror_report() Markus Armbruster
` (15 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
qerror_report() is a transitional interface to help with converting
existing HMP commands to QMP. It should not be used elsewhere.
do_drive_del() is an HMP command that won't be converted to QMP (we'll
create a new QMP command instead). It uses both qerror_report() and
error_report(). Convert the former to the latter.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 24b1c3b..7478d4d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -34,7 +34,6 @@
#include "hw/block/block.h"
#include "block/blockjob.h"
#include "monitor/monitor.h"
-#include "qapi/qmp/qerror.h"
#include "qemu/option.h"
#include "qemu/config-file.h"
#include "qapi/qmp/types.h"
@@ -1772,7 +1771,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
bs = bdrv_find(id);
if (!bs) {
- qerror_report(QERR_DEVICE_NOT_FOUND, id);
+ error_report("Device '%s' not found", id);
return -1;
}
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 03/18] qemu-nbd: Don't use qerror_report()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
2014-05-13 16:02 ` [Qemu-devel] [PATCH 01/18] blockdev: Don't use qerror_report_err() in drive_init() Markus Armbruster
2014-05-13 16:02 ` [Qemu-devel] [PATCH 02/18] blockdev: Don't use qerror_report() in do_drive_del() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-13 19:39 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 04/18] block/rbd: Propagate errors to open and create methods Markus Armbruster
` (14 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
qerror_report() is a transitional interface to help with converting
existing HMP commands to QMP. It should not be used elsewhere.
Replace by error_report().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-nbd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index eed79fa..621e654 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -223,7 +223,7 @@ static int tcp_socket_incoming(const char *address, uint16_t port)
int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0, &local_err);
if (local_err != NULL) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));
error_free(local_err);
}
return fd;
@@ -235,7 +235,7 @@ static int unix_socket_incoming(const char *path)
int fd = unix_listen(path, NULL, 0, &local_err);
if (local_err != NULL) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));
error_free(local_err);
}
return fd;
@@ -247,7 +247,7 @@ static int unix_socket_outgoing(const char *path)
int fd = unix_connect(path, &local_err);
if (local_err != NULL) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));
error_free(local_err);
}
return fd;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 04/18] block/rbd: Propagate errors to open and create methods
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (2 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 03/18] qemu-nbd: Don't use qerror_report() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-13 19:51 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls Markus Armbruster
` (13 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Josh Durgin, stefanha
Completes the conversion to Error started in commit 015a103^..d5124c0.
Cc: Josh Durgin <josh.durgin@inktank.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/rbd.c | 66 +++++++++++++++++++++++++++++++------------------------------
1 file changed, 34 insertions(+), 32 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index dbc79f4..aa7a864 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -105,7 +105,7 @@ typedef struct BDRVRBDState {
static int qemu_rbd_next_tok(char *dst, int dst_len,
char *src, char delim,
const char *name,
- char **p)
+ char **p, Error **errp)
{
int l;
char *end;
@@ -128,10 +128,10 @@ static int qemu_rbd_next_tok(char *dst, int dst_len,
}
l = strlen(src);
if (l >= dst_len) {
- error_report("%s too long", name);
+ error_setg(errp, "%s too long", name);
return -EINVAL;
} else if (l == 0) {
- error_report("%s too short", name);
+ error_setg(errp, "%s too short", name);
return -EINVAL;
}
@@ -157,13 +157,15 @@ static int qemu_rbd_parsename(const char *filename,
char *pool, int pool_len,
char *snap, int snap_len,
char *name, int name_len,
- char *conf, int conf_len)
+ char *conf, int conf_len,
+ Error **errp)
{
const char *start;
char *p, *buf;
int ret;
if (!strstart(filename, "rbd:", &start)) {
+ error_setg(errp, "File name must start with 'rbd:'");
return -EINVAL;
}
@@ -172,7 +174,7 @@ static int qemu_rbd_parsename(const char *filename,
*snap = '\0';
*conf = '\0';
- ret = qemu_rbd_next_tok(pool, pool_len, p, '/', "pool name", &p);
+ ret = qemu_rbd_next_tok(pool, pool_len, p, '/', "pool name", &p, errp);
if (ret < 0 || !p) {
ret = -EINVAL;
goto done;
@@ -180,21 +182,23 @@ static int qemu_rbd_parsename(const char *filename,
qemu_rbd_unescape(pool);
if (strchr(p, '@')) {
- ret = qemu_rbd_next_tok(name, name_len, p, '@', "object name", &p);
+ ret = qemu_rbd_next_tok(name, name_len, p, '@', "object name", &p,
+ errp);
if (ret < 0) {
goto done;
}
- ret = qemu_rbd_next_tok(snap, snap_len, p, ':', "snap name", &p);
+ ret = qemu_rbd_next_tok(snap, snap_len, p, ':', "snap name", &p, errp);
qemu_rbd_unescape(snap);
} else {
- ret = qemu_rbd_next_tok(name, name_len, p, ':', "object name", &p);
+ ret = qemu_rbd_next_tok(name, name_len, p, ':', "object name", &p,
+ errp);
}
qemu_rbd_unescape(name);
if (ret < 0 || !p) {
goto done;
}
- ret = qemu_rbd_next_tok(conf, conf_len, p, '\0', "configuration", &p);
+ ret = qemu_rbd_next_tok(conf, conf_len, p, '\0', "configuration", &p, errp);
done:
g_free(buf);
@@ -229,7 +233,7 @@ static char *qemu_rbd_parse_clientname(const char *conf, char *clientname)
return NULL;
}
-static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
+static int qemu_rbd_set_conf(rados_t cluster, const char *conf, Error **errp)
{
char *p, *buf;
char name[RBD_MAX_CONF_NAME_SIZE];
@@ -241,20 +245,20 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
while (p) {
ret = qemu_rbd_next_tok(name, sizeof(name), p,
- '=', "conf option name", &p);
+ '=', "conf option name", &p, errp);
if (ret < 0) {
break;
}
qemu_rbd_unescape(name);
if (!p) {
- error_report("conf option %s has no value", name);
+ error_setg(errp, "conf option %s has no value", name);
ret = -EINVAL;
break;
}
ret = qemu_rbd_next_tok(value, sizeof(value), p,
- ':', "conf option value", &p);
+ ':', "conf option value", &p, errp);
if (ret < 0) {
break;
}
@@ -263,7 +267,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
if (strcmp(name, "conf") == 0) {
ret = rados_conf_read_file(cluster, value);
if (ret < 0) {
- error_report("error reading conf file %s", value);
+ error_setg(errp, "error reading conf file %s", value);
break;
}
} else if (strcmp(name, "id") == 0) {
@@ -271,7 +275,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
} else {
ret = rados_conf_set(cluster, name, value);
if (ret < 0) {
- error_report("invalid conf option %s", name);
+ error_setg(errp, "invalid conf option %s", name);
ret = -EINVAL;
break;
}
@@ -285,6 +289,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
Error **errp)
{
+ Error *local_err = NULL;
int64_t bytes = 0;
int64_t objsize;
int obj_order = 0;
@@ -301,7 +306,7 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
if (qemu_rbd_parsename(filename, pool, sizeof(pool),
snap_buf, sizeof(snap_buf),
name, sizeof(name),
- conf, sizeof(conf)) < 0) {
+ conf, sizeof(conf), &local_err) < 0) {
return -EINVAL;
}
@@ -313,11 +318,11 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
if (options->value.n) {
objsize = options->value.n;
if ((objsize - 1) & objsize) { /* not a power of 2? */
- error_report("obj size needs to be power of 2");
+ error_setg(errp, "obj size needs to be power of 2");
return -EINVAL;
}
if (objsize < 4096) {
- error_report("obj size too small");
+ error_setg(errp, "obj size too small");
return -EINVAL;
}
obj_order = ffs(objsize) - 1;
@@ -328,7 +333,7 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
clientname = qemu_rbd_parse_clientname(conf, clientname_buf);
if (rados_create(&cluster, clientname) < 0) {
- error_report("error initializing");
+ error_setg(errp, "error initializing");
return -EIO;
}
@@ -338,20 +343,19 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
}
if (conf[0] != '\0' &&
- qemu_rbd_set_conf(cluster, conf) < 0) {
- error_report("error setting config options");
+ qemu_rbd_set_conf(cluster, conf, &local_err) < 0) {
rados_shutdown(cluster);
return -EIO;
}
if (rados_connect(cluster) < 0) {
- error_report("error connecting");
+ error_setg(errp, "error connecting");
rados_shutdown(cluster);
return -EIO;
}
if (rados_ioctx_create(cluster, pool, &io_ctx) < 0) {
- error_report("error opening pool %s", pool);
+ error_setg(errp, "error opening pool %s", pool);
rados_shutdown(cluster);
return -EIO;
}
@@ -441,8 +445,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (local_err) {
- qerror_report_err(local_err);
- error_free(local_err);
+ error_propagate(errp, local_err);
qemu_opts_del(opts);
return -EINVAL;
}
@@ -452,7 +455,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
if (qemu_rbd_parsename(filename, pool, sizeof(pool),
snap_buf, sizeof(snap_buf),
s->name, sizeof(s->name),
- conf, sizeof(conf)) < 0) {
+ conf, sizeof(conf), &local_err) < 0) {
r = -EINVAL;
goto failed_opts;
}
@@ -460,7 +463,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
clientname = qemu_rbd_parse_clientname(conf, clientname_buf);
r = rados_create(&s->cluster, clientname);
if (r < 0) {
- error_report("error initializing");
+ error_setg(&local_err, "error initializing");
goto failed_opts;
}
@@ -488,28 +491,27 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
}
if (conf[0] != '\0') {
- r = qemu_rbd_set_conf(s->cluster, conf);
+ r = qemu_rbd_set_conf(s->cluster, conf, &local_err);
if (r < 0) {
- error_report("error setting config options");
goto failed_shutdown;
}
}
r = rados_connect(s->cluster);
if (r < 0) {
- error_report("error connecting");
+ error_setg(&local_err, "error connecting");
goto failed_shutdown;
}
r = rados_ioctx_create(s->cluster, pool, &s->io_ctx);
if (r < 0) {
- error_report("error opening pool %s", pool);
+ error_setg(&local_err, "error opening pool %s", pool);
goto failed_shutdown;
}
r = rbd_open(s->io_ctx, s->name, &s->image, s->snap);
if (r < 0) {
- error_report("error reading header from %s", s->name);
+ error_setg(&local_err, "error reading header from %s", s->name);
goto failed_open;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 04/18] block/rbd: Propagate errors to open and create methods
2014-05-13 16:02 ` [Qemu-devel] [PATCH 04/18] block/rbd: Propagate errors to open and create methods Markus Armbruster
@ 2014-05-13 19:51 ` Eric Blake
2014-05-14 5:41 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Eric Blake @ 2014-05-13 19:51 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: kwolf, Josh Durgin, stefanha
[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]
On 05/13/2014 10:02 AM, Markus Armbruster wrote:
> Completes the conversion to Error started in commit 015a103^..d5124c0.
>
> Cc: Josh Durgin <josh.durgin@inktank.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> block/rbd.c | 66 +++++++++++++++++++++++++++++++------------------------------
> 1 file changed, 34 insertions(+), 32 deletions(-)
>
> @@ -285,6 +289,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
> static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
> Error **errp)
> {
> + Error *local_err = NULL;
> int64_t bytes = 0;
> int64_t objsize;
> int obj_order = 0;
> @@ -301,7 +306,7 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
> if (qemu_rbd_parsename(filename, pool, sizeof(pool),
> snap_buf, sizeof(snap_buf),
> name, sizeof(name),
> - conf, sizeof(conf)) < 0) {
> + conf, sizeof(conf), &local_err) < 0) {
> return -EINVAL;
Doesn't this leak local_err?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 04/18] block/rbd: Propagate errors to open and create methods
2014-05-13 19:51 ` Eric Blake
@ 2014-05-14 5:41 ` Markus Armbruster
0 siblings, 0 replies; 47+ messages in thread
From: Markus Armbruster @ 2014-05-14 5:41 UTC (permalink / raw)
To: Eric Blake; +Cc: kwolf, Josh Durgin, qemu-devel, stefanha
Eric Blake <eblake@redhat.com> writes:
> On 05/13/2014 10:02 AM, Markus Armbruster wrote:
>> Completes the conversion to Error started in commit 015a103^..d5124c0.
>>
>> Cc: Josh Durgin <josh.durgin@inktank.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> block/rbd.c | 66 +++++++++++++++++++++++++++++++------------------------------
>> 1 file changed, 34 insertions(+), 32 deletions(-)
>>
>
>> @@ -285,6 +289,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
>> static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
>> Error **errp)
>> {
>> + Error *local_err = NULL;
>> int64_t bytes = 0;
>> int64_t objsize;
>> int obj_order = 0;
>> @@ -301,7 +306,7 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
>> if (qemu_rbd_parsename(filename, pool, sizeof(pool),
>> snap_buf, sizeof(snap_buf),
>> name, sizeof(name),
>> - conf, sizeof(conf)) < 0) {
>> + conf, sizeof(conf), &local_err) < 0) {
>> return -EINVAL;
>
> Doesn't this leak local_err?
error_propagate() missing, will fix, thanks!
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (3 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 04/18] block/rbd: Propagate errors to open and create methods Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-14 9:11 ` Richard W.M. Jones
2014-05-14 14:57 ` Richard W.M. Jones
2014-05-13 16:02 ` [Qemu-devel] [PATCH 06/18] block/ssh: Propagate errors through check_host_key() Markus Armbruster
` (12 subsequent siblings)
17 siblings, 2 replies; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Richard W.M. Jones, stefanha
libssh2_session_last_error() already returns the error code.
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/ssh.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index aa63c9d..e38d232 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -121,10 +121,9 @@ session_error_report(BDRVSSHState *s, const char *fs, ...)
char *ssh_err;
int ssh_err_code;
- libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
/* This is not an errno. See <libssh2.h>. */
- ssh_err_code = libssh2_session_last_errno((s)->session);
-
+ ssh_err_code = libssh2_session_last_error(s->session,
+ &ssh_err, NULL, 0);
error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code);
}
@@ -145,9 +144,9 @@ sftp_error_report(BDRVSSHState *s, const char *fs, ...)
int ssh_err_code;
unsigned long sftp_err_code;
- libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
/* This is not an errno. See <libssh2.h>. */
- ssh_err_code = libssh2_session_last_errno((s)->session);
+ ssh_err_code = libssh2_session_last_error(s->session,
+ &ssh_err, NULL, 0);
/* See <libssh2_sftp.h>. */
sftp_err_code = libssh2_sftp_last_error((s)->sftp);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
2014-05-13 16:02 ` [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls Markus Armbruster
@ 2014-05-14 9:11 ` Richard W.M. Jones
2014-05-14 11:06 ` Markus Armbruster
2014-05-14 14:57 ` Richard W.M. Jones
1 sibling, 1 reply; 47+ messages in thread
From: Richard W.M. Jones @ 2014-05-14 9:11 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha
On Tue, May 13, 2014 at 06:02:39PM +0200, Markus Armbruster wrote:
> libssh2_session_last_error() already returns the error code.
>
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> block/ssh.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index aa63c9d..e38d232 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -121,10 +121,9 @@ session_error_report(BDRVSSHState *s, const char *fs, ...)
> char *ssh_err;
> int ssh_err_code;
>
> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
> /* This is not an errno. See <libssh2.h>. */
> - ssh_err_code = libssh2_session_last_errno((s)->session);
> -
> + ssh_err_code = libssh2_session_last_error(s->session,
> + &ssh_err, NULL, 0);
> error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code);
> }
>
> @@ -145,9 +144,9 @@ sftp_error_report(BDRVSSHState *s, const char *fs, ...)
> int ssh_err_code;
> unsigned long sftp_err_code;
>
> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
> /* This is not an errno. See <libssh2.h>. */
> - ssh_err_code = libssh2_session_last_errno((s)->session);
> + ssh_err_code = libssh2_session_last_error(s->session,
> + &ssh_err, NULL, 0);
> /* See <libssh2_sftp.h>. */
> sftp_err_code = libssh2_sftp_last_error((s)->sftp);
Yes, I'm not quite sure what was happening here. I checked the source
of libssh2 and as you say, libssh2_session_last_error returns the
error code, so there is no need to call libssh2_session_last_errno as
well.
Therefore, ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
2014-05-14 9:11 ` Richard W.M. Jones
@ 2014-05-14 11:06 ` Markus Armbruster
2014-05-14 12:01 ` Richard W.M. Jones
0 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-14 11:06 UTC (permalink / raw)
To: Richard W.M. Jones; +Cc: kwolf, qemu-devel, stefanha
"Richard W.M. Jones" <rjones@redhat.com> writes:
> On Tue, May 13, 2014 at 06:02:39PM +0200, Markus Armbruster wrote:
>> libssh2_session_last_error() already returns the error code.
>>
>> Cc: "Richard W.M. Jones" <rjones@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> block/ssh.c | 9 ++++-----
>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/ssh.c b/block/ssh.c
>> index aa63c9d..e38d232 100644
>> --- a/block/ssh.c
>> +++ b/block/ssh.c
>> @@ -121,10 +121,9 @@ session_error_report(BDRVSSHState *s, const char *fs, ...)
>> char *ssh_err;
>> int ssh_err_code;
>>
>> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
>> /* This is not an errno. See <libssh2.h>. */
>> - ssh_err_code = libssh2_session_last_errno((s)->session);
>> -
>> + ssh_err_code = libssh2_session_last_error(s->session,
>> + &ssh_err, NULL, 0);
>> error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code);
>> }
>>
>> @@ -145,9 +144,9 @@ sftp_error_report(BDRVSSHState *s, const char *fs, ...)
>> int ssh_err_code;
>> unsigned long sftp_err_code;
>>
>> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
>> /* This is not an errno. See <libssh2.h>. */
>> - ssh_err_code = libssh2_session_last_errno((s)->session);
>> + ssh_err_code = libssh2_session_last_error(s->session,
>> + &ssh_err, NULL, 0);
>> /* See <libssh2_sftp.h>. */
>> sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>
> Yes, I'm not quite sure what was happening here. I checked the source
> of libssh2 and as you say, libssh2_session_last_error returns the
> error code, so there is no need to call libssh2_session_last_errno as
> well.
>
> Therefore, ACK.
Did you review the patch? If yes, I'd like to convert your ACK to a
formal Reviewed-by. Same for PATCH 06-09.
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
2014-05-14 11:06 ` Markus Armbruster
@ 2014-05-14 12:01 ` Richard W.M. Jones
2014-05-14 14:48 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Richard W.M. Jones @ 2014-05-14 12:01 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha
On Wed, May 14, 2014 at 01:06:21PM +0200, Markus Armbruster wrote:
> "Richard W.M. Jones" <rjones@redhat.com> writes:
>
> > On Tue, May 13, 2014 at 06:02:39PM +0200, Markus Armbruster wrote:
> >> libssh2_session_last_error() already returns the error code.
> >>
> >> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >> block/ssh.c | 9 ++++-----
> >> 1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/block/ssh.c b/block/ssh.c
> >> index aa63c9d..e38d232 100644
> >> --- a/block/ssh.c
> >> +++ b/block/ssh.c
> >> @@ -121,10 +121,9 @@ session_error_report(BDRVSSHState *s, const char *fs, ...)
> >> char *ssh_err;
> >> int ssh_err_code;
> >>
> >> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
> >> /* This is not an errno. See <libssh2.h>. */
> >> - ssh_err_code = libssh2_session_last_errno((s)->session);
> >> -
> >> + ssh_err_code = libssh2_session_last_error(s->session,
> >> + &ssh_err, NULL, 0);
> >> error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code);
> >> }
> >>
> >> @@ -145,9 +144,9 @@ sftp_error_report(BDRVSSHState *s, const char *fs, ...)
> >> int ssh_err_code;
> >> unsigned long sftp_err_code;
> >>
> >> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
> >> /* This is not an errno. See <libssh2.h>. */
> >> - ssh_err_code = libssh2_session_last_errno((s)->session);
> >> + ssh_err_code = libssh2_session_last_error(s->session,
> >> + &ssh_err, NULL, 0);
> >> /* See <libssh2_sftp.h>. */
> >> sftp_err_code = libssh2_sftp_last_error((s)->sftp);
> >
> > Yes, I'm not quite sure what was happening here. I checked the source
> > of libssh2 and as you say, libssh2_session_last_error returns the
> > error code, so there is no need to call libssh2_session_last_errno as
> > well.
> >
> > Therefore, ACK.
>
> Did you review the patch? If yes, I'd like to convert your ACK to a
> formal Reviewed-by. Same for PATCH 06-09.
Yes, I applied all 5 of them to qemu.git, compiled it and also checked
over the patches.
So ACK to all of them.
Do I need to do something else?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
2014-05-14 12:01 ` Richard W.M. Jones
@ 2014-05-14 14:48 ` Markus Armbruster
0 siblings, 0 replies; 47+ messages in thread
From: Markus Armbruster @ 2014-05-14 14:48 UTC (permalink / raw)
To: Richard W.M. Jones; +Cc: kwolf, qemu-devel, stefanha
"Richard W.M. Jones" <rjones@redhat.com> writes:
> On Wed, May 14, 2014 at 01:06:21PM +0200, Markus Armbruster wrote:
>> "Richard W.M. Jones" <rjones@redhat.com> writes:
>>
>> > On Tue, May 13, 2014 at 06:02:39PM +0200, Markus Armbruster wrote:
>> >> libssh2_session_last_error() already returns the error code.
>> >>
>> >> Cc: "Richard W.M. Jones" <rjones@redhat.com>
>> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> ---
>> >> block/ssh.c | 9 ++++-----
>> >> 1 file changed, 4 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/block/ssh.c b/block/ssh.c
>> >> index aa63c9d..e38d232 100644
>> >> --- a/block/ssh.c
>> >> +++ b/block/ssh.c
>> >> @@ -121,10 +121,9 @@ session_error_report(BDRVSSHState *s, const char *fs, ...)
>> >> char *ssh_err;
>> >> int ssh_err_code;
>> >>
>> >> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
>> >> /* This is not an errno. See <libssh2.h>. */
>> >> - ssh_err_code = libssh2_session_last_errno((s)->session);
>> >> -
>> >> + ssh_err_code = libssh2_session_last_error(s->session,
>> >> + &ssh_err, NULL, 0);
>> >> error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code);
>> >> }
>> >>
>> >> @@ -145,9 +144,9 @@ sftp_error_report(BDRVSSHState *s, const char *fs, ...)
>> >> int ssh_err_code;
>> >> unsigned long sftp_err_code;
>> >>
>> >> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
>> >> /* This is not an errno. See <libssh2.h>. */
>> >> - ssh_err_code = libssh2_session_last_errno((s)->session);
>> >> + ssh_err_code = libssh2_session_last_error(s->session,
>> >> + &ssh_err, NULL, 0);
>> >> /* See <libssh2_sftp.h>. */
>> >> sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>> >
>> > Yes, I'm not quite sure what was happening here. I checked the source
>> > of libssh2 and as you say, libssh2_session_last_error returns the
>> > error code, so there is no need to call libssh2_session_last_errno as
>> > well.
>> >
>> > Therefore, ACK.
>>
>> Did you review the patch? If yes, I'd like to convert your ACK to a
>> formal Reviewed-by. Same for PATCH 06-09.
>
> Yes, I applied all 5 of them to qemu.git, compiled it and also checked
> over the patches.
>
> So ACK to all of them.
>
> Do I need to do something else?
We're pretty formal about commit tags on this list.
We interpret "Acked-by" conservatively. Something like "this patch
touches stuff I'm responsible for, I'm aware of the patch, and I don't
object to it".
To say "I reviewed this patch, and I think it's ready for commit", we
use "Reviewed-by".
"ACK" is neither, so I asked for clarification :)
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
2014-05-13 16:02 ` [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls Markus Armbruster
2014-05-14 9:11 ` Richard W.M. Jones
@ 2014-05-14 14:57 ` Richard W.M. Jones
1 sibling, 0 replies; 47+ messages in thread
From: Richard W.M. Jones @ 2014-05-14 14:57 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha
On Tue, May 13, 2014 at 06:02:39PM +0200, Markus Armbruster wrote:
> libssh2_session_last_error() already returns the error code.
>
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
> block/ssh.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index aa63c9d..e38d232 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -121,10 +121,9 @@ session_error_report(BDRVSSHState *s, const char *fs, ...)
> char *ssh_err;
> int ssh_err_code;
>
> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
> /* This is not an errno. See <libssh2.h>. */
> - ssh_err_code = libssh2_session_last_errno((s)->session);
> -
> + ssh_err_code = libssh2_session_last_error(s->session,
> + &ssh_err, NULL, 0);
> error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code);
> }
>
> @@ -145,9 +144,9 @@ sftp_error_report(BDRVSSHState *s, const char *fs, ...)
> int ssh_err_code;
> unsigned long sftp_err_code;
>
> - libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
> /* This is not an errno. See <libssh2.h>. */
> - ssh_err_code = libssh2_session_last_errno((s)->session);
> + ssh_err_code = libssh2_session_last_error(s->session,
> + &ssh_err, NULL, 0);
> /* See <libssh2_sftp.h>. */
> sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>
> --
> 1.8.1.4
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 06/18] block/ssh: Propagate errors through check_host_key()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (4 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-14 14:57 ` Richard W.M. Jones
2014-05-13 16:02 ` [Qemu-devel] [PATCH 07/18] block/ssh: Propagate errors through authenticate() Markus Armbruster
` (11 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Richard W.M. Jones, stefanha
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/ssh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 49 insertions(+), 19 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index e38d232..1df1946 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -106,6 +106,31 @@ static void ssh_state_free(BDRVSSHState *s)
}
}
+static void GCC_FMT_ATTR(3, 4)
+session_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...)
+{
+ va_list args;
+ char *msg;
+
+ va_start(args, fs);
+ msg = g_strdup_vprintf(fs, args);
+ va_end(args);
+
+ if (s->session) {
+ char *ssh_err;
+ int ssh_err_code;
+
+ /* This is not an errno. See <libssh2.h>. */
+ ssh_err_code = libssh2_session_last_error(s->session,
+ &ssh_err, NULL, 0);
+ error_setg(errp, "%s: %s (libssh2 error code: %d)",
+ msg, ssh_err, ssh_err_code);
+ } else {
+ error_setg(errp, "%s", msg);
+ }
+ g_free(msg);
+}
+
/* Wrappers around error_report which make sure to dump as much
* information from libssh2 as possible.
*/
@@ -242,7 +267,7 @@ static void ssh_parse_filename(const char *filename, QDict *options,
}
static int check_host_key_knownhosts(BDRVSSHState *s,
- const char *host, int port)
+ const char *host, int port, Error **errp)
{
const char *home;
char *knh_file = NULL;
@@ -256,14 +281,15 @@ static int check_host_key_knownhosts(BDRVSSHState *s,
hostkey = libssh2_session_hostkey(s->session, &len, &type);
if (!hostkey) {
ret = -EINVAL;
- session_error_report(s, "failed to read remote host key");
+ session_error_setg(errp, s, "failed to read remote host key");
goto out;
}
knh = libssh2_knownhost_init(s->session);
if (!knh) {
ret = -EINVAL;
- session_error_report(s, "failed to initialize known hosts support");
+ session_error_setg(errp, s,
+ "failed to initialize known hosts support");
goto out;
}
@@ -288,21 +314,23 @@ static int check_host_key_knownhosts(BDRVSSHState *s,
break;
case LIBSSH2_KNOWNHOST_CHECK_MISMATCH:
ret = -EINVAL;
- session_error_report(s, "host key does not match the one in known_hosts (found key %s)",
- found->key);
+ session_error_setg(errp, s,
+ "host key does not match the one in known_hosts"
+ " (found key %s)", found->key);
goto out;
case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND:
ret = -EINVAL;
- session_error_report(s, "no host key was found in known_hosts");
+ session_error_setg(errp, s, "no host key was found in known_hosts");
goto out;
case LIBSSH2_KNOWNHOST_CHECK_FAILURE:
ret = -EINVAL;
- session_error_report(s, "failure matching the host key with known_hosts");
+ session_error_setg(errp, s,
+ "failure matching the host key with known_hosts");
goto out;
default:
ret = -EINVAL;
- session_error_report(s, "unknown error matching the host key with known_hosts (%d)",
- r);
+ session_error_setg(errp, s, "unknown error matching the host key"
+ " with known_hosts (%d)", r);
goto out;
}
@@ -357,20 +385,20 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len,
static int
check_host_key_hash(BDRVSSHState *s, const char *hash,
- int hash_type, size_t fingerprint_len)
+ int hash_type, size_t fingerprint_len, Error **errp)
{
const char *fingerprint;
fingerprint = libssh2_hostkey_hash(s->session, hash_type);
if (!fingerprint) {
- session_error_report(s, "failed to read remote host key");
+ session_error_setg(errp, s, "failed to read remote host key");
return -EINVAL;
}
if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_len,
hash) != 0) {
- error_report("remote host key does not match host_key_check '%s'",
- hash);
+ error_setg(errp, "remote host key does not match host_key_check '%s'",
+ hash);
return -EPERM;
}
@@ -378,7 +406,7 @@ check_host_key_hash(BDRVSSHState *s, const char *hash,
}
static int check_host_key(BDRVSSHState *s, const char *host, int port,
- const char *host_key_check)
+ const char *host_key_check, Error **errp)
{
/* host_key_check=no */
if (strcmp(host_key_check, "no") == 0) {
@@ -388,21 +416,21 @@ static int check_host_key(BDRVSSHState *s, const char *host, int port,
/* host_key_check=md5:xx:yy:zz:... */
if (strncmp(host_key_check, "md5:", 4) == 0) {
return check_host_key_hash(s, &host_key_check[4],
- LIBSSH2_HOSTKEY_HASH_MD5, 16);
+ LIBSSH2_HOSTKEY_HASH_MD5, 16, errp);
}
/* host_key_check=sha1:xx:yy:zz:... */
if (strncmp(host_key_check, "sha1:", 5) == 0) {
return check_host_key_hash(s, &host_key_check[5],
- LIBSSH2_HOSTKEY_HASH_SHA1, 20);
+ LIBSSH2_HOSTKEY_HASH_SHA1, 20, errp);
}
/* host_key_check=yes */
if (strcmp(host_key_check, "yes") == 0) {
- return check_host_key_knownhosts(s, host, port);
+ return check_host_key_knownhosts(s, host, port, errp);
}
- error_report("unknown host_key_check setting (%s)", host_key_check);
+ error_setg(errp, "unknown host_key_check setting (%s)", host_key_check);
return -EINVAL;
}
@@ -541,8 +569,10 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
}
/* Check the remote host's key against known_hosts. */
- ret = check_host_key(s, host, port, host_key_check);
+ ret = check_host_key(s, host, port, host_key_check, &err);
if (ret < 0) {
+ qerror_report_err(err);
+ error_free(err);
goto err;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 06/18] block/ssh: Propagate errors through check_host_key()
2014-05-13 16:02 ` [Qemu-devel] [PATCH 06/18] block/ssh: Propagate errors through check_host_key() Markus Armbruster
@ 2014-05-14 14:57 ` Richard W.M. Jones
0 siblings, 0 replies; 47+ messages in thread
From: Richard W.M. Jones @ 2014-05-14 14:57 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha
On Tue, May 13, 2014 at 06:02:40PM +0200, Markus Armbruster wrote:
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
> block/ssh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 49 insertions(+), 19 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index e38d232..1df1946 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -106,6 +106,31 @@ static void ssh_state_free(BDRVSSHState *s)
> }
> }
>
> +static void GCC_FMT_ATTR(3, 4)
> +session_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...)
> +{
> + va_list args;
> + char *msg;
> +
> + va_start(args, fs);
> + msg = g_strdup_vprintf(fs, args);
> + va_end(args);
> +
> + if (s->session) {
> + char *ssh_err;
> + int ssh_err_code;
> +
> + /* This is not an errno. See <libssh2.h>. */
> + ssh_err_code = libssh2_session_last_error(s->session,
> + &ssh_err, NULL, 0);
> + error_setg(errp, "%s: %s (libssh2 error code: %d)",
> + msg, ssh_err, ssh_err_code);
> + } else {
> + error_setg(errp, "%s", msg);
> + }
> + g_free(msg);
> +}
> +
> /* Wrappers around error_report which make sure to dump as much
> * information from libssh2 as possible.
> */
> @@ -242,7 +267,7 @@ static void ssh_parse_filename(const char *filename, QDict *options,
> }
>
> static int check_host_key_knownhosts(BDRVSSHState *s,
> - const char *host, int port)
> + const char *host, int port, Error **errp)
> {
> const char *home;
> char *knh_file = NULL;
> @@ -256,14 +281,15 @@ static int check_host_key_knownhosts(BDRVSSHState *s,
> hostkey = libssh2_session_hostkey(s->session, &len, &type);
> if (!hostkey) {
> ret = -EINVAL;
> - session_error_report(s, "failed to read remote host key");
> + session_error_setg(errp, s, "failed to read remote host key");
> goto out;
> }
>
> knh = libssh2_knownhost_init(s->session);
> if (!knh) {
> ret = -EINVAL;
> - session_error_report(s, "failed to initialize known hosts support");
> + session_error_setg(errp, s,
> + "failed to initialize known hosts support");
> goto out;
> }
>
> @@ -288,21 +314,23 @@ static int check_host_key_knownhosts(BDRVSSHState *s,
> break;
> case LIBSSH2_KNOWNHOST_CHECK_MISMATCH:
> ret = -EINVAL;
> - session_error_report(s, "host key does not match the one in known_hosts (found key %s)",
> - found->key);
> + session_error_setg(errp, s,
> + "host key does not match the one in known_hosts"
> + " (found key %s)", found->key);
> goto out;
> case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND:
> ret = -EINVAL;
> - session_error_report(s, "no host key was found in known_hosts");
> + session_error_setg(errp, s, "no host key was found in known_hosts");
> goto out;
> case LIBSSH2_KNOWNHOST_CHECK_FAILURE:
> ret = -EINVAL;
> - session_error_report(s, "failure matching the host key with known_hosts");
> + session_error_setg(errp, s,
> + "failure matching the host key with known_hosts");
> goto out;
> default:
> ret = -EINVAL;
> - session_error_report(s, "unknown error matching the host key with known_hosts (%d)",
> - r);
> + session_error_setg(errp, s, "unknown error matching the host key"
> + " with known_hosts (%d)", r);
> goto out;
> }
>
> @@ -357,20 +385,20 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len,
>
> static int
> check_host_key_hash(BDRVSSHState *s, const char *hash,
> - int hash_type, size_t fingerprint_len)
> + int hash_type, size_t fingerprint_len, Error **errp)
> {
> const char *fingerprint;
>
> fingerprint = libssh2_hostkey_hash(s->session, hash_type);
> if (!fingerprint) {
> - session_error_report(s, "failed to read remote host key");
> + session_error_setg(errp, s, "failed to read remote host key");
> return -EINVAL;
> }
>
> if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_len,
> hash) != 0) {
> - error_report("remote host key does not match host_key_check '%s'",
> - hash);
> + error_setg(errp, "remote host key does not match host_key_check '%s'",
> + hash);
> return -EPERM;
> }
>
> @@ -378,7 +406,7 @@ check_host_key_hash(BDRVSSHState *s, const char *hash,
> }
>
> static int check_host_key(BDRVSSHState *s, const char *host, int port,
> - const char *host_key_check)
> + const char *host_key_check, Error **errp)
> {
> /* host_key_check=no */
> if (strcmp(host_key_check, "no") == 0) {
> @@ -388,21 +416,21 @@ static int check_host_key(BDRVSSHState *s, const char *host, int port,
> /* host_key_check=md5:xx:yy:zz:... */
> if (strncmp(host_key_check, "md5:", 4) == 0) {
> return check_host_key_hash(s, &host_key_check[4],
> - LIBSSH2_HOSTKEY_HASH_MD5, 16);
> + LIBSSH2_HOSTKEY_HASH_MD5, 16, errp);
> }
>
> /* host_key_check=sha1:xx:yy:zz:... */
> if (strncmp(host_key_check, "sha1:", 5) == 0) {
> return check_host_key_hash(s, &host_key_check[5],
> - LIBSSH2_HOSTKEY_HASH_SHA1, 20);
> + LIBSSH2_HOSTKEY_HASH_SHA1, 20, errp);
> }
>
> /* host_key_check=yes */
> if (strcmp(host_key_check, "yes") == 0) {
> - return check_host_key_knownhosts(s, host, port);
> + return check_host_key_knownhosts(s, host, port, errp);
> }
>
> - error_report("unknown host_key_check setting (%s)", host_key_check);
> + error_setg(errp, "unknown host_key_check setting (%s)", host_key_check);
> return -EINVAL;
> }
>
> @@ -541,8 +569,10 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> }
>
> /* Check the remote host's key against known_hosts. */
> - ret = check_host_key(s, host, port, host_key_check);
> + ret = check_host_key(s, host, port, host_key_check, &err);
> if (ret < 0) {
> + qerror_report_err(err);
> + error_free(err);
> goto err;
> }
>
> --
> 1.8.1.4
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 07/18] block/ssh: Propagate errors through authenticate()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (5 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 06/18] block/ssh: Propagate errors through check_host_key() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-14 14:57 ` Richard W.M. Jones
2014-05-13 16:02 ` [Qemu-devel] [PATCH 08/18] block/ssh: Propagate errors through connect_to_ssh() Markus Armbruster
` (10 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Richard W.M. Jones, stefanha
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/ssh.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index 1df1946..18186ba 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -434,7 +434,7 @@ static int check_host_key(BDRVSSHState *s, const char *host, int port,
return -EINVAL;
}
-static int authenticate(BDRVSSHState *s, const char *user)
+static int authenticate(BDRVSSHState *s, const char *user, Error **errp)
{
int r, ret;
const char *userauthlist;
@@ -445,7 +445,8 @@ static int authenticate(BDRVSSHState *s, const char *user)
userauthlist = libssh2_userauth_list(s->session, user, strlen(user));
if (strstr(userauthlist, "publickey") == NULL) {
ret = -EPERM;
- error_report("remote server does not support \"publickey\" authentication");
+ error_setg(errp,
+ "remote server does not support \"publickey\" authentication");
goto out;
}
@@ -453,17 +454,18 @@ static int authenticate(BDRVSSHState *s, const char *user)
agent = libssh2_agent_init(s->session);
if (!agent) {
ret = -EINVAL;
- session_error_report(s, "failed to initialize ssh-agent support");
+ session_error_setg(errp, s, "failed to initialize ssh-agent support");
goto out;
}
if (libssh2_agent_connect(agent)) {
ret = -ECONNREFUSED;
- session_error_report(s, "failed to connect to ssh-agent");
+ session_error_setg(errp, s, "failed to connect to ssh-agent");
goto out;
}
if (libssh2_agent_list_identities(agent)) {
ret = -EINVAL;
- session_error_report(s, "failed requesting identities from ssh-agent");
+ session_error_setg(errp, s,
+ "failed requesting identities from ssh-agent");
goto out;
}
@@ -474,7 +476,8 @@ static int authenticate(BDRVSSHState *s, const char *user)
}
if (r < 0) {
ret = -EINVAL;
- session_error_report(s, "failed to obtain identity from ssh-agent");
+ session_error_setg(errp, s,
+ "failed to obtain identity from ssh-agent");
goto out;
}
r = libssh2_agent_userauth(agent, user, identity);
@@ -488,8 +491,8 @@ static int authenticate(BDRVSSHState *s, const char *user)
}
ret = -EPERM;
- error_report("failed to authenticate using publickey authentication "
- "and the identities held by your ssh-agent");
+ error_setg(errp, "failed to authenticate using publickey authentication "
+ "and the identities held by your ssh-agent");
out:
if (agent != NULL) {
@@ -577,8 +580,10 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
}
/* Authenticate. */
- ret = authenticate(s, user);
+ ret = authenticate(s, user, &err);
if (ret < 0) {
+ qerror_report_err(err);
+ error_free(err);
goto err;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 07/18] block/ssh: Propagate errors through authenticate()
2014-05-13 16:02 ` [Qemu-devel] [PATCH 07/18] block/ssh: Propagate errors through authenticate() Markus Armbruster
@ 2014-05-14 14:57 ` Richard W.M. Jones
0 siblings, 0 replies; 47+ messages in thread
From: Richard W.M. Jones @ 2014-05-14 14:57 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha
On Tue, May 13, 2014 at 06:02:41PM +0200, Markus Armbruster wrote:
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
> block/ssh.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index 1df1946..18186ba 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -434,7 +434,7 @@ static int check_host_key(BDRVSSHState *s, const char *host, int port,
> return -EINVAL;
> }
>
> -static int authenticate(BDRVSSHState *s, const char *user)
> +static int authenticate(BDRVSSHState *s, const char *user, Error **errp)
> {
> int r, ret;
> const char *userauthlist;
> @@ -445,7 +445,8 @@ static int authenticate(BDRVSSHState *s, const char *user)
> userauthlist = libssh2_userauth_list(s->session, user, strlen(user));
> if (strstr(userauthlist, "publickey") == NULL) {
> ret = -EPERM;
> - error_report("remote server does not support \"publickey\" authentication");
> + error_setg(errp,
> + "remote server does not support \"publickey\" authentication");
> goto out;
> }
>
> @@ -453,17 +454,18 @@ static int authenticate(BDRVSSHState *s, const char *user)
> agent = libssh2_agent_init(s->session);
> if (!agent) {
> ret = -EINVAL;
> - session_error_report(s, "failed to initialize ssh-agent support");
> + session_error_setg(errp, s, "failed to initialize ssh-agent support");
> goto out;
> }
> if (libssh2_agent_connect(agent)) {
> ret = -ECONNREFUSED;
> - session_error_report(s, "failed to connect to ssh-agent");
> + session_error_setg(errp, s, "failed to connect to ssh-agent");
> goto out;
> }
> if (libssh2_agent_list_identities(agent)) {
> ret = -EINVAL;
> - session_error_report(s, "failed requesting identities from ssh-agent");
> + session_error_setg(errp, s,
> + "failed requesting identities from ssh-agent");
> goto out;
> }
>
> @@ -474,7 +476,8 @@ static int authenticate(BDRVSSHState *s, const char *user)
> }
> if (r < 0) {
> ret = -EINVAL;
> - session_error_report(s, "failed to obtain identity from ssh-agent");
> + session_error_setg(errp, s,
> + "failed to obtain identity from ssh-agent");
> goto out;
> }
> r = libssh2_agent_userauth(agent, user, identity);
> @@ -488,8 +491,8 @@ static int authenticate(BDRVSSHState *s, const char *user)
> }
>
> ret = -EPERM;
> - error_report("failed to authenticate using publickey authentication "
> - "and the identities held by your ssh-agent");
> + error_setg(errp, "failed to authenticate using publickey authentication "
> + "and the identities held by your ssh-agent");
>
> out:
> if (agent != NULL) {
> @@ -577,8 +580,10 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> }
>
> /* Authenticate. */
> - ret = authenticate(s, user);
> + ret = authenticate(s, user, &err);
> if (ret < 0) {
> + qerror_report_err(err);
> + error_free(err);
> goto err;
> }
>
> --
> 1.8.1.4
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 08/18] block/ssh: Propagate errors through connect_to_ssh()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (6 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 07/18] block/ssh: Propagate errors through authenticate() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-14 14:57 ` Richard W.M. Jones
2014-05-13 16:02 ` [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods Markus Armbruster
` (9 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Richard W.M. Jones, stefanha
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/ssh.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index 18186ba..26078c4 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -506,10 +506,9 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp)
}
static int connect_to_ssh(BDRVSSHState *s, QDict *options,
- int ssh_flags, int creat_mode)
+ int ssh_flags, int creat_mode, Error **errp)
{
int r, ret;
- Error *err = NULL;
const char *host, *user, *path, *host_key_check;
int port;
@@ -528,6 +527,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
} else {
user = g_get_user_name();
if (!user) {
+ error_setg_errno(errp, errno, "Can't get user name");
ret = -errno;
goto err;
}
@@ -544,11 +544,9 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
s->hostport = g_strdup_printf("%s:%d", host, port);
/* Open the socket and connect. */
- s->sock = inet_connect(s->hostport, &err);
- if (err != NULL) {
+ s->sock = inet_connect(s->hostport, errp);
+ if (s->sock < 0) {
ret = -errno;
- qerror_report_err(err);
- error_free(err);
goto err;
}
@@ -556,7 +554,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
s->session = libssh2_session_init();
if (!s->session) {
ret = -EINVAL;
- session_error_report(s, "failed to initialize libssh2 session");
+ session_error_setg(errp, s, "failed to initialize libssh2 session");
goto err;
}
@@ -567,30 +565,26 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
r = libssh2_session_handshake(s->session, s->sock);
if (r != 0) {
ret = -EINVAL;
- session_error_report(s, "failed to establish SSH session");
+ session_error_setg(errp, s, "failed to establish SSH session");
goto err;
}
/* Check the remote host's key against known_hosts. */
- ret = check_host_key(s, host, port, host_key_check, &err);
+ ret = check_host_key(s, host, port, host_key_check, errp);
if (ret < 0) {
- qerror_report_err(err);
- error_free(err);
goto err;
}
/* Authenticate. */
- ret = authenticate(s, user, &err);
+ ret = authenticate(s, user, errp);
if (ret < 0) {
- qerror_report_err(err);
- error_free(err);
goto err;
}
/* Start SFTP. */
s->sftp = libssh2_sftp_init(s->session);
if (!s->sftp) {
- session_error_report(s, "failed to initialize sftp handle");
+ session_error_setg(errp, s, "failed to initialize sftp handle");
ret = -EINVAL;
goto err;
}
@@ -645,6 +639,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
Error **errp)
{
+ Error *local_err = NULL;
BDRVSSHState *s = bs->opaque;
int ret;
int ssh_flags;
@@ -657,8 +652,10 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
}
/* Start up SSH. */
- ret = connect_to_ssh(s, options, ssh_flags, 0);
+ ret = connect_to_ssh(s, options, ssh_flags, 0, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto err;
}
@@ -718,8 +715,11 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
r = connect_to_ssh(&s, uri_options,
LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE|
- LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC, 0644);
+ LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC,
+ 0644, &local_err);
if (r < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
ret = r;
goto out;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 08/18] block/ssh: Propagate errors through connect_to_ssh()
2014-05-13 16:02 ` [Qemu-devel] [PATCH 08/18] block/ssh: Propagate errors through connect_to_ssh() Markus Armbruster
@ 2014-05-14 14:57 ` Richard W.M. Jones
0 siblings, 0 replies; 47+ messages in thread
From: Richard W.M. Jones @ 2014-05-14 14:57 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha
On Tue, May 13, 2014 at 06:02:42PM +0200, Markus Armbruster wrote:
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
> block/ssh.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index 18186ba..26078c4 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -506,10 +506,9 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp)
> }
>
> static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> - int ssh_flags, int creat_mode)
> + int ssh_flags, int creat_mode, Error **errp)
> {
> int r, ret;
> - Error *err = NULL;
> const char *host, *user, *path, *host_key_check;
> int port;
>
> @@ -528,6 +527,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> } else {
> user = g_get_user_name();
> if (!user) {
> + error_setg_errno(errp, errno, "Can't get user name");
> ret = -errno;
> goto err;
> }
> @@ -544,11 +544,9 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> s->hostport = g_strdup_printf("%s:%d", host, port);
>
> /* Open the socket and connect. */
> - s->sock = inet_connect(s->hostport, &err);
> - if (err != NULL) {
> + s->sock = inet_connect(s->hostport, errp);
> + if (s->sock < 0) {
> ret = -errno;
> - qerror_report_err(err);
> - error_free(err);
> goto err;
> }
>
> @@ -556,7 +554,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> s->session = libssh2_session_init();
> if (!s->session) {
> ret = -EINVAL;
> - session_error_report(s, "failed to initialize libssh2 session");
> + session_error_setg(errp, s, "failed to initialize libssh2 session");
> goto err;
> }
>
> @@ -567,30 +565,26 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> r = libssh2_session_handshake(s->session, s->sock);
> if (r != 0) {
> ret = -EINVAL;
> - session_error_report(s, "failed to establish SSH session");
> + session_error_setg(errp, s, "failed to establish SSH session");
> goto err;
> }
>
> /* Check the remote host's key against known_hosts. */
> - ret = check_host_key(s, host, port, host_key_check, &err);
> + ret = check_host_key(s, host, port, host_key_check, errp);
> if (ret < 0) {
> - qerror_report_err(err);
> - error_free(err);
> goto err;
> }
>
> /* Authenticate. */
> - ret = authenticate(s, user, &err);
> + ret = authenticate(s, user, errp);
> if (ret < 0) {
> - qerror_report_err(err);
> - error_free(err);
> goto err;
> }
>
> /* Start SFTP. */
> s->sftp = libssh2_sftp_init(s->session);
> if (!s->sftp) {
> - session_error_report(s, "failed to initialize sftp handle");
> + session_error_setg(errp, s, "failed to initialize sftp handle");
> ret = -EINVAL;
> goto err;
> }
> @@ -645,6 +639,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
> Error **errp)
> {
> + Error *local_err = NULL;
> BDRVSSHState *s = bs->opaque;
> int ret;
> int ssh_flags;
> @@ -657,8 +652,10 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
> }
>
> /* Start up SSH. */
> - ret = connect_to_ssh(s, options, ssh_flags, 0);
> + ret = connect_to_ssh(s, options, ssh_flags, 0, &local_err);
> if (ret < 0) {
> + qerror_report_err(local_err);
> + error_free(local_err);
> goto err;
> }
>
> @@ -718,8 +715,11 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
>
> r = connect_to_ssh(&s, uri_options,
> LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE|
> - LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC, 0644);
> + LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC,
> + 0644, &local_err);
> if (r < 0) {
> + qerror_report_err(local_err);
> + error_free(local_err);
> ret = r;
> goto out;
> }
> --
> 1.8.1.4
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (7 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 08/18] block/ssh: Propagate errors through connect_to_ssh() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-14 9:13 ` Richard W.M. Jones
2014-05-14 14:58 ` Richard W.M. Jones
2014-05-13 16:02 ` [Qemu-devel] [PATCH 10/18] block/vvfat: Propagate errors through enable_write_target() Markus Armbruster
` (8 subsequent siblings)
17 siblings, 2 replies; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Richard W.M. Jones, stefanha
Completes the conversion to Error started in commit 015a103^..d5124c0.
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/ssh.c | 47 ++++++++++++++++++++++-------------------------
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index 26078c4..b212971 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -131,29 +131,34 @@ session_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...)
g_free(msg);
}
-/* Wrappers around error_report which make sure to dump as much
- * information from libssh2 as possible.
- */
-static void GCC_FMT_ATTR(2, 3)
-session_error_report(BDRVSSHState *s, const char *fs, ...)
+static void GCC_FMT_ATTR(3, 4)
+sftp_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...)
{
va_list args;
+ char *msg;
va_start(args, fs);
- error_vprintf(fs, args);
+ msg = g_strdup_vprintf(fs, args);
+ va_end(args);
- if ((s)->session) {
+ if (s->sftp) {
char *ssh_err;
int ssh_err_code;
+ unsigned long sftp_err_code;
/* This is not an errno. See <libssh2.h>. */
ssh_err_code = libssh2_session_last_error(s->session,
&ssh_err, NULL, 0);
- error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code);
- }
+ /* See <libssh2_sftp.h>. */
+ sftp_err_code = libssh2_sftp_last_error((s)->sftp);
- va_end(args);
- error_printf("\n");
+ error_setg(errp,
+ "%s: %s (libssh2 error code: %d, sftp error code: %lu)",
+ msg, ssh_err, ssh_err_code, sftp_err_code);
+ } else {
+ error_setg(errp, "%s", msg);
+ }
+ g_free(msg);
}
static void GCC_FMT_ATTR(2, 3)
@@ -594,14 +599,14 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
path, ssh_flags, creat_mode);
s->sftp_handle = libssh2_sftp_open(s->sftp, path, ssh_flags, creat_mode);
if (!s->sftp_handle) {
- session_error_report(s, "failed to open remote file '%s'", path);
+ session_error_setg(errp, s, "failed to open remote file '%s'", path);
ret = -EINVAL;
goto err;
}
r = libssh2_sftp_fstat(s->sftp_handle, &s->attrs);
if (r < 0) {
- sftp_error_report(s, "failed to read file attributes");
+ sftp_error_setg(errp, s, "failed to read file attributes");
return -EINVAL;
}
@@ -639,7 +644,6 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
Error **errp)
{
- Error *local_err = NULL;
BDRVSSHState *s = bs->opaque;
int ret;
int ssh_flags;
@@ -652,10 +656,8 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
}
/* Start up SSH. */
- ret = connect_to_ssh(s, options, ssh_flags, 0, &local_err);
+ ret = connect_to_ssh(s, options, ssh_flags, 0, errp);
if (ret < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
goto err;
}
@@ -686,7 +688,6 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
Error **errp)
{
int r, ret;
- Error *local_err = NULL;
int64_t total_size = 0;
QDict *uri_options = NULL;
BDRVSSHState s;
@@ -705,10 +706,8 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
DPRINTF("total_size=%" PRIi64, total_size);
uri_options = qdict_new();
- r = parse_uri(filename, uri_options, &local_err);
+ r = parse_uri(filename, uri_options, errp);
if (r < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
ret = r;
goto out;
}
@@ -716,10 +715,8 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
r = connect_to_ssh(&s, uri_options,
LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE|
LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC,
- 0644, &local_err);
+ 0644, errp);
if (r < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
ret = r;
goto out;
}
@@ -728,7 +725,7 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
libssh2_sftp_seek64(s.sftp_handle, total_size-1);
r2 = libssh2_sftp_write(s.sftp_handle, c, 1);
if (r2 < 0) {
- sftp_error_report(&s, "truncate failed");
+ sftp_error_setg(errp, &s, "truncate failed");
ret = -EINVAL;
goto out;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods
2014-05-13 16:02 ` [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods Markus Armbruster
@ 2014-05-14 9:13 ` Richard W.M. Jones
2014-05-14 14:58 ` Richard W.M. Jones
1 sibling, 0 replies; 47+ messages in thread
From: Richard W.M. Jones @ 2014-05-14 9:13 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha
The last four patches are just a refactoring to use qemu error
handling, therefore ACK to all four of them.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods
2014-05-13 16:02 ` [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods Markus Armbruster
2014-05-14 9:13 ` Richard W.M. Jones
@ 2014-05-14 14:58 ` Richard W.M. Jones
1 sibling, 0 replies; 47+ messages in thread
From: Richard W.M. Jones @ 2014-05-14 14:58 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha
On Tue, May 13, 2014 at 06:02:43PM +0200, Markus Armbruster wrote:
> Completes the conversion to Error started in commit 015a103^..d5124c0.
>
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
> block/ssh.c | 47 ++++++++++++++++++++++-------------------------
> 1 file changed, 22 insertions(+), 25 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index 26078c4..b212971 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -131,29 +131,34 @@ session_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...)
> g_free(msg);
> }
>
> -/* Wrappers around error_report which make sure to dump as much
> - * information from libssh2 as possible.
> - */
> -static void GCC_FMT_ATTR(2, 3)
> -session_error_report(BDRVSSHState *s, const char *fs, ...)
> +static void GCC_FMT_ATTR(3, 4)
> +sftp_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...)
> {
> va_list args;
> + char *msg;
>
> va_start(args, fs);
> - error_vprintf(fs, args);
> + msg = g_strdup_vprintf(fs, args);
> + va_end(args);
>
> - if ((s)->session) {
> + if (s->sftp) {
> char *ssh_err;
> int ssh_err_code;
> + unsigned long sftp_err_code;
>
> /* This is not an errno. See <libssh2.h>. */
> ssh_err_code = libssh2_session_last_error(s->session,
> &ssh_err, NULL, 0);
> - error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code);
> - }
> + /* See <libssh2_sftp.h>. */
> + sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>
> - va_end(args);
> - error_printf("\n");
> + error_setg(errp,
> + "%s: %s (libssh2 error code: %d, sftp error code: %lu)",
> + msg, ssh_err, ssh_err_code, sftp_err_code);
> + } else {
> + error_setg(errp, "%s", msg);
> + }
> + g_free(msg);
> }
>
> static void GCC_FMT_ATTR(2, 3)
> @@ -594,14 +599,14 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> path, ssh_flags, creat_mode);
> s->sftp_handle = libssh2_sftp_open(s->sftp, path, ssh_flags, creat_mode);
> if (!s->sftp_handle) {
> - session_error_report(s, "failed to open remote file '%s'", path);
> + session_error_setg(errp, s, "failed to open remote file '%s'", path);
> ret = -EINVAL;
> goto err;
> }
>
> r = libssh2_sftp_fstat(s->sftp_handle, &s->attrs);
> if (r < 0) {
> - sftp_error_report(s, "failed to read file attributes");
> + sftp_error_setg(errp, s, "failed to read file attributes");
> return -EINVAL;
> }
>
> @@ -639,7 +644,6 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
> Error **errp)
> {
> - Error *local_err = NULL;
> BDRVSSHState *s = bs->opaque;
> int ret;
> int ssh_flags;
> @@ -652,10 +656,8 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
> }
>
> /* Start up SSH. */
> - ret = connect_to_ssh(s, options, ssh_flags, 0, &local_err);
> + ret = connect_to_ssh(s, options, ssh_flags, 0, errp);
> if (ret < 0) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> goto err;
> }
>
> @@ -686,7 +688,6 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
> Error **errp)
> {
> int r, ret;
> - Error *local_err = NULL;
> int64_t total_size = 0;
> QDict *uri_options = NULL;
> BDRVSSHState s;
> @@ -705,10 +706,8 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
> DPRINTF("total_size=%" PRIi64, total_size);
>
> uri_options = qdict_new();
> - r = parse_uri(filename, uri_options, &local_err);
> + r = parse_uri(filename, uri_options, errp);
> if (r < 0) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> ret = r;
> goto out;
> }
> @@ -716,10 +715,8 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
> r = connect_to_ssh(&s, uri_options,
> LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE|
> LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC,
> - 0644, &local_err);
> + 0644, errp);
> if (r < 0) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> ret = r;
> goto out;
> }
> @@ -728,7 +725,7 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options,
> libssh2_sftp_seek64(s.sftp_handle, total_size-1);
> r2 = libssh2_sftp_write(s.sftp_handle, c, 1);
> if (r2 < 0) {
> - sftp_error_report(&s, "truncate failed");
> + sftp_error_setg(errp, &s, "truncate failed");
> ret = -EINVAL;
> goto out;
> }
> --
> 1.8.1.4
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 10/18] block/vvfat: Propagate errors through enable_write_target()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (8 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-14 16:57 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories() Markus Armbruster
` (7 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Continues the conversion of the open method to Error started in commit
015a103.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/vvfat.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index c3af7ff..3eccd68 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -979,7 +979,7 @@ static int init_directories(BDRVVVFATState* s,
static BDRVVVFATState *vvv = NULL;
#endif
-static int enable_write_target(BDRVVVFATState *s);
+static int enable_write_target(BDRVVVFATState *s, Error **errp);
static int is_consistent(BDRVVVFATState *s);
static void vvfat_rebind(BlockDriverState *bs)
@@ -1160,7 +1160,7 @@ DLOG(if (stderr == NULL) {
s->sector_count = cyls * heads * secs - (s->first_sectors_number - 1);
if (qemu_opt_get_bool(opts, "rw", false)) {
- ret = enable_write_target(s);
+ ret = enable_write_target(s, errp);
if (ret < 0) {
goto fail;
}
@@ -2904,11 +2904,10 @@ static BlockDriver vvfat_write_target = {
.bdrv_close = write_target_close,
};
-static int enable_write_target(BDRVVVFATState *s)
+static int enable_write_target(BDRVVVFATState *s, Error **errp)
{
BlockDriver *bdrv_qcow;
QEMUOptionParameter *options;
- Error *local_err = NULL;
int ret;
int size = sector2cluster(s, s->sector_count);
s->used_clusters = calloc(size, 1);
@@ -2918,6 +2917,7 @@ static int enable_write_target(BDRVVVFATState *s)
s->qcow_filename = g_malloc(1024);
ret = get_tmp_filename(s->qcow_filename, 1024);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "can't create temporary file");
goto err;
}
@@ -2926,20 +2926,16 @@ 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, &local_err);
+ ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, errp);
if (ret < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
goto err;
}
s->qcow = NULL;
ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
- &local_err);
+ errp);
if (ret < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
goto err;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 10/18] block/vvfat: Propagate errors through enable_write_target()
2014-05-13 16:02 ` [Qemu-devel] [PATCH 10/18] block/vvfat: Propagate errors through enable_write_target() Markus Armbruster
@ 2014-05-14 16:57 ` Eric Blake
2014-05-14 17:36 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Eric Blake @ 2014-05-14 16:57 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: kwolf, stefanha
[-- Attachment #1: Type: text/plain, Size: 798 bytes --]
On 05/13/2014 10:02 AM, Markus Armbruster wrote:
> Continues the conversion of the open method to Error started in commit
> 015a103.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> block/vvfat.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> s->qcow = NULL;
> ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, NULL,
> BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
> - &local_err);
> + errp);
Pre-existing; indentation looks odd here if you want to touch it up at
the same time. But that's not a show-stopper.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH 10/18] block/vvfat: Propagate errors through enable_write_target()
2014-05-14 16:57 ` Eric Blake
@ 2014-05-14 17:36 ` Markus Armbruster
0 siblings, 0 replies; 47+ messages in thread
From: Markus Armbruster @ 2014-05-14 17:36 UTC (permalink / raw)
To: Eric Blake; +Cc: kwolf, qemu-devel, stefanha
Eric Blake <eblake@redhat.com> writes:
> On 05/13/2014 10:02 AM, Markus Armbruster wrote:
>> Continues the conversion of the open method to Error started in commit
>> 015a103.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> block/vvfat.c | 16 ++++++----------
>> 1 file changed, 6 insertions(+), 10 deletions(-)
>>
>
>> s->qcow = NULL;
>> ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, NULL,
>> BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
>> - &local_err);
>> + errp);
>
> Pre-existing; indentation looks odd here if you want to touch it up at
> the same time. But that's not a show-stopper.
Can take care of it when I respin to correct the leak in 04/18.
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (9 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 10/18] block/vvfat: Propagate errors through enable_write_target() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-14 17:45 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 12/18] block/sheepdog: Propagate errors through connect_to_sdog() Markus Armbruster
` (6 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Completes the conversion of the open method to Error started in commit
015a103.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/vvfat.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 3eccd68..c6a1211 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -831,7 +831,8 @@ static inline off_t cluster2sector(BDRVVVFATState* s, uint32_t cluster_num)
}
static int init_directories(BDRVVVFATState* s,
- const char *dirname, int heads, int secs)
+ const char *dirname, int heads, int secs,
+ Error **errp)
{
bootsector_t* bootsector;
mapping_t* mapping;
@@ -892,8 +893,8 @@ static int init_directories(BDRVVVFATState* s,
if (mapping->mode & MODE_DIRECTORY) {
mapping->begin = cluster;
if(read_directory(s, i)) {
- fprintf(stderr, "Could not read directory %s\n",
- mapping->path);
+ error_setg(errp, "Could not read directory %s",
+ mapping->path);
return -1;
}
mapping = array_get(&(s->mapping), i);
@@ -919,9 +920,10 @@ static int init_directories(BDRVVVFATState* s,
cluster = mapping->end;
if(cluster > s->cluster_count) {
- fprintf(stderr,"Directory does not fit in FAT%d (capacity %.2f MB)\n",
- s->fat_type, s->sector_count / 2000.0);
- return -EINVAL;
+ error_setg(errp,
+ "Directory does not fit in FAT%d (capacity %.2f MB)",
+ s->fat_type, s->sector_count / 2000.0);
+ return -1;
}
/* fix fat for entry */
@@ -1169,7 +1171,7 @@ DLOG(if (stderr == NULL) {
bs->total_sectors = cyls * heads * secs;
- if (init_directories(s, dirname, heads, secs)) {
+ if (init_directories(s, dirname, heads, secs, errp)) {
ret = -EIO;
goto fail;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories()
2014-05-13 16:02 ` [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories() Markus Armbruster
@ 2014-05-14 17:45 ` Eric Blake
2014-05-14 19:48 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Eric Blake @ 2014-05-14 17:45 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: kwolf, stefanha
[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]
On 05/13/2014 10:02 AM, Markus Armbruster wrote:
> Completes the conversion of the open method to Error started in commit
> 015a103.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> block/vvfat.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> @@ -892,8 +893,8 @@ static int init_directories(BDRVVVFATState* s,
> if (mapping->mode & MODE_DIRECTORY) {
> mapping->begin = cluster;
> if(read_directory(s, i)) {
> - fprintf(stderr, "Could not read directory %s\n",
> - mapping->path);
> + error_setg(errp, "Could not read directory %s",
> + mapping->path);
I see you fixed some TABs in the process; is it worth widening the fix
to the rest of the 'if' statement? I don't care either way, as long as
checkpatch.pl didn't call you out (the new code is correct, even though
the existing code was not).
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories()
2014-05-14 17:45 ` Eric Blake
@ 2014-05-14 19:48 ` Markus Armbruster
0 siblings, 0 replies; 47+ messages in thread
From: Markus Armbruster @ 2014-05-14 19:48 UTC (permalink / raw)
To: Eric Blake; +Cc: kwolf, qemu-devel, stefanha
Eric Blake <eblake@redhat.com> writes:
> On 05/13/2014 10:02 AM, Markus Armbruster wrote:
>> Completes the conversion of the open method to Error started in commit
>> 015a103.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> block/vvfat.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> @@ -892,8 +893,8 @@ static int init_directories(BDRVVVFATState* s,
>> if (mapping->mode & MODE_DIRECTORY) {
>> mapping->begin = cluster;
>> if(read_directory(s, i)) {
>> - fprintf(stderr, "Could not read directory %s\n",
>> - mapping->path);
>> + error_setg(errp, "Could not read directory %s",
>> + mapping->path);
>
> I see you fixed some TABs in the process; is it worth widening the fix
> to the rest of the 'if' statement? I don't care either way, as long as
> checkpatch.pl didn't call you out (the new code is correct, even though
> the existing code was not).
checkpatch is happy.
The fewer lines in vvfat.c git blames on me, the happier I am ;)
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 12/18] block/sheepdog: Propagate errors through connect_to_sdog()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (10 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-14 18:19 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 13/18] block/sheepdog: Propagate errors through get_sheep_fd() Markus Armbruster
` (5 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, MORITA Kazutaka, stefanha
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/sheepdog.c | 77 ++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 55 insertions(+), 22 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 2c3fb01..ff0aa89 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -526,17 +526,16 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
return acb;
}
-static int connect_to_sdog(BDRVSheepdogState *s)
+static int connect_to_sdog(BDRVSheepdogState *s, Error **errp)
{
int fd;
- Error *err = NULL;
if (s->is_unix) {
- fd = unix_connect(s->host_spec, &err);
+ fd = unix_connect(s->host_spec, errp);
} else {
- fd = inet_connect(s->host_spec, &err);
+ fd = inet_connect(s->host_spec, errp);
- if (err == NULL) {
+ if (fd >= 0) {
int ret = socket_set_nodelay(fd);
if (ret < 0) {
error_report("%s", strerror(errno));
@@ -544,10 +543,7 @@ static int connect_to_sdog(BDRVSheepdogState *s)
}
}
- if (err != NULL) {
- qerror_report_err(err);
- error_free(err);
- } else {
+ if (fd >= 0) {
qemu_set_nonblock(fd);
}
@@ -916,10 +912,13 @@ static void co_write_request(void *opaque)
*/
static int get_sheep_fd(BDRVSheepdogState *s)
{
+ Error *local_err = NULL;
int fd;
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return fd;
}
@@ -1063,14 +1062,17 @@ static int find_vdi_name(BDRVSheepdogState *s, const char *filename,
uint32_t snapid, const char *tag, uint32_t *vid,
bool lock)
{
+ Error *local_err = NULL;
int ret, fd;
SheepdogVdiReq hdr;
SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
unsigned int wlen, rlen = 0;
char buf[SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN];
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return fd;
}
@@ -1263,12 +1265,15 @@ static int write_object(int fd, char *buf, uint64_t oid, uint8_t copies,
/* update inode with the latest state */
static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag)
{
+ Error *local_err = NULL;
SheepdogInode *inode;
int ret = 0, fd;
uint32_t vid = 0;
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return -EIO;
}
@@ -1436,8 +1441,10 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
s->is_snapshot = true;
}
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
ret = fd;
goto out;
}
@@ -1474,14 +1481,17 @@ out:
static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot)
{
+ Error *local_err = NULL;
SheepdogVdiReq hdr;
SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
int fd, ret;
unsigned int wlen, rlen = 0;
char buf[SD_MAX_VDI_LEN];
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return fd;
}
@@ -1730,6 +1740,7 @@ out:
static void sd_close(BlockDriverState *bs)
{
+ Error *local_err = NULL;
BDRVSheepdogState *s = bs->opaque;
SheepdogVdiReq hdr;
SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
@@ -1738,8 +1749,10 @@ static void sd_close(BlockDriverState *bs)
DPRINTF("%s\n", s->name);
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return;
}
@@ -1774,6 +1787,7 @@ static int64_t sd_getlength(BlockDriverState *bs)
static int sd_truncate(BlockDriverState *bs, int64_t offset)
{
+ Error *local_err = NULL;
BDRVSheepdogState *s = bs->opaque;
int ret, fd;
unsigned int datalen;
@@ -1786,8 +1800,10 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
return -EINVAL;
}
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return fd;
}
@@ -1846,6 +1862,7 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
/* Delete current working VDI on the snapshot chain */
static bool sd_delete(BDRVSheepdogState *s)
{
+ Error *local_err = NULL;
unsigned int wlen = SD_MAX_VDI_LEN, rlen = 0;
SheepdogVdiReq hdr = {
.opcode = SD_OP_DEL_VDI,
@@ -1856,8 +1873,10 @@ static bool sd_delete(BDRVSheepdogState *s)
SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
int fd, ret;
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return false;
}
@@ -1885,6 +1904,7 @@ static bool sd_delete(BDRVSheepdogState *s)
*/
static int sd_create_branch(BDRVSheepdogState *s)
{
+ Error *local_err = NULL;
int ret, fd;
uint32_t vid;
char *buf;
@@ -1907,8 +1927,10 @@ static int sd_create_branch(BDRVSheepdogState *s)
DPRINTF("%" PRIx32 " is created.\n", vid);
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
ret = fd;
goto out;
}
@@ -2122,6 +2144,7 @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
{
+ Error *local_err = NULL;
BDRVSheepdogState *s = bs->opaque;
int ret, fd;
uint32_t new_vid;
@@ -2151,8 +2174,10 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
/* refresh inode. */
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
ret = fd;
goto cleanup;
}
@@ -2249,6 +2274,7 @@ static int sd_snapshot_delete(BlockDriverState *bs,
static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
{
+ Error *local_err = NULL;
BDRVSheepdogState *s = bs->opaque;
SheepdogReq req;
int fd, nr = 1024, ret, max = BITS_TO_LONGS(SD_NR_VDIS) * sizeof(long);
@@ -2263,8 +2289,10 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
vdi_inuse = g_malloc(max);
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
ret = fd;
goto out;
}
@@ -2290,8 +2318,10 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
hval = fnv_64a_buf(s->name, strlen(s->name), FNV1A_64_INIT);
start_nr = hval & (SD_NR_VDIS - 1);
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
ret = fd;
goto out;
}
@@ -2341,6 +2371,7 @@ out:
static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data,
int64_t pos, int size, int load)
{
+ Error *local_err = NULL;
bool create;
int fd, ret = 0, remaining = size;
unsigned int data_len;
@@ -2349,8 +2380,10 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data,
uint32_t vdi_index;
uint32_t vdi_id = load ? s->inode.parent_vdi_id : s->inode.vdi_id;
- fd = connect_to_sdog(s);
+ fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return fd;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 13/18] block/sheepdog: Propagate errors through get_sheep_fd()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (11 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 12/18] block/sheepdog: Propagate errors through connect_to_sdog() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-15 18:54 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc() Markus Armbruster
` (4 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, MORITA Kazutaka, stefanha
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/sheepdog.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index ff0aa89..b932d68 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -668,7 +668,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
enum AIOCBState aiocb_type);
static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag);
-static int get_sheep_fd(BDRVSheepdogState *s);
+static int get_sheep_fd(BDRVSheepdogState *s, Error **errp);
static void co_write_request(void *opaque);
static AIOReq *find_pending_req(BDRVSheepdogState *s, uint64_t oid)
@@ -705,6 +705,7 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid)
static coroutine_fn void reconnect_to_sdog(void *opaque)
{
+ Error *local_err = NULL;
BDRVSheepdogState *s = opaque;
AIOReq *aio_req, *next;
@@ -719,9 +720,11 @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
/* Try to reconnect the sheepdog server every one second. */
while (s->fd < 0) {
- s->fd = get_sheep_fd(s);
+ s->fd = get_sheep_fd(s, &local_err);
if (s->fd < 0) {
DPRINTF("Wait for connection to be established\n");
+ qerror_report_err(local_err);
+ error_free(local_err);
co_aio_sleep_ns(bdrv_get_aio_context(s->bs), QEMU_CLOCK_REALTIME,
1000000000ULL);
}
@@ -910,15 +913,12 @@ static void co_write_request(void *opaque)
* We cannot use this descriptor for other operations because
* the block driver may be on waiting response from the server.
*/
-static int get_sheep_fd(BDRVSheepdogState *s)
+static int get_sheep_fd(BDRVSheepdogState *s, Error **errp)
{
- Error *local_err = NULL;
int fd;
- fd = connect_to_sdog(s, &local_err);
+ fd = connect_to_sdog(s, errp);
if (fd < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
return fd;
}
@@ -1415,8 +1415,10 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
if (ret < 0) {
goto out;
}
- s->fd = get_sheep_fd(s);
+ s->fd = get_sheep_fd(s, &local_err);
if (s->fd < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
ret = s->fd;
goto out;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (12 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 13/18] block/sheepdog: Propagate errors through get_sheep_fd() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-15 18:59 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 15/18] block/sheepdog: Propagate errors through do_sd_create() Markus Armbruster
` (3 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, MORITA Kazutaka, stefanha
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/sheepdog.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index b932d68..a8385bb 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1537,27 +1537,24 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot)
return 0;
}
-static int sd_prealloc(const char *filename)
+static int sd_prealloc(const char *filename, Error **errp)
{
BlockDriverState *bs = NULL;
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_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
- NULL, &local_err);
+ NULL, errp);
if (ret < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
goto out;
}
vdi_size = bdrv_getlength(bs);
if (vdi_size < 0) {
ret = vdi_size;
- goto out;
+ goto out_setg;
}
max_idx = DIV_ROUND_UP(vdi_size, SD_DATA_OBJ_SIZE);
@@ -1568,13 +1565,15 @@ static int sd_prealloc(const char *filename)
*/
ret = bdrv_pread(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
if (ret < 0) {
- goto out;
+ goto out_setg;
}
ret = bdrv_pwrite(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
if (ret < 0) {
- goto out;
+ goto out_setg;
}
}
+out_setg:
+ error_setg_errno(errp, -ret, "Can't pre-allocate");
out:
if (bs) {
bdrv_unref(bs);
@@ -1734,7 +1733,11 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
goto out;
}
- ret = sd_prealloc(filename);
+ ret = sd_prealloc(filename, &local_err);
+ if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ }
out:
g_free(s);
return ret;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc()
2014-05-13 16:02 ` [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc() Markus Armbruster
@ 2014-05-15 18:59 ` Eric Blake
2014-05-15 19:45 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Eric Blake @ 2014-05-15 18:59 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: kwolf, stefanha, MORITA Kazutaka
[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]
On 05/13/2014 10:02 AM, Markus Armbruster wrote:
> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> block/sheepdog.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> @@ -1568,13 +1565,15 @@ static int sd_prealloc(const char *filename)
> */
> ret = bdrv_pread(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
> if (ret < 0) {
> - goto out;
> + goto out_setg;
> }
> ret = bdrv_pwrite(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
> if (ret < 0) {
> - goto out;
> + goto out_setg;
> }
> }
> +out_setg:
> + error_setg_errno(errp, -ret, "Can't pre-allocate");
This unconditionally sets errp even when the for loop completes
normally. Are you sure you want this control flow? Maybe you are
missing a 'goto out' prior to the 'out_setg' label?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc()
2014-05-15 18:59 ` Eric Blake
@ 2014-05-15 19:45 ` Markus Armbruster
2014-05-16 8:54 ` Markus Armbruster
0 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-15 19:45 UTC (permalink / raw)
To: Eric Blake; +Cc: kwolf, MORITA Kazutaka, qemu-devel, stefanha
Eric Blake <eblake@redhat.com> writes:
> On 05/13/2014 10:02 AM, Markus Armbruster wrote:
>> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> block/sheepdog.c | 21 ++++++++++++---------
>> 1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> @@ -1568,13 +1565,15 @@ static int sd_prealloc(const char *filename)
>> */
>> ret = bdrv_pread(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
>> if (ret < 0) {
>> - goto out;
>> + goto out_setg;
>> }
>> ret = bdrv_pwrite(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
>> if (ret < 0) {
>> - goto out;
>> + goto out_setg;
>> }
>> }
>> +out_setg:
>> + error_setg_errno(errp, -ret, "Can't pre-allocate");
>
> This unconditionally sets errp even when the for loop completes
> normally. Are you sure you want this control flow? Maybe you are
> missing a 'goto out' prior to the 'out_setg' label?
You're right, will fix, thanks!
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc()
2014-05-15 19:45 ` Markus Armbruster
@ 2014-05-16 8:54 ` Markus Armbruster
0 siblings, 0 replies; 47+ messages in thread
From: Markus Armbruster @ 2014-05-16 8:54 UTC (permalink / raw)
To: Eric Blake; +Cc: kwolf, MORITA Kazutaka, qemu-devel, stefanha
Markus Armbruster <armbru@redhat.com> writes:
> Eric Blake <eblake@redhat.com> writes:
>
>> On 05/13/2014 10:02 AM, Markus Armbruster wrote:
>>> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>> block/sheepdog.c | 21 ++++++++++++---------
>>> 1 file changed, 12 insertions(+), 9 deletions(-)
>>>
>>> @@ -1568,13 +1565,15 @@ static int sd_prealloc(const char *filename)
>>> */
>>> ret = bdrv_pread(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
>>> if (ret < 0) {
>>> - goto out;
>>> + goto out_setg;
>>> }
>>> ret = bdrv_pwrite(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
>>> if (ret < 0) {
>>> - goto out;
>>> + goto out_setg;
>>> }
>>> }
>>> +out_setg:
>>> + error_setg_errno(errp, -ret, "Can't pre-allocate");
>>
>> This unconditionally sets errp even when the for loop completes
>> normally. Are you sure you want this control flow? Maybe you are
>> missing a 'goto out' prior to the 'out_setg' label?
>
> You're right, will fix, thanks!
I went over the whole series again looking for similar mistakes, and
found a few in PATCH 04 and 17. Respin coming.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH 15/18] block/sheepdog: Propagate errors through do_sd_create()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (13 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-15 19:02 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 16/18] block/sheepdog: Propagate errors through find_vdi_name() Markus Armbruster
` (2 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, MORITA Kazutaka, stefanha
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/sheepdog.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index a8385bb..d20a94c 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1481,19 +1481,17 @@ out:
return ret;
}
-static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot)
+static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot,
+ Error **errp)
{
- Error *local_err = NULL;
SheepdogVdiReq hdr;
SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
int fd, ret;
unsigned int wlen, rlen = 0;
char buf[SD_MAX_VDI_LEN];
- fd = connect_to_sdog(s, &local_err);
+ fd = connect_to_sdog(s, errp);
if (fd < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
return fd;
}
@@ -1522,11 +1520,12 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot)
closesocket(fd);
if (ret) {
+ error_setg_errno(errp, -ret, "create failed");
return ret;
}
if (rsp->result != SD_RES_SUCCESS) {
- error_report("%s, %s", sd_strerror(rsp->result), s->inode.name);
+ error_setg(errp, "%s, %s", sd_strerror(rsp->result), s->inode.name);
return -EIO;
}
@@ -1728,15 +1727,19 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
bdrv_unref(bs);
}
- ret = do_sd_create(s, &vid, 0);
- if (!prealloc || ret) {
+ ret = do_sd_create(s, &vid, 0, &local_err);
+ if (ret) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto out;
}
- ret = sd_prealloc(filename, &local_err);
- if (ret < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
+ if (prealloc) {
+ ret = sd_prealloc(filename, &local_err);
+ if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ }
}
out:
g_free(s);
@@ -1925,8 +1928,10 @@ static int sd_create_branch(BDRVSheepdogState *s)
* false bail out.
*/
deleted = sd_delete(s);
- ret = do_sd_create(s, &vid, !deleted);
+ ret = do_sd_create(s, &vid, !deleted, &local_err);
if (ret) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto out;
}
@@ -2194,8 +2199,10 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
goto cleanup;
}
- ret = do_sd_create(s, &new_vid, 1);
+ ret = do_sd_create(s, &new_vid, 1, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
error_report("failed to create inode for snapshot. %s",
strerror(errno));
goto cleanup;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 16/18] block/sheepdog: Propagate errors through find_vdi_name()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (14 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 15/18] block/sheepdog: Propagate errors through do_sd_create() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-15 19:06 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 17/18] block/sheepdog: Propagate errors to open and create methods Markus Armbruster
2014-05-13 16:02 ` [Qemu-devel] [PATCH 18/18] block/sheepdog: Don't use qerror_report() Markus Armbruster
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, MORITA Kazutaka, stefanha
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/sheepdog.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index d20a94c..2cf192d 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1060,19 +1060,16 @@ static int parse_vdiname(BDRVSheepdogState *s, const char *filename,
static int find_vdi_name(BDRVSheepdogState *s, const char *filename,
uint32_t snapid, const char *tag, uint32_t *vid,
- bool lock)
+ bool lock, Error **errp)
{
- Error *local_err = NULL;
int ret, fd;
SheepdogVdiReq hdr;
SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
unsigned int wlen, rlen = 0;
char buf[SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN];
- fd = connect_to_sdog(s, &local_err);
+ fd = connect_to_sdog(s, errp);
if (fd < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
return fd;
}
@@ -1097,12 +1094,13 @@ static int find_vdi_name(BDRVSheepdogState *s, const char *filename,
ret = do_req(fd, (SheepdogReq *)&hdr, buf, &wlen, &rlen);
if (ret) {
+ error_setg_errno(errp, -ret, "cannot get vdi info");
goto out;
}
if (rsp->result != SD_RES_SUCCESS) {
- error_report("cannot get vdi info, %s, %s %" PRIu32 " %s",
- sd_strerror(rsp->result), filename, snapid, tag);
+ error_setg(errp, "cannot get vdi info, %s, %s %" PRIu32 " %s",
+ sd_strerror(rsp->result), filename, snapid, tag);
if (rsp->result == SD_RES_NO_VDI) {
ret = -ENOENT;
} else {
@@ -1279,8 +1277,10 @@ static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag)
inode = g_malloc(sizeof(s->inode));
- ret = find_vdi_name(s, s->name, snapid, tag, &vid, false);
+ ret = find_vdi_name(s, s->name, snapid, tag, &vid, false, &local_err);
if (ret) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto out;
}
@@ -1423,8 +1423,10 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
- ret = find_vdi_name(s, vdi, snapid, tag, &vid, true);
+ ret = find_vdi_name(s, vdi, snapid, tag, &vid, true, &local_err);
if (ret) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto out;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 17/18] block/sheepdog: Propagate errors to open and create methods
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (15 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 16/18] block/sheepdog: Propagate errors through find_vdi_name() Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-15 19:07 ` Eric Blake
2014-05-13 16:02 ` [Qemu-devel] [PATCH 18/18] block/sheepdog: Don't use qerror_report() Markus Armbruster
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, MORITA Kazutaka, stefanha
Completes the conversion to Error started in commit 015a103^..d5124c0.
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/sheepdog.c | 30 +++++++-----------------------
1 file changed, 7 insertions(+), 23 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 2cf192d..cd5981e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1391,8 +1391,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (local_err) {
- qerror_report_err(local_err);
- error_free(local_err);
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto out;
}
@@ -1415,18 +1414,14 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
if (ret < 0) {
goto out;
}
- s->fd = get_sheep_fd(s, &local_err);
+ s->fd = get_sheep_fd(s, errp);
if (s->fd < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
ret = s->fd;
goto out;
}
- ret = find_vdi_name(s, vdi, snapid, tag, &vid, true, &local_err);
+ ret = find_vdi_name(s, vdi, snapid, tag, &vid, true, errp);
if (ret) {
- qerror_report_err(local_err);
- error_free(local_err);
goto out;
}
@@ -1445,10 +1440,8 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
s->is_snapshot = true;
}
- fd = connect_to_sdog(s, &local_err);
+ fd = connect_to_sdog(s, errp);
if (fd < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
ret = fd;
goto out;
}
@@ -1648,7 +1641,6 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
char tag[SD_MAX_VDI_TAG_LEN];
uint32_t snapid;
bool prealloc = false;
- Error *local_err = NULL;
s = g_malloc0(sizeof(BDRVSheepdogState));
@@ -1710,10 +1702,8 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
bs = NULL;
ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, NULL,
- &local_err);
+ errp);
if (ret < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
goto out;
}
@@ -1729,19 +1719,13 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
bdrv_unref(bs);
}
- ret = do_sd_create(s, &vid, 0, &local_err);
+ ret = do_sd_create(s, &vid, 0, errp);
if (ret) {
- qerror_report_err(local_err);
- error_free(local_err);
goto out;
}
if (prealloc) {
- ret = sd_prealloc(filename, &local_err);
- if (ret < 0) {
- qerror_report_err(local_err);
- error_free(local_err);
- }
+ ret = sd_prealloc(filename, errp);
}
out:
g_free(s);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH 18/18] block/sheepdog: Don't use qerror_report()
2014-05-13 16:02 [Qemu-devel] [PATCH 00/18] [PATCH 00/18] block: Purge qerror_report() Markus Armbruster
` (16 preceding siblings ...)
2014-05-13 16:02 ` [Qemu-devel] [PATCH 17/18] block/sheepdog: Propagate errors to open and create methods Markus Armbruster
@ 2014-05-13 16:02 ` Markus Armbruster
2014-05-15 19:08 ` Eric Blake
17 siblings, 1 reply; 47+ messages in thread
From: Markus Armbruster @ 2014-05-13 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, MORITA Kazutaka, stefanha
qerror_report() is a transitional interface to help with converting
existing HMP commands to QMP. It should not be used elsewhere.
Replace by error_report().
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/sheepdog.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index cd5981e..d3386b6 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -723,7 +723,7 @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
s->fd = get_sheep_fd(s, &local_err);
if (s->fd < 0) {
DPRINTF("Wait for connection to be established\n");
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));
error_free(local_err);
co_aio_sleep_ns(bdrv_get_aio_context(s->bs), QEMU_CLOCK_REALTIME,
1000000000ULL);
@@ -1270,7 +1270,7 @@ static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag)
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
return -EIO;
}
@@ -1279,7 +1279,7 @@ static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag)
ret = find_vdi_name(s, s->name, snapid, tag, &vid, false, &local_err);
if (ret) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
goto out;
}
@@ -1745,7 +1745,7 @@ static void sd_close(BlockDriverState *bs)
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
return;
}
@@ -1796,7 +1796,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
return fd;
}
@@ -1869,7 +1869,7 @@ static bool sd_delete(BDRVSheepdogState *s)
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
return false;
}
@@ -1916,7 +1916,7 @@ static int sd_create_branch(BDRVSheepdogState *s)
deleted = sd_delete(s);
ret = do_sd_create(s, &vid, !deleted, &local_err);
if (ret) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
goto out;
}
@@ -1925,7 +1925,7 @@ static int sd_create_branch(BDRVSheepdogState *s)
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
ret = fd;
goto out;
@@ -2172,7 +2172,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
/* refresh inode. */
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
ret = fd;
goto cleanup;
@@ -2187,7 +2187,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
ret = do_sd_create(s, &new_vid, 1, &local_err);
if (ret < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
error_report("failed to create inode for snapshot. %s",
strerror(errno));
@@ -2289,7 +2289,7 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
ret = fd;
goto out;
@@ -2318,7 +2318,7 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
ret = fd;
goto out;
@@ -2380,7 +2380,7 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data,
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- qerror_report_err(local_err);
+ error_report("%s", error_get_pretty(local_err));;
error_free(local_err);
return fd;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread