On Thu, Apr 10, 2025 at 11:45:15 -0400, Laine Stump wrote:
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;
...
> 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.
Great.
> 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!
With this small change
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>