Re: [Qemu-devel] Should memory hotplug work with vhost-user backends?

[Raphael Norwitz]

On Wed, Jul 03, 2019 at 11:04:31AM +0100, Stefan Hajnoczi wrote:
> On Tue, Jul 02, 2019 at 10:08:54PM +0000, Raphael Norwitz wrote:
> > For background I am trying to work around a ram slot limit imposed by the 
> > vhost-user protocol. We are having trouble reconciling the comment here: 
> > https://github.com/qemu/qemu/blob/master/hw/virtio/vhost-user.c#L333  that 
> > “For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE., we just 
> > need to send it once the first time” and the high level implementation of 
> > memory hot-add, which calls set_mem_table every time a VM hot adds memory.
> > 
> > A few questions:
> > 1.
> > What exactly is the check `if 
> > (vhost_user_one_time_request(msg->hdr.request) && dev->vq_index != 0)` for? 
> > In the message for commit b931bfbf042983f311b3b09894d8030b2755a638, which 
> > introduced the check, I see it says “non-vring specific messages[, which 
> > should] be sent only once” and gives VHOST_USER_SET_MEM_TABLE as an example 
> > one such message. The `vhost_user_one_time_request()` call clearly checks 
> > whether this type of message is the kind of message is supposed to be sent 
> > once of which VHOST_USER_SET_MEM_TABLE is one. Why, then, does this commit 
> > add the check if `dev->vq_index != 0`? It seems like there is a latent 
> > assumption that after the first call dev->vq_index should be set to some 
> > value greater than one, however for many cases such as vhost-user-scsi 
> > devices we can see this is clearly not the case 
> > https://github.com/qemu/qemu/blob/master/hw/scsi/vhost-user-scsi.c#L95. Is 
> > this check then ‘broken’ for such devices?
> > 
> > 2.
> > If this check is indeed broken for such devices, and set_mem_table call is 
> > only supposed to be run once for such devices, is the ability to call it 
> > multiple times technically a bug for devices such as vhost-user-scsci 
> > devices? If so, this would imply that the existing ability to hot add 
> > memory to vhost-user-scsi devices is by extension technically a 
> > bug/unintended behavior. Is this the case?
> 
> Hi Raphael,
> David Gilbert and I recently came to the conclusion that memory hotplug
> is not safe in vhost-user device backends built using libvhost-user.

Hi David, Sefan,

Just want to follow up here. Sorry - I know this was a while ago.

I am looking to add postcopy migration support for my patch set lifting
the vhost-user max ram slots limitation
(https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06641.html)
and it seems the most convienient way to do this would be to add support
for my new protocol feature in libvhost-user and then test with
vhost-user-bridge.

I've briefly looked through the libvhost-user code and the hot-add path
looks safe enough to me (or at least no more broken than the regular
vhost-user memory hot-add path).

Can you elaborate a little more about why memory hot-add is unsafe with
vhost-user device backends built with libvhost-user, as opposed to those
using the raw vhost-user protocol semantics?


Thanks,
Raphael

> 
> It's likely that memory hotplug hasn't been fully thought through at the
> protocol specification and QEMU vhost-user master implementation levels
> either.
> 
> We didn't investigate deeper for the time being, but I'm not surprised
> that you've found inconsistencies.  The ability to hotplug memory is a
> valuable feature.  It will be necessary to get it working sooner or
> later.
> 
> Are you going to work on it?
> 
> Stefan