one
one copied to clipboard
LIBVIRT_URI not being used, hardcoded `qemu:///system` instead.
Libvirt uri seems to be hardcoded and not being readed from the sourced env var at /var/lib/one/remotes/etc/vmm/kvm/kvmrc :
export LIBVIRT_URI=qemu+tcp://localhost/system
this is not working then. http://docs.opennebula.org/5.8/deployment/open_cloud_host_setup/kvm_driver.html#opennebula-configuration
https://github.com/OpenNebula/one/blob/def9661b4aa7c708d51679eb051ce8bebce09d80/src/im_mad/remotes/kvm-probes.d/machines-models.rb#L33
https://github.com/OpenNebula/one/blob/def9661b4aa7c708d51679eb051ce8bebce09d80/src/im_mad/remotes/kvm-probes.d/machines-models.rb#L92
https://github.com/OpenNebula/one/blob/441cf1f7f9e726cb5f200d661d50e92a4042fff7/src/im_mad/remotes/kvm-probes.d/kvm.rb#L32
Progress Status
- [ ] Branch created
- [ ] Code committed to development branch
- [ ] Testing - QA
- [ ] Documentation
- [ ] Release notes - resolved issues, compatibility, known issues
- [ ] Code committed to upstream release/hotfix branches
- [ ] Documentation committed to upstream release/hotfix branches
FTR: I'm working on a PR. Seems to work so far, but want to test some more before submitting it.
So a bunch of problems here. And questions to upstream.
There's roughly 90 places where the virsh
executable is used/called throughout the ONE codebase.
Basically from within vnm_mad, tm_mad, im_mad, vmm_mad scripts. Some of the scripts are shell, some are ruby.
Handling all of these cases by replacing virsh -c qemu:///system
with something like virsh -c $LIBVIRT_URI
seems problematic. Especially given that the source of LIBVIRT_URI is src/vmm_mad/remotes/kvm/kvmrc. Sourcing, reading, including that from all the consumers in vnm_mad, tm_mad, im_mad, vmm_mad does not seem to be a good solution.
I instead propose the following.
- prepend a custom bin directory to the PATH (e.g. $SCRIPTS_REMOTE_DIR/bin (aka /var/tmp/one/bin)) for all script executions
- introduce a script in $SCRIPTS_REMOTE_DIR/bin/virsh that knows how to properly connect to libvirt
- replace all occurrences of
virsh -c qemu:///system
orvirsh -c $LIBVIRT_URI
with just plainvirsh
This way we can handle all the implementation details in once place. Any new scripts added in the future don't have to worry about LIBVIRT_URI and such things.
Given that LIBVIRT_URI is used in many other places then just vmm_mad I propose to define the variable somewhere else then its current location (src/vmm_mad/remotes/kvm/kvmrc). If you agree, where would such a 'global' variable go? And how could it be exposed to the $SCRIPTS_REMOTE_DIR/bin/virsh wrapper script?
Is there some central place where all the remote scripts are called from?
Example script for $SCRIPTS_REMOTE_DIR/bin/virsh
#!/bin/sh
remove_from_path() {
# Remove the given directory from the PATH.
PATH="${PATH//":$1:"/":"}" # delete any instances in the middle
PATH="${PATH/#"$1:"/}" # delete any instance at the beginning
PATH="${PATH/%":$1"/}" # delete any instance at the end
export PATH
}
mydir=$(cd "${0%/*}"; pwd -P)
# Remove mydir from path to unhide the real virsh executable.
remove_from_path "$mydir"
# TODO: get LIBVIRT_URI from some where.
exec virsh --connect "$LIBVIRT_URI" $@
IMO it is better to patch the 4 places where "qemu://system" is hard-coded instead of hijacking the virsh binary in a custom path. Several reasons:
- If there are additional scripts they will need to be updated too this means all known and unknown addons
- It will become harder for the general users to debug issues because the virsh is hijack-ed but the user will try with default paths and experience different results.
The files in question are:
src/vnm_mad/remotes/lib/command.rb
src/im_mad/remotes/kvm-probes.d/kvm.rb
src/im_mad/remotes/kvm-probes.d/machines-models.rb
src/im_mad/remotes/lxd-probes.d/lxd.rb
On a side note in why not adapting the ruby code from https://github.com/OpenNebula/one/blob/master/src/vmm_mad/remotes/kvm/poll in the other places?
Then call the script one-virsh or whatever. Why would you want to depend on a env variable that is defined in some shell script/rc file in one mad from all others. And read/parse that variable from every script/shell/ruby/executable again and again.
Anyway. Upstream does not seem to be interested. I'm not working on this. Feel free to PR with your preferred solution.