On Tue, Oct 27, 2015 at 05:30:41PM +0800, Yuanhan Liu wrote: > On Tue, Oct 27, 2015 at 11:17:16AM +0200, Michael S. Tsirkin wrote: > > On Tue, Oct 27, 2015 at 03:20:40PM +0900, Tetsuya Mukawa wrote: > > > On 2015/10/26 14:42, Yuanhan Liu wrote: > > > > On Mon, Oct 26, 2015 at 02:24:08PM +0900, Tetsuya Mukawa wrote: > > > >> On 2015/10/22 21:35, Yuanhan Liu wrote: > > > > ... > > > >>> @@ -292,13 +300,13 @@ user_get_vring_base(struct vhost_device_ctx ctx, > > > >>> * sent and only sent in vhost_vring_stop. > > > >>> * TODO: cleanup the vring, it isn't usable since here. > > > >>> */ > > > >>> - if ((dev->virtqueue[VIRTIO_RXQ]->kickfd) >= 0) { > > > >>> - close(dev->virtqueue[VIRTIO_RXQ]->kickfd); > > > >>> - dev->virtqueue[VIRTIO_RXQ]->kickfd = -1; > > > >>> + if ((dev->virtqueue[state->index]->kickfd + VIRTIO_RXQ) >= 0) { > > > >>> + close(dev->virtqueue[state->index + > > > >>> VIRTIO_RXQ]->kickfd); > > > >>> + dev->virtqueue[state->index + VIRTIO_RXQ]->kickfd = -1; > > > >>> } > > > >> Hi Yuanhan, > > > >> > > > >> Please let me make sure whether below is correct. > > > >> if ((dev->virtqueue[state->index]->kickfd + VIRTIO_RXQ) >= 0) { > > > >> > > > >>> - if ((dev->virtqueue[VIRTIO_TXQ]->kickfd) >= 0) { > > > >>> - close(dev->virtqueue[VIRTIO_TXQ]->kickfd); > > > >>> - dev->virtqueue[VIRTIO_TXQ]->kickfd = -1; > > > >>> + if ((dev->virtqueue[state->index]->kickfd + VIRTIO_TXQ) >= 0) { > > > >>> + close(dev->virtqueue[state->index + > > > >>> VIRTIO_TXQ]->kickfd); > > > >>> + dev->virtqueue[state->index + VIRTIO_TXQ]->kickfd = -1; > > > >> Also, same question here. > > > > Oops, silly typos... Thanks for catching it out! > > > > > > > > Here is an update patch (Thomas, please let me know if you prefer me > > > > to send the whole patchset for you to apply): > > > > > > Hi Yuanhan, > > > > > > I may miss one more issue here. > > > Could you please see below patch I've submitted today? > > > (I may find a similar issue, so I've fixed it also in below patch.) > > > > > > - http://dpdk.org/dev/patchwork/patch/8038/ > > > > > > Thanks, > > > Tetsuya > > > > Looking at that, at least when MQ is enabled, please don't key > > stopping queues off GET_VRING_BASE. > > Yes, that's only a workaround. I guess it has been there for quite a > while, maybe at the time qemu doesn't send RESET_OWNER message.
RESET_OWNER was a bad idea since it basically closes everything. > > There are ENABLE/DISABLE messages for that. > > That's something new, That's part of multiqueue support. If you ignore them, nothing works properly. > though I have plan to use them instead, we still > need to make sure our code work with old qemu, without ENABLE/DISABLE > messages. OK but don't rely on this for new code. > And I will think more while enabling live migration: I should have > more time to address issues like this at that time. > > > Generally guys, don't take whatever QEMU happens to do for > > granted! Look at the protocol spec under doc/specs directory, > > if you are making more assumptions you must document them! > > Indeed. And we will try to address them bit by bit in future. > > --yliu But don't pile up these workarounds meanwhile. I'm very worried. The way you are carrying on, each new QEMU is likely to break your assumptions. -- MST