[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 06/13] nbd: remove peppering of nbd_client_connected
From: |
Eric Blake |
Subject: |
Re: [PULL 06/13] nbd: remove peppering of nbd_client_connected |
Date: |
Fri, 13 May 2022 15:42:09 -0500 |
User-agent: |
NeoMutt/20220429-54-2d64b2 |
On Thu, May 12, 2022 at 05:16:04PM +0100, Peter Maydell wrote:
> On Tue, 26 Apr 2022 at 21:21, Eric Blake <eblake@redhat.com> wrote:
> >
> > From: Paolo Bonzini <pbonzini@redhat.com>
> >
> > It is unnecessary to check nbd_client_connected() because every time
> > s->state is moved out of NBD_CLIENT_CONNECTED the socket is shut down
> > and all coroutines are resumed.
>
> Hi; Coverity points out (CID 1488362) that this part of this change
> has resulted in some dead code:
>
> > @@ -512,7 +508,7 @@ static int coroutine_fn
> > nbd_co_send_request(BlockDriverState *bs,
> > if (qiov) {
> > qio_channel_set_cork(s->ioc, true);
> > rc = nbd_send_request(s->ioc, request);
> > - if (nbd_client_connected(s) && rc >= 0) {
> > + if (rc >= 0) {
> > if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
> > NULL) < 0) {
> > rc = -EIO;
>
> because the change means this code is now
>
> if (rc >= 0) {
> if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
> NULL) < 0) {
> rc = -EIO;
> }
> } else if (rc >= 0) {
> rc = -EIO;
> }
>
> and the "else if" clause is dead and can be deleted.
Thanks for reporting it. I can prepare a patch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org