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 :
>
> Hi Peter,
> If I read your reply to Benoit correctly, you only addressed the
> questions about nfs_client_close(). Here are Benoits other comments and
> my thoughts on them:
>
>>> +static void
>>> +nfs_co_generic_cb(int status, struct nfs_context *nfs, void *data,
>>> + void *private_data)
>>> +{
>>> + NFSRPC *task = private_data;
>>> + task->complete = 1;
>>> + task->status = status;
>>> + if (task->status > 0 && task->iov) {
>>> + if (task->status <= task->iov->size) {
>> It feel very odd to compare something named status with something named size.
>
> Maybe the argument name isn't ideal. Something like 'ret' or 'result'
> would be more commonplace.
I took the "status" from libnfs examples. I can change it in the qemu code if
you like.
>
> Not critical but would be nice to change.
>
>>> +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.
Peter
