On Mon, Oct 12, 2020 at 7:12 AM chenjiajun <chenjiaj...@huawei.com> wrote:
>
>
>
> On 2020/10/2 10:05, Raphael Norwitz wrote:
> > On Mon, Sep 28, 2020 at 9:17 AM Jiajun Chen <chenjiaj...@huawei.com> wrote:
> >>
> >> Used_memslots is equal to dev->mem->nregions now, it is true for
> >> vhost kernel, but not for vhost user, which uses the memory regions
> >> that have file descriptor. In fact, not all of the memory regions
> >> have file descriptor.
> >> It is usefully in some scenarios, e.g. used_memslots is 8, and only
> >> 5 memory slots can be used by vhost user, it is failed to hot plug
> >> a new memory RAM because vhost_has_free_slot just returned false,
> >> but we can hot plug it safely in fact.
> >>
> >> --
> >> ChangeList:
> >> v3:
> >> -make used_memslots a member of struct vhost_dev instead of a global
> >> static value
> >>
> >> v2:
> >> -eliminating useless used_memslots_exceeded variable and
> >> used_memslots_is_exceeded() API
> >>
> >> v1:
> >> -vhost-user: add separate memslot counter for vhost-user
> >>
> >> Signed-off-by: Jiajun Chen <chenjiaj...@huawei.com>
> >> Signed-off-by: Jianjay Zhou <jianjay.z...@huawei.com>
> >
> > I'm happy with this from a vhost/vhost-user perspective. vhost-backend
> > change looks good too. I'm a little confused by what's going on with
> > net/vhost-user.c.
> >
> >> ---
> >> hw/virtio/vhost-backend.c | 12 ++++++++++
> >> hw/virtio/vhost-user.c | 25 +++++++++++++++++++++
> >> hw/virtio/vhost.c | 37 +++++++++++++++++++++++--------
> >> include/hw/virtio/vhost-backend.h | 5 +++++
> >> include/hw/virtio/vhost.h | 1 +
> >> net/vhost-user.c | 7 ++++++
> >> 6 files changed, 78 insertions(+), 9 deletions(-)
> >>
> >
> >> diff --git a/net/vhost-user.c b/net/vhost-user.c
> >> index 17532daaf3..7e93955537 100644
> >> --- a/net/vhost-user.c
> >> +++ b/net/vhost-user.c
> >> @@ -20,6 +20,7 @@
> >> #include "qemu/error-report.h"
> >> #include "qemu/option.h"
> >> #include "trace.h"
> >> +#include "include/hw/virtio/vhost.h"
> >>
> >> typedef struct NetVhostUserState {
> >> NetClientState nc;
> >> @@ -347,6 +348,12 @@ static int net_vhost_user_init(NetClientState *peer,
> >> const char *device,
> >> qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
> >> net_vhost_user_event, NULL, nc0->name,
> >> NULL,
> >> true);
> >
> > Can you elaborate on this check here? What does it have to do with
> > fixing memslots accounting? Maybe it should be in a separate change?
> >
> When the number of virtual machine memslots exceeds the upper limit of the
> back-end support,
> QEMU main thread may enters an endless loop and cannot process other requests.
> And number of memslots will not automatically decrease, so add a check here
> to exit from loop
> in this scenario. For the newly started virtual machine, boot fails; for the
> hot plug network card,
> hot plug fails.
I don't understand what you mean by "number of memslots will not
automatically decrease". Where did this happen before and what changes
when the new memslots counter is introduced?
> >> +
> >> + if (!vhost_has_free_slot()) {
> >> + error_report("used memslots exceeded the backend limit, quit "
> >> + "loop");
> >> + goto err;
> >> + }
> >> } while (!s->started);
> >>
> >> assert(s->vhost_net);
> >> --
> >> 2.27.0.dirty
> >>
> > .
> >
