Re: [PATCH 5/5] qemu: make passt+vhostuser reconnect behave identically to passt+user
On 4/10/25 2:55 AM, Jiri Denemark wrote:
On Thu, Apr 10, 2025 at 02:58:30 -0400, Laine Stump wrote:
> When "original passt" support was added, we decided that we always
> wanted to reconnect (i.e. restart the passt process) if it was somehow
> terminated. Generic vhost-user only turns on reconnect if specified in
> the config, but there is no reason to require this if the other end of
> the vhost-user socket is a passt process - we know what has happened
> and what we want to do; no reason to make the default configuration
> "do the *wrong* thing".
>
> Resolves: https://issues.redhat.com/browse/RHEL-80169
>
> Signed-off-by: Laine Stump <laine(a)redhat.com>
> ---
> src/qemu/qemu_passt.c | 6 ++++++
> .../qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args | 6 +++---
> .../schema-reorder-domain-subelements.x86_64-latest.args | 2 +-
> 3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
> index bc495eca1e..018630a5de 100644
> --- a/src/qemu/qemu_passt.c
> +++ b/src/qemu/qemu_passt.c
> @@ -182,6 +182,12 @@ qemuPasstPrepareVhostUser(virDomainObj *vm,
> */
> g_free(net->data.vhostuser->data.nix.path);
> net->data.vhostuser->data.nix.path = qemuPasstCreateSocketPath(vm, net);
> +
> + /* reconnect is always enabled, with timeout always at 5 seconds, when
> + * using passt
> + */
> + net->data.vhostuser->data.nix.reconnect.enabled = VIR_TRISTATE_BOOL_YES;
> + net->data.vhostuser->data.nix.reconnect.timeout = 5;
Hmm, shouldn't this be reflected in the XML?
Nope. Actually I thought about this when giving the commit messages a
final look before I sent last night/morning - just as with the socket
path for the associated character device is hardcoded when vhostuser is
being used for passt, the reconnect timeout for the character device is
hardcoded, so it's not just a "default", it is the only possible value.
I'll reflect that in the commit log.
The entire net->data.vhostuser part of the object isn't configurable
when vhostuser is being used for passt, the corresponding attributes in
<source> aren't allowed in the input XML, and won't be output when the
object is formatted.
That is, I would expect a
post-parse function to set these defaults.
And the current code
overrides user specified settings (either disabling or setting a
different timeout), which doesn't is not right.
Except that the user can't set it :-)
Actually, looking at our
documentation the <reconnect> it seems we don't even support setting
reconnect for vhost-user with passt backend.
Correct. This is the same behavior as for non-vhostuser passt, where we
decided that there was no downside to having reconnect always enabled
and no reasonable way to explain how or why a user should modify the
timeout, so made it always on and always 5 second timeout.
If that's the case, the
code is fine as is except for the following nit, which I'd like to see
addressed anyway...
And I suggest relacing the 5 seconds timeout with a macro that would
also be used in qemuPasstAddNetProps.
Sure, I can do that.
Thanks for the quick review!