[pve-devel] [PATCH qemu 1/2] vm-network-scripts: move scripts to /usr/libexec

[Fabian Grünbichler]

On November 18, 2024 2:15 pm, Maximiliano Sandoval wrote:
> 
> Fiona Ebner <f.ebner at proxmox.com> writes:
> 
>> It's the "qemu-server" repository, not "qemu".
>>
>> Am 09.10.24 um 14:55 schrieb Maximiliano Sandoval:
>>> Moves the network scripts from /var/lib/qemu-server into
>>> /usr/libexec/qemu-server.
>>>
>>> /usr/libexec is described as binaries run by programs which are not
>>> intended to be directly executed by the user on [FHS 4.7]. On the other
>>> hand /var/lib corresponds to variable state information, which does not
>>> fit the use case here, see [FHS 5.8].
>>>
>>> [FHS 4.7]: https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s07.html
>>> [FHS 5.8]: https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch05s08.html
>>>
>>
>> Can this cause issues for a brief time during upgrade/unpacking, i.e.
>>
>> A) if the old QemuServer.pm is still loaded while the scripts get
>> removed from its old location (or to be more precise, a QEMU process
>> with the old parameter is started at that time)
> 
> I tested this at the time and it was not an issue.

this can easily happen - packages are unpacked first (including removal
of files no longer shipped by the new version!) and then configured (the
latter triggers the reload of the services).

> 
>> B) if the new QemuServer.pm gets loaded before the scripts are extracted
>> to their new location (or to be more precise, a QEMU process with the
>> new parameter is started at that time)
> 
> This might be an issue, but no idea how to test this, it sounds
> extremely racy. It might make more sense to hold this for pve 9?

this can also happen if something loads QemuServer.pm while the
unpacking is happening (this is way more unlikely then A) above though,
as the race window is much smaller).

>> Is there something preventing those from happening? Otherwise, we might
>> need to ship the scripts to both locations until the next major release
>> and drop them from the old location only then.

I think we do need to ship the file in both places before 9.0 and switch
over to using the new location (i.e., almost what this patch does), and
then we can drop the old location with 9.0 since we know that no
reference to the old location should still be in use by then..

the only other alternative I see is an unconditional hardlink in
preinst, but that is quite messy and I am not sure if it's possible to
get this right for all the corner cases, and it would also mean that
that link is not known to dpkg/doesn't belong to the package, so..
shipping two copies for a transitional period seems better ;)

shipping a symlink (from old to new) would reduce the race window to
just between unpacking the symlink and its target in the new location
(depending on ordering there might even not be a race window at all?),
but it is also not that much better than just shipping the scripts
twice..

>>
>>> Signed-off-by: Maximiliano Sandoval <m.sandoval at proxmox.com>
>>> ---
>>>  PVE/QemuServer.pm                     |  4 ++--
>>>  vm-network-scripts/Makefile           | 10 +++++-----
>>>  vm-network-scripts/pve-bridge-hotplug |  2 +-
>>>  3 files changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>> index b26da505..88100638 100644
>>> --- a/PVE/QemuServer.pm
>>> +++ b/PVE/QemuServer.pm
>>> @@ -1759,8 +1759,8 @@ sub print_netdev_full {
>>>      my $script = $hotplug ? "pve-bridge-hotplug" : "pve-bridge";
>>>
>>>      if ($net->{bridge}) {
>>> -	$netdev = "type=tap,id=$netid,ifname=${ifname},script=/var/lib/qemu-server/$script"
>>> -	    .",downscript=/var/lib/qemu-server/pve-bridgedown$vhostparam";
>>> +	$netdev = "type=tap,id=$netid,ifname=${ifname},script=/usr/libexec/qemu-server/$script"
>>> +	    .",downscript=/usr/libexec/qemu-server/pve-bridgedown$vhostparam";

e.g., this part here is called by any start task, which runs in task
worker (own process), so could trigger both A) (task started before
QemuServer.pm has been unpacked, but execution of script by forked Qemu
happens after the old script has already vanished) and B) (unpack of
QemuServer.pm happens before unpack of new script, task gets started and
manages to get to the point of Qemu executing the script before it has
been unpacked)

>>>      } else {
>>>          $netdev = "type=user,id=$netid,hostname=$vmname";
>>>      }
>>> diff --git a/vm-network-scripts/Makefile b/vm-network-scripts/Makefile
>>> index 5ba537d0..dc2c85ff 100644
>>> --- a/vm-network-scripts/Makefile
>>> +++ b/vm-network-scripts/Makefile
>>> @@ -1,12 +1,12 @@
>>>  DESTDIR=
>>> -VARLIBDIR=$(DESTDIR)/var/lib/qemu-server
>>> +LIBEXECDIR=$(DESTDIR)/usr/libexec/qemu-server
>>>
>>>  .PHONY: install
>>>  install: pve-bridge pve-bridge-hotplug pve-bridgedown
>>> -	install -d ${VARLIBDIR}
>>> -	install -m 0755 pve-bridge ${VARLIBDIR}/pve-bridge
>>> -	install -m 0755 pve-bridge-hotplug ${VARLIBDIR}/pve-bridge-hotplug
>>> -	install -m 0755 pve-bridgedown ${VARLIBDIR}/pve-bridgedown
>>> +	install -d ${LIBEXECDIR}
>>> +	install -m 0755 pve-bridge ${LIBEXECDIR}/pve-bridge
>>> +	install -m 0755 pve-bridge-hotplug ${LIBEXECDIR}/pve-bridge-hotplug
>>> +	install -m 0755 pve-bridgedown ${LIBEXECDIR}/pve-bridgedown
>>>
>>>  .PHONY: clean
>>>  clean:
>>> diff --git a/vm-network-scripts/pve-bridge-hotplug b/vm-network-scripts/pve-bridge-hotplug
>>> index f36ed408..3ae01ea5 100755
>>> --- a/vm-network-scripts/pve-bridge-hotplug
>>> +++ b/vm-network-scripts/pve-bridge-hotplug
>>> @@ -1,3 +1,3 @@
>>>  #!/bin/sh
>>>
>>> -exec /var/lib/qemu-server/pve-bridge --hotplug "$@"
>>> +exec /usr/libexec/qemu-server/pve-bridge --hotplug "$@"
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>