[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] 9pfs: fix v9fs_lock error case
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH] 9pfs: fix v9fs_lock error case |
Date: |
Thu, 26 Jan 2017 12:54:33 +0100 |
On Thu, 26 Jan 2017 11:07:05 +0100
Paolo Bonzini <address@hidden> wrote:
> In this case, we are marshaling an error status instead of the errno value.
> Reorganize the out and out_nofid labels to look like all the other cases.
> Coverity reports this because the "err = -ENOENT" and "err = -EINVAL"
> assignments above are dead, overwritten by the call to pdu_marshal.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
There was a confusion indeed: if the server fails it should report it to the
client with an RERROR message.
Responding an RLOCK message with a P9_LOCK_ERROR status only makes sense
when actually implementing locking (i.e. calling flock() on the backend),
which isn't the case in QEMU as stated in the comment above v9fs_lock().
We should hence always report a P9_LOCK_SUCCESS status when responding
an RLOCK message.
Just to make it clear, I've modified your patch to open code this and
pushed it to https://github.com/gkurz/qemu/commits/9p-next .
BTW, I've registered to https://scan.coverity.com/projects/qemu as
Peter suggested on IRC. I'll have a look at the other 9pfs issues.
Cheers.
--
Greg
> hw/9pfs/9p.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 99e9472..d028eca 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3045,14 +3045,15 @@ static void coroutine_fn v9fs_lock(void *opaque)
> goto out;
> }
> status = P9_LOCK_SUCCESS;
> -out:
> - put_fid(pdu, fidp);
> -out_nofid:
> err = pdu_marshal(pdu, offset, "b", status);
> - if (err > 0) {
> - err += offset;
> + if (err < 0) {
> + goto out;
> }
> + err += offset;
> trace_v9fs_lock_return(pdu->tag, pdu->id, status);
> +out:
> + put_fid(pdu, fidp);
> +out_nofid:
> pdu_complete(pdu, err);
> v9fs_string_free(&flock.client_id);
> }