Re: [pve-devel] [PATCH pve-nvidia-vgpu-helper v2 4/4] debian: add and install nvidia-vgpu systemd template unit file

From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>,
	Hannes Duerr <h.duerr@proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-nvidia-vgpu-helper v2 4/4] debian: add and install nvidia-vgpu systemd template unit file
Date: Wed, 22 Jan 2025 14:14:12 +0100	[thread overview]
Message-ID: <fa1425a5-c8c8-4bed-814f-25169759e5e4@proxmox.com> (raw)
In-Reply-To: <01858afc-d11e-4890-9fd2-30643c607e82@proxmox.com>

Am 21.01.25 um 11:36 schrieb Dominik Csapak:
> On 1/21/25 11:04, Hannes Duerr wrote:
>> diff --git a/debian/nvidia-vgpud@.service b/debian/nvidia-vgpud@.service
>> new file mode 100644
>> index 0000000..b3c1220
>> --- /dev/null
>> +++ b/debian/nvidia-vgpud@.service
>
> mhmm not sure if it's so good to reuse the exiting name of the nvidia service
> for this. also not sure how systemd likes it if there is a service 'foo.service'
> and a template 'foo@.service' (though my guess would be that it's not that big of an issue)
> 
> i'd like to seee a distinct name for this (maybe even with pve prefix) like:
> 
> pve-nvidia-sriov@.service
> 
> Then it's clear where it comes from and what it's for (vgpud is an nvidia daemon
> that does not really has anything to do with what this does)
> Or do you have any objections to that?
> 
> also, would it make sense to add a
> 
> ---
> ConditionPathExists=/usr/lib/nvidia/sriov-manage
> ---
> 
> too?
> 
> otherwise users that enable it accidentally or too early run into an ugly error
> (though that can be desired too)

An option might be to replace the ExecStartPre "sleep 5" with an actual pre-start
command provided by the helper that ensures that everything is setup correctly
and prints telling errors otherwise. That can then also handle any sleep that is
required, maybe in a more time-efficient way – like loop and check for the required
condition with shorter sleeps in-between, the wiki also mentions that this might
need to be adapted depending on the setup/HW.

Anyway, it would be good to get an actual commit message, it can be short but
could describe what this does and why a sleep is required in the first place,
also mentioning that this was taken over from the wiki [0] could be nice to
know.

https://pve.proxmox.com/mediawiki/index.php?title=NVIDIA_vGPU_on_Proxmox_VE&oldid=12125#Enabling_SR-IOV

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel