block: Purge qerror_report()
cover block: Purge qerror_report()
qerror_report() is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere. This series purges it from the block subsystem.
It is based on Fam's "[PATCH v19 00/16] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD".
Markus Armbruster (18):
blockdev: Don't use qerror_report_err() in drive_init()
blockdev: Don't use qerror_report() in do_drive_del()
qemu-nbd: Don't use qerror_report()
block/rbd: Propagate errors to open and create methods block/ssh: Drop superfluous libssh2_session_last_errno() calls block/ssh: Propagate errors through check_host_key() block/ssh: Propagate errors through authenticate() block/ssh: Propagate errors through connect_to_ssh() block/ssh: Propagate errors to open and create methods block/vvfat: Propagate errors through enable_write_target() block/vvfat: Propagate errors through init_directories() block/sheepdog: Propagate errors through connect_to_sdog() block/sheepdog: Propagate errors through get_sheep_fd() block/sheepdog: Propagate errors through sd_prealloc() block/sheepdog: Propagate errors through do_sd_create() block/sheepdog: Propagate errors through find_vdi_name() block/sheepdog: Propagate errors to open and create methods block/sheepdog: Don't use qerror_report()
block/rbd.c | 66 ++++++++++++------------ block/sheepdog.c | 135 ++++++++++++++++++++++++++++++------------------- block/ssh.c | 151 +++++++++++++++++++++++++++++++++---------------------- block/vvfat.c | 32 ++++++------ blockdev.c | 7 ++- qemu-nbd.c | 6 +-- 6 files changed, 229 insertions(+), 168 deletions(-)
Signature · 1 line
1.8.1.4
PATCH 1/18 blockdev: Don't use qerror_report_err() in drive_init()
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.
--- blockdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
blockdev.c +2 -2
@@ -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;
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 01/18] blockdev: Don't use qerror_report_err() in drive_init()
Quoted 4 lines · Markus Armbruster
qerror_report_err()is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere.
Quoted 8 lines
drive_init()is not meant to be used by QMP commands. It uses bothqerror_report_err()anderror_report(). Convert the former to the latter.Signed-off-by Markus Armbruster <[email protected]>--- blockdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 2/18 blockdev: Don't use qerror_report() in do_drive_del()
In reply to Markus Armbrusterqerror_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.
--- blockdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
blockdev.c +1 -2
@@ -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)) {
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 02/18] blockdev: Don't use qerror_report() in do_drive_del()
Quoted 11 lines · Markus Armbruster
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 bothqerror_report()anderror_report(). Convert the former to the latter.Signed-off-by Markus Armbruster <[email protected]>--- blockdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 3/18 qemu-nbd: Don't use qerror_report()
In reply to Markus Armbrusterqerror_report() is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere. Replace by error_report().
--- qemu-nbd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
qemu-nbd.c +3 -3
@@ -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;
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 03/18] qemu-nbd: Don't use qerror_report()
Quoted 8 lines · Markus Armbruster
qerror_report()is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere. Replace byerror_report().Signed-off-by Markus Armbruster <[email protected]>--- qemu-nbd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 4/18 block/rbd: Propagate errors to open and create methods
In reply to Markus ArmbrusterCompletes the conversion to Error started in commit 015a103^..d5124c0.
--- block/rbd.c | 66 +++++++++++++++++++++++++++++++------------------------------ 1 file changed, 34 insertions(+), 32 deletions(-)
block/rbd.c +34 -32
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 04/18] block/rbd: Propagate errors to open and create methods
Quoted 18 lines · Markus Armbruster
Completes the conversion to Error started in commit 015a103^..d5124c0.
Cc Josh Durgin <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/rbd.c | 66 +++++++++++++++++++++++++++++++------------------------------ 1 file changed, 34 insertions(+), 32 deletions(-)block/rbd.c +1
-0@@ -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 ;
block/rbd.c static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options, · line 310
Quoted 6 lines
block/rbd.c +1
-1@@ -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?
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 04/18] block/rbd: Propagate errors to open and create methods
Quoted 19 lines · Eric Blake
Quoted 18 lines · Markus Armbruster
Completes the conversion to Error started in commit 015a103^..d5124c0.
Cc Josh Durgin <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/rbd.c | 66 +++++++++++++++++++++++++++++++------------------------------ 1 file changed, 34 insertions(+), 32 deletions(-)block/rbd.c +1
-0@@ -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,
Quoted 8 lines
block/rbd.c static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options, · line 310
Quoted 6 lines
block/rbd.c +1
-1if (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!
PATCH 5/18 block/ssh: Drop superfluous libssh2_session_last_errno() calls
In reply to Markus Armbrusterlibssh2_session_last_error() already returns the error code.
--- block/ssh.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
block/ssh.c +4 -5
@@ -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);
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
Quoted 26 lines · Markus Armbruster
libssh2_session_last_error()already returns the error code.Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/ssh.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)block/ssh.c +2
-3@@ -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); }
block/ssh.c sftp_error_report(BDRVSSHState *s, const char *fs, ...) · line 151
Quoted 10 lines
block/ssh.c +2
-2@@ -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.
Signature · 5 lines
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
Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
Quoted 43 lines · Richard W.M. Jones
Quoted 26 lines · Markus Armbruster
libssh2_session_last_error()already returns the error code.Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/ssh.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)block/ssh.c +2
-3@@ -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); }block/ssh.c sftp_error_report(BDRVSSHState *s, const char *fs, ...) · line 151
Quoted 10 lines
block/ssh.c +2
-2@@ -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_errorreturns the error code, so there is no need to calllibssh2_session_last_errnoas well.
Quoted 1 line
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.
Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
Quoted 45 lines · Markus Armbruster
Quoted 43 lines · Richard W.M. Jones
Quoted 26 lines · Markus Armbruster
libssh2_session_last_error()already returns the error code.Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/ssh.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)block/ssh.c +2
-3@@ -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); }block/ssh.c sftp_error_report(BDRVSSHState *s, const char *fs, ...) · line 151
Quoted 10 lines
block/ssh.c +2
-2@@ -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_errorreturns the error code, so there is no need to calllibssh2_session_last_errnoas well.
Quoted 4 lines
Quoted 1 line
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.
Signature · 4 lines
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
Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
Quoted 51 lines · Richard W.M. Jones
Quoted 45 lines · Markus Armbruster
Quoted 43 lines · Richard W.M. Jones
Quoted 26 lines · Markus Armbruster
libssh2_session_last_error()already returns the error code.Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/ssh.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)block/ssh.c +2
-3@@ -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); }block/ssh.c sftp_error_report(BDRVSSHState *s, const char *fs, ...) · line 151
Quoted 10 lines
block/ssh.c +2
-2@@ -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_errorreturns the error code, so there is no need to calllibssh2_session_last_errnoas well.Quoted 4 lines
Quoted 1 line
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.
Quoted 6 lines
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 :)
Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls
In reply to Markus ArmbrusterQuoted 4 lines · Markus Armbruster
libssh2_session_last_error()already returns the error code.Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>
Quoted 34 lines
block/ssh.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)block/ssh.c +4
-5@@ -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);Signature · 1 line
1.8.1.4
Signature · 4 lines
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
PATCH 6/18 block/ssh: Propagate errors through check_host_key()
In reply to Markus Armbruster--- block/ssh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 19 deletions(-)
block/ssh.c +49 -19
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 06/18] block/ssh: Propagate errors through check_host_key()
Quoted 2 lines · Markus Armbruster
Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>
Quoted 170 lines
block/ssh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 19 deletions(-)block/ssh.c +49
-19@@ -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; }Signature · 1 line
1.8.1.4
Signature · 5 lines
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/
PATCH 7/18 block/ssh: Propagate errors through authenticate()
In reply to Markus Armbruster--- block/ssh.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
block/ssh.c +14 -9
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 07/18] block/ssh: Propagate errors through authenticate()
Quoted 2 lines · Markus Armbruster
Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>
Quoted 83 lines
block/ssh.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)block/ssh.c +14
-9@@ -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; }Signature · 1 line
1.8.1.4
Signature · 5 lines
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/
PATCH 8/18 block/ssh: Propagate errors through connect_to_ssh()
In reply to Markus Armbruster--- block/ssh.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)
block/ssh.c +17 -17
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 08/18] block/ssh: Propagate errors through connect_to_ssh()
Quoted 2 lines · Markus Armbruster
Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>
Quoted 120 lines
block/ssh.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)block/ssh.c +17
-17@@ -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; }Signature · 1 line
1.8.1.4
Signature · 5 lines
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
PATCH 9/18 block/ssh: Propagate errors to open and create methods
In reply to Markus ArmbrusterCompletes the conversion to Error started in commit 015a103^..d5124c0.
--- block/ssh.c | 47 ++++++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 25 deletions(-)
block/ssh.c +22 -25
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods
The last four patches are just a refactoring to use qemu error handling, therefore ACK to all four of them.
Rich.
Signature · 5 lines
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
Re: [Qemu-devel] [PATCH 09/18] block/ssh: Propagate errors to open and create methods
In reply to Markus ArmbrusterQuoted 4 lines · Markus Armbruster
Completes the conversion to Error started in commit 015a103^..d5124c0.
Cc "Richard W.M. Jones" <[email protected]>Signed-off-by Markus Armbruster <[email protected]>
Quoted 133 lines
block/ssh.c | 47 ++++++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 25 deletions(-)block/ssh.c +22
-25@@ -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; }Signature · 1 line
1.8.1.4
Signature · 5 lines
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/
PATCH 10/18 block/vvfat: Propagate errors through enable_write_target()
In reply to Markus ArmbrusterContinues the conversion of the open method to Error started in commit 015a103.
--- block/vvfat.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
block/vvfat.c +6 -10
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 10/18] block/vvfat: Propagate errors through enable_write_target()
Quoted 12 lines · Markus Armbruster
Continues the conversion of the open method to Error started in commit 015a103.
Signed-off-by Markus Armbruster <[email protected]>--- 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,
block/vvfat.c static int enable_write_target(BDRVVVFATState *s) · line 2937
Quoted 2 lines
block/vvfat.c +1
-1- &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.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 10/18] block/vvfat: Propagate errors through enable_write_target()
Quoted 10 lines · Eric Blake
Quoted 8 lines · Markus Armbruster
Continues the conversion of the open method to Error started in commit 015a103.
Signed-off-by Markus Armbruster <[email protected]>--- block/vvfat.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
Quoted 8 lines
block/vvfat.c static int enable_write_target(BDRVVVFATState *s) · line 2937
Quoted 5 lines
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,block/vvfat.c +1
-1- &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.
Quoted 1 line
Reviewed-by Eric Blake <[email protected]>
Thanks!
PATCH 11/18 block/vvfat: Propagate errors through init_directories()
In reply to Markus ArmbrusterCompletes the conversion of the open method to Error started in commit 015a103.
--- block/vvfat.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
block/vvfat.c +9 -7
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories()
Quoted 9 lines · Markus Armbruster
Completes the conversion of the open method to Error started in commit 015a103.
Signed-off-by Markus Armbruster <[email protected]>--- block/vvfat.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
block/vvfat.c static int init_directories(BDRVVVFATState* s, · line 897
Quoted 7 lines
block/vvfat.c +2
-2@@ -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).
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories()
Quoted 9 lines · Eric Blake
Quoted 8 lines · Markus Armbruster
Completes the conversion of the open method to Error started in commit 015a103.
Signed-off-by Markus Armbruster <[email protected]>--- block/vvfat.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
Quoted 13 lines
block/vvfat.c static int init_directories(BDRVVVFATState* s, · line 897
Quoted 8 lines
block/vvfat.c +2
-2@@ -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 ;)
Quoted 1 line
Reviewed-by Eric Blake <[email protected]>
Thanks!
PATCH 12/18 block/sheepdog: Propagate errors through connect_to_sdog()
In reply to Markus Armbruster--- block/sheepdog.c | 77 ++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 22 deletions(-)
block/sheepdog.c +55 -22
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 12/18] block/sheepdog: Propagate errors through connect_to_sdog()
Quoted 6 lines · Markus Armbruster
Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 77 ++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 22 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 13/18 block/sheepdog: Propagate errors through get_sheep_fd()
In reply to Markus Armbruster--- block/sheepdog.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
block/sheepdog.c +10 -8
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 13/18] block/sheepdog: Propagate errors through get_sheep_fd()
Quoted 6 lines · Markus Armbruster
Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 14/18 block/sheepdog: Propagate errors through sd_prealloc()
In reply to Markus Armbruster--- block/sheepdog.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
block/sheepdog.c +12 -9
@@ -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;
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc()
Quoted 11 lines · Markus Armbruster
Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)block/sheepdog.c +0
-1@@ -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;
block/sheepdog.c static int sd_prealloc(const char *filename) · line 1576
Quoted 10 lines
block/sheepdog.c +4
-1+ 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?
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc()
Quoted 18 lines · Eric Blake
Quoted 17 lines · Markus Armbruster
Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)block/sheepdog.c +2
-2@@ -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;
Quoted 8 lines
block/sheepdog.c static int sd_prealloc(const char *filename) · line 1576
Quoted 4 lines
block/sheepdog.c +2
-0} } +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!
Re: [Qemu-devel] [PATCH 14/18] block/sheepdog: Propagate errors through sd_prealloc()
Quoted 29 lines · Markus Armbruster
Quoted 18 lines · Eric Blake
Quoted 17 lines · Markus Armbruster
Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)block/sheepdog.c +2
-2@@ -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;Quoted 8 lines
block/sheepdog.c static int sd_prealloc(const char *filename) · line 1576
Quoted 4 lines
block/sheepdog.c +2
-0} } +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?
Quoted 1 line
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.
PATCH 15/18 block/sheepdog: Propagate errors through do_sd_create()
In reply to Markus Armbruster--- block/sheepdog.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)
block/sheepdog.c +21 -14
@@ -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;
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 15/18] block/sheepdog: Propagate errors through do_sd_create()
Quoted 6 lines · Markus Armbruster
Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 16/18 block/sheepdog: Propagate errors through find_vdi_name()
In reply to Markus Armbruster--- block/sheepdog.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
block/sheepdog.c +11 -9
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 16/18] block/sheepdog: Propagate errors through find_vdi_name()
Quoted 6 lines · Markus Armbruster
Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 17/18 block/sheepdog: Propagate errors to open and create methods
In reply to Markus ArmbrusterCompletes the conversion to Error started in commit 015a103^..d5124c0.
--- block/sheepdog.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-)
block/sheepdog.c +7 -23
@@ -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);
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 17/18] block/sheepdog: Propagate errors to open and create methods
Quoted 8 lines · Markus Armbruster
Completes the conversion to Error started in commit 015a103^..d5124c0.
Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 18/18 block/sheepdog: Don't use qerror_report()
In reply to Markus Armbrusterqerror_report() is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere. Replace by error_report().
--- block/sheepdog.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
block/sheepdog.c +13 -13
@@ -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; }
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 18/18] block/sheepdog: Don't use qerror_report()
Quoted 10 lines · Markus Armbruster
qerror_report()is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere. Replace byerror_report().Cc MORITA Kazutaka <[email protected]>Signed-off-by Markus Armbruster <[email protected]>--- block/sheepdog.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org