qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Hawkins Jiawei <yin31149@gmail.com>
To: Eugenio Perez Martin <eperezma@redhat.com>
Cc: jasowang@redhat.com, 18801353760@163.com, qemu-devel@nongnu.org
Subject: Re: [PATCH 2/2] vdpa: send CVQ state load commands in parallel
Date: Thu, 20 Apr 2023 23:22:42 +0800	[thread overview]
Message-ID: <CAKrof1OSxO1Y41T5-OWpncPnBzkQ6PBpSN8SNR_pkZF4ueBnpw@mail.gmail.com> (raw)
In-Reply-To: <CAKrof1OoPecP-8A_UXtEeyOsinciuceTuBFm_GLoV-wo1OQLOA@mail.gmail.com>

On Thu, 20 Apr 2023 at 19:38, Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> On Thu, 20 Apr 2023 at 01:43, Eugenio Perez Martin <eperezma@redhat.com> wrote:
> >
> > On Wed, Apr 19, 2023 at 1:50 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
> > >
> > > +         ++status) {
> > > +        if (*status != VIRTIO_NET_OK) {
> > > +            ++r;
> > > +        }
> > > +    }
> > > +
> > > +    return r;
> >
> > Although the caller is fine with >=0, I think we should keep the 0 ==
> > success. The number of commands delivered does not make a lot of sense
> > for the callers, just if the call succeeded or not.
>
> Thanks for the explanation, I will refactor the patch as you suggested.

I still have some questions about the check on device used buffers.

My initial thought was to return the number of commands whose
`in buffer` value is not VIRTIO_NET_OK.

If we are not supposed to return value > 0, what should we return
if some commands' `in buffer` value  is not VIRTIO_NET_OK.

Should we return an error code, such as EINVAL(Invalid argument),
indicating that QEMU can not successfully send all SVQ commands
in the current state. Or should we just do not check the device used buffers,
and return 0 when QEMU finishes polling?

Thanks!

>
> >
> > Thanks!
> >
> > >  }
> > >
> > >  static NetClientInfo net_vhost_vdpa_cvq_info = {
> > > --
> > > 2.25.1
> > >
> >


  reply	other threads:[~2023-04-20 15:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-19 11:49 [PATCH 0/2] Send all the SVQ control commands in parallel Hawkins Jiawei
2023-04-19 11:49 ` [PATCH 1/2] vdpa: rename vhost_vdpa_net_cvq_add() Hawkins Jiawei
2023-04-19 11:49 ` [PATCH 2/2] vdpa: send CVQ state load commands in parallel Hawkins Jiawei
2023-04-19 17:42   ` Eugenio Perez Martin
2023-04-20 11:38     ` Hawkins Jiawei
2023-04-20 15:22       ` Hawkins Jiawei [this message]
2023-04-19 17:16 ` [PATCH 0/2] Send all the SVQ control " Eugenio Perez Martin
2023-04-20  8:32   ` Hawkins Jiawei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKrof1OSxO1Y41T5-OWpncPnBzkQ6PBpSN8SNR_pkZF4ueBnpw@mail.gmail.com \
    --to=yin31149@gmail.com \
    --cc=18801353760@163.com \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).