Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LIBVIRT_URI not being used, hardcoded qemu:///system instead. #3768

Open
7 tasks
zakkg3 opened this issue Sep 24, 2019 · 5 comments
Open
7 tasks

LIBVIRT_URI not being used, hardcoded qemu:///system instead. #3768

zakkg3 opened this issue Sep 24, 2019 · 5 comments

Comments

@zakkg3
Copy link

zakkg3 commented Sep 24, 2019

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

cmd = 'virsh -r -c qemu:///system capabilities'

cmd = "virsh -r -c qemu:///system cpu-models #{a}"

nodeinfo_text = `virsh -c qemu:///system nodeinfo`

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
@zakkg3 zakkg3 changed the title envvar form sourced file LIBVIRT_URI not being used, hardcoded system instead. LIBVIRT_URI not being used, hardcoded system instead. Sep 24, 2019
@zakkg3 zakkg3 changed the title LIBVIRT_URI not being used, hardcoded system instead. LIBVIRT_URI not being used, hardcoded qemu:///system instead. Sep 24, 2019
asteven added a commit to asteven/one that referenced this issue Sep 24, 2019
…env var

Signed-off-by: Steven Armstrong <steven.armstrong@id.ethz.ch>
asteven added a commit to asteven/one that referenced this issue Sep 25, 2019
… scripts

Signed-off-by: Steven Armstrong <steven.armstrong@id.ethz.ch>
@asteven
Copy link
Contributor

asteven commented Sep 26, 2019

FTR: I'm working on a PR. Seems to work so far, but want to test some more before submitting it.

@asteven
Copy link
Contributor

asteven commented Oct 8, 2019

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 or virsh -c $LIBVIRT_URI with just plain virsh

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?

@asteven
Copy link
Contributor

asteven commented Oct 8, 2019

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" $@

@atodorov-storpool
Copy link
Contributor

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?

@asteven
Copy link
Contributor

asteven commented Dec 6, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants