<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 26, 2023, 09:33 Hanna Czenczek <<a href="mailto:hreitz@redhat.com" target="_blank">hreitz@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 25.09.23 22:48, Stefan Hajnoczi wrote:<br>
> On Fri, Sep 15, 2023 at 12:25:25PM +0200, Hanna Czenczek wrote:<br>
>> RFC:<br>
>> <a href="https://lists.nongnu.org/archive/html/qemu-devel/2023-03/msg04263.html" rel="noreferrer noreferrer" target="_blank">https://lists.nongnu.org/archive/html/qemu-devel/2023-03/msg04263.html</a><br>
>><br>
>> v1:<br>
>> <a href="https://lists.nongnu.org/archive/html/qemu-devel/2023-04/msg01575.html" rel="noreferrer noreferrer" target="_blank">https://lists.nongnu.org/archive/html/qemu-devel/2023-04/msg01575.html</a><br>
>><br>
>> v2:<br>
>> <a href="https://lists.nongnu.org/archive/html/qemu-devel/2023-07/msg02604.html" rel="noreferrer noreferrer" target="_blank">https://lists.nongnu.org/archive/html/qemu-devel/2023-07/msg02604.html</a><br>
>><br>
>> Hi,<br>
>><br>
>> I’ve decided not to work on vhost-user SUSPEND/RESUME for now – it is<br>
>> not technically required for virtio-fs migration, which is the actual<br>
>> priority for me now.  While we do want to have SUSPEND/RESUME at some<br>
>> point, the only practically existing reason for it is to be able to<br>
>> implement vhost-level resetting in virtiofsd, but that is not related to<br>
>> migration.<br>
> QEMU sends VHOST_USER_SET_STATUS 0 in vhost_dev_stop(). Are you assuming<br>
> that virtiofs back-ends do not reset the device upon receiving this<br>
> message?<br></blockquote></div></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Absolutely.  vhost_dev_stop() is not in the migration-specific path, but <br>
is called whenever the vCPUs are stopped, which indeed happens to be <br>
part of migration, but is also used in other cases, like QMP stop.  We <br>
have identified that it is wrong that we reset the back-end just because <br>
the vCPUs are stopped (e.g. on migration), but it is what we do right <br>
now when the VM is paused (e.g. through QMP stop).<br>
<br>
Therefore, stateful back-ends cannot implement reset lest stop/cont <br>
breaks the device.  I don’t think anybody really cares whether a <br>
vhost-user back-end actually resets its internal state (if there is any) <br>
when the guest driver asks for a reset on the virtio level, as long as <br>
the guest driver is then able to initialize the device afterwards.</blockquote><div><br></div><div>Some devices send configuration change notifications. For example, virtio-net speed and duplex changes.</div><div><br></div><div>Imagine a network boot ROM runs and the firmware resets the virtio-net device when transferring control to the loaded kernel. Before the kernel driver initializes the device again, the vhost-user-net back-end reports a speed or duplex change and sends a Configuration Change Notification to the guest. The guest receives a spurious interrupt because the vhost-user-net device wasn't actually reset.</div><div><br></div><div>I'm concerned that ignoring reset matters (admittedly in corner cases) and think that stateful device functionality shouldn't be added to the vhost-user protocol without a solution for reset. This patch series changes the vhost-user protocol, which is used by many different devices, not just virtiofs. The solution should work for vhost-user devices of any type and not be based on what we can get away with when running the current QEMU + virtiofsd.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I do <br>
think people care that stop/cont works, so it follows to me that no <br>
stateful back-end will reset its internal state when receiving a <br>
virtio/vhost reset.  If they do, stop/cont breaks, which is a <br>
user-visible bug that needs to be addressed – either properly by <br>
implementing SUSPEND/RESUME in both qemu and the affected back-ends, or <br>
by using a similar work-around to virtiofsd, which is to ignore reset <br>
commands.<br></blockquote></div></div><div dir="auto"><br></div><div>Properly, please.<br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
It’s hard for me to imagine that people don’t care that stop/cont breaks <br>
some vhost-user back-end, but suddenly would start to care that <br>
migration doesn’t work – especially given that first of all someone will <br>
need to manually implement any migration support in that back-end even <br>
with this series, which means that really, the only back-end we are <br>
talking about here is our virtiofsd.  To this day I’m not even aware of <br>
any other back-end that has internal state.<br></blockquote><div><br></div><div>Another one I can think of is vhost-user-gpu.</div><div><br></div><div>Why did you give up on implementing SUSPEND/RESUME?<br></div><div><br></div><div>Stefan<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hanna<br>
<br>
<br>
</blockquote></div></div></div>
</div></div></div>