On Jul 3, 2019, at 11:57 AM, Michael S. Tsirkin <m...@redhat.com<mailto:m...@redhat.com>> 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<http://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. IIUC the comment refers to multiple virtqueue. It is trying to say that we do not need to send VHOST_USER_SET_MEM_TABLE for each virtqueue. A few questions: 1. What exactly is the check `if (vhost_user_one_time_request(msg->hdr.request) && dev->vq_index != 0)` for? Some backends register multiple dev instances per backend: one for each virtqueue. This check avoids sending VHOST_USER_SET_MEM_TABLE more than once in this case. 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://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_qemu_qemu_blob_master_hw_scsi_vhost-2Duser-2Dscsi.c-23&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=In4gmR1pGzKB8G5p6LUrWqkSMec2L5EtXZow_FZNJZk&m=JQ2hn-RhTfyhZ0Fxq5mzERaoVzb7cT8vcI7Xv20J3yk&s=aIeMP5ifKxF3no26c_M5x1rnVH0fhoFU2iRG9BPRNBk&e= L95. Is this check then ‘broken’ for such devices? I think vhost-scsi has a single instance per backend, that is why vq_index is 0. 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? Thanks, Raphael I don't think that is the case. It's possible that memory hotplug has bugs with vhost-user, but I don't think it's anything fundamental. -- MST Thanks for the responses Stefan and MST, I’ll be looking into it and posting an RFC when I'm ready.
