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?
> +
> + 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
>
