Hi, On Wed, Feb 19, 2025 at 10:48:33AM +0100, Peter Krempa wrote: > 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. Alright, I don't plan to fight for this. Just to be 100% clear, we have something that runs users scripts which receives xml input over stdin and prints xml output over stdout already. I just thought we could as well use something implemented in libvirt for it instead of redoing it in the mgmt layer. > > 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". The "might" and "let's see" indeed begs for investigation as I'm not sure either but if from your experience you think it isn't a good practice, alright. I'll be careful in proposing anything related to it. Thanks for the quick replies, Victor
Attachment:
signature.asc
Description: PGP signature
