On 31.01.2014 09:57, Stefan Hajnoczi wrote:
On Thu, Jan 30, 2014 at 10:35 PM, Peter Lieven <p...@kamp.de> wrote:
Am 30.01.2014 um 15:22 schrieb Stefan Hajnoczi <stefa...@gmail.com>:
On Wed, Jan 29, 2014 at 05:19:59PM +0100, Benoît Canet wrote:
Le Wednesday 29 Jan 2014 à 09:50:21 (+0100), Peter Lieven a écrit :
+static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp) {
+ NFSClient *client = bs->opaque;
+ int64_t ret;
+ QemuOpts *opts;
+ Error *local_err = NULL;
+
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
+ qemu_opts_absorb_qdict(opts, options, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
I have seen more usage of error_propagate(errp, local_err); in QEMU code.
Maybe I am missing the point.
Yes, I think you are right. The Error should be propagated to the
caller. It's not clear to me whether we can ever get an error from
qemu_opts_absorb_qdict() in this call site though.
Is there any action I should take here? If yes, can you advise what
to do please.
The issue is that nfs_file_open() takes an Error **errp argument.
This means the function should report detailed errors using the Error
object.
The patch prints and then discards the local_error instead of
propagating it to the caller's errp.
We should just propagate the error instead of printing it:
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
goto ...;
Ok, you are just referring to this part in nfs_file_open:
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
error_free(local_err);
return -EINVAL;
}
which I would change to:
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
return -EINVAL;
}
The use of error_setg in nfs_client_open is ok?
Peter
