On Wed, Feb 19, 2025 at 09:48:30 +0100, Victor Toso wrote: > On Tue, Feb 18, 2025 at 07:35:33PM +0100, Jiri Denemark wrote: > > On Tue, Feb 18, 2025 at 19:21:56 +0100, Victor Toso wrote: > > > Hi, > > > > > > On Tue, Feb 18, 2025 at 07:11:45PM +0100, Jiri Denemark wrote: > > > > On Tue, Feb 18, 2025 at 18:41:46 +0100, Victor Toso wrote: > > > > > Hi, > > > > > > > > > > I'm particularly interested in the PoC of KubeVirt to allow > > > > > custom changes in libvirt domain xml in a more straightforward > > > > > manner than they have Today [0]. > > > > > > > > > > When I was looking into the libvirt hooks, I was a bit excited > > > > > when I read: > > > > > > > > > > > you can also place several hook scripts in the directory > > > > > > /etc/libvirt/hooks/qemu.d/. They are executed in alphabetical > > > > > > order after main script. In this case each script also acts as > > > > > > filter and can modify the domain XML and print it out on its > > > > > > standard output. This script output is passed to standard > > > > > > input next script in order. Empty output from any script is > > > > > > also identical to copying the input XML without changing it. > > > > > > In case any script returns failure common process will be > > > > > > aborted, but all scripts from the directory will are executed. > > > > > > > > > > But that's not the case for every situation. When the domain is > > > > > being defined the scripts are run but the output is ignored [1] > > > > > > > > > > Is there a reason for that? > > > > > > > > Libvirt only checks the output of pre-migration hook so that you can > > > > change the XML on incoming migration on the destination host rather than > > > > providing the updated XML to the migrate API. > > > > > > > > The above paragraph talks about domain XML passing from one script to > > > > the following one when there's more scripts defined in qemu.d/ > > > > > > > > Jirka > > > > > > Indeed, I understood it after failing and re-reading the docs. My > > > question is more related to the possibility of using it in other > > > scenarios. Would patches for that be welcomed? > > > > It depends. Do you have a specific use case which would benefit > > from the change? > > The use case is to allow users be able to test and apply changes > to domain xml using libvirt hook system instead of going through > mgmt application. I don't think the libvirt hooks are the correct approach for this, especially those for filtering XML. Additionally IMO libvirt hooks should check ABI stability of the config thus disallow many of the modifications you would envision. > KubeVirt itself only apply changes for what it is part of its > supported features but users of KubeVirt might need to tweak > those settings. I dummy example is enabling clipboard for VNC [0] > but there are a lot of tweaks for specific hardware pass-through > or tests that developers try too. Another interesting example is > changing the emulator to run a script so we can actually use > strace/gdb for debugability [1] Kubevirt itself can present the XML it created to an external process to modify it; as in do the hook at kubevirt level if you want to do this or provide internal config options to modify the XML. An example for this is the qemu XML namespace we have, which allows you to add extra commandline parameters to qemu via <qemu:commandline>. This is something that we do not provide support for (as in it works; but if you break stuff using it it's your problem) but is inside libvirt's config instead of instructing users to create wrapper programs around qemu. > [0] https://github.com/kubevirt/kubevirt/issues/10306#issuecomment-1881549873 > [1] https://kubevirt.io/user-guide/debug_virt_stack/launch-qemu-strace/ > > > In most cases hooks are run when the XML has already been > > processed and used for preparing the domain so it's too late > > for any XML changes. The only place where it might > > theoretically be early enough for XML changes (I haven't really > > checked the code to see if this is true) would be "prepare" > > hook. > > One issue that has happened in KubeVirt with current hook feature > there is KubeVirt reserving network interfaces but in order to do > so it end up calling the hook mechanism multiple times, see the > commit log [2] > > [2] https://github.com/kubevirt/kubevirt/commit/637be26ef415dbe77a003e3 > > This lead to a situation where the user needs to consider their > hook script in KubeVirt as something that might be called > multiple times. > > So, I too wonder if "prepare" is enough but the end result I > would expect is that the user can try locally their hook and then > just push that into a ConfigMap and see it applied in the same > way with their k8s VM. > > > But why would anyone want to change the definition at this > > point rather providing the correct XML in the first place? > > I hope I was able to reply to this. > > > It's not like migration when one host does not know the > > situation on another host. With the prepare hook both the hook > > and the API to create a domain are called on a single host. > > > > Jirka > > Yeah, using hooks for migration is the follow-up thing I plan to > look as we have custom code in KubeVirt that might be better off > in hooks, let's see. This certainly requires clarification. IMO there isn't many cases when "code would be better of in hook".
