On Thu, 26 Jan 2017 11:07:05 +0100 Paolo Bonzini <pbonz...@redhat.com> 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 <pbonz...@redhat.com> > --- 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); > }