From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id EBAAE779E0 for ; Wed, 28 Apr 2021 15:44:15 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E1B20F63A for ; Wed, 28 Apr 2021 15:44:15 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 8E867F629 for ; Wed, 28 Apr 2021 15:44:14 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6884146490 for ; Wed, 28 Apr 2021 15:44:14 +0200 (CEST) Message-ID: <1e2ebd8b-cfff-c377-6739-a4b9e6164140@proxmox.com> Date: Wed, 28 Apr 2021 15:44:13 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:89.0) Gecko/20100101 Thunderbird/89.0 Content-Language: en-US To: Oguz Bektas , Proxmox VE development discussion References: <20210428111608.694745-1-o.bektas@proxmox.com> <20210428114221.GA11014@gaia.proxmox.com> <5d8c65ce-68fd-85f4-2770-594390463536@proxmox.com> <20210428122024.GA806626@gaia.proxmox.com> From: Thomas Lamprecht In-Reply-To: <20210428122024.GA806626@gaia.proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.006 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH docs] pveproxy: improve LISTEN_IP doc X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Apr 2021 13:44:16 -0000 On 28.04.21 14:20, Oguz Bektas wrote: > On Wed, Apr 28, 2021 at 01:51:51PM +0200, Thomas Lamprecht wrote: >> On 28.04.21 13:42, Oguz Bektas wrote: >>>>> + >>>>> + LISTEN_IP="fe80::d8ee:34ff:fe37:4579%vmbr0" >>>>> + >>>>> +After the change you have to restart `pveproxy` for it to take effect: >>>> >>>> I'd specifically state that a reload is not enough and then add a small warning that >>>> a restart can stop some existing workers (not all, but e.g., shell connection is stopped >>>> and reconnected which may loose info on CTs without a screen/tmux instance running). >>>> Also, there's a short time window where no new connections are accepted IIRC (albeit >>>> I was the one fixing that for reload it's been to long since then, so not sure anymore) >>> >>> i think the phrasing "you have to restart" already emphasizes this, >>> adding too many warnings or notes would just confuse users in my >>> opinion. >> >> No, it's clear that something needs to be restarted, but "restart" is a general >> overused term which can mean lots of things (even reboot for some). >> >>> >>> though i don't see any harm in making the **restart** bold in that >>> sentence and adding that small warning about possible connection drop. >> >> as said, restart is often used for the general semantic thing, be it reload or restart, >> so this is really not clear. > > why is it not clear? there's not a single instance of 'systemctl reload' > command in the documentation, and in instances where a service restart > is required, we specify 'systemctl restart' without any mention, e.g., > in pvenode.adoc when setting up certificates) > > so i don't see why users would imagine to 'reload' the service instead > of 'restart' when it's clearly written in bold, and the command is > right beneath the explanation... > >> The gui also only triggers a reload, IIRC (pls. check) >> and thus "restarting" (it's named that way there) from there would not help. > > yes, on the button it says "restart" but the tasklog says "reload". > so to me that sounds like a mislabeled button with wrong/lax use > of "restart" instead of the correct "reload". I *always* use try-reload-or-restart, as I want to avoid service disruption, as most people actually running relevant systems do... What I surely do not is reading all the docs, comparing how often reload vs. restart is used and then try to read into that and conclude something from that... I *never* saw anybody complaining about an additional note of some side-effects that may come unexpected, but I saw quite some situations where there was no such note and user run into issues, I do not suggest such changes just for the hell of it... > >> >> You just need to write it in such a way that it is not confusing, then it is not >> a problem. > > there's really nothing confusing IMO (besides the button). but if you > insist i will send another patch with the changes you recommended. You said you would write it in a confusing way when adding an explicit note, so I suggest doing it in a not confusing manner ;) > > some possible alternative phrasing i'd suggest: > > ===== > After the change you have to **restart** `pveproxy` and `spiceproxy` for > it to take effect (**reload** or restart from GUI does not suffice): > systemctl restart pveproxy spiceproxy > ===== > > or much simpler and less confusing: > ===== > Run the following command to restart the proxy servers and apply the change: > systemctl restart pveproxy spiceproxy > ===== > > with a note about the connection drop during restart. > I applied the fixes, the notes and another small grammar style fix for the comma after "Similarly," myself, seemed to be the faster way to resolve this nicely..