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

vmimage: allow the qemu-img binary to be overriden #3374

Merged

Conversation

clebergnu
Copy link
Contributor

Avocado currently searches for the qemu-img binary on the system,
picking the first one available (respecting the PATH environment
variable and looking at common places).

In some specific circurnstances, such as when testing QEMU itself,
it's desirable to use a matching qemu-img binary. Let's allow this
configuration to be done at the module level, to be less intrusive on
the API, so that the vast majority of vmimage users don't have to
care about it.

Signed-off-by: Cleber Rosa crosa@redhat.com

Avocado currently searches for the qemu-img binary on the system,
picking the first one available (respecting the PATH environment
variable and looking at common places).

In some specific circurnstances, such as when testing QEMU itself,
it's desirable to use a matching qemu-img binary.  Let's allow this
configuration to be done at the module level, to be less intrusive on
the API, so that the vast majority of vmimage users don't have to
care about it.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
@packit-as-a-service
Copy link

Congratulations! The build has finished successfully. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/avocado-framework-avocado-3374
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@codecov
Copy link

codecov bot commented Nov 15, 2019

Codecov Report

Merging #3374 into master will decrease coverage by <.01%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3374      +/-   ##
==========================================
- Coverage   69.86%   69.85%   -0.01%     
==========================================
  Files         135      135              
  Lines       16536    16539       +3     
==========================================
+ Hits        11553    11554       +1     
- Misses       4983     4985       +2
Impacted Files Coverage Δ
avocado/utils/vmimage.py 70.32% <25%> (-0.46%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f017b27...5a19093. Read the comment docs.

#: The "qemu-img" binary used when creating the snapshot images. If
#: set to None (the default), it will attempt to find a suitable binary
#: with :func:`avocado.utils.path.find_command`, which uses the the
#: system's PATH environment variable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clebergnu, Out of curiosity: Why #: on comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable gets automatically documented with that comment style.

#: set to None (the default), it will attempt to find a suitable binary
#: with :func:`avocado.utils.path.find_command`, which uses the the
#: system's PATH environment variable
QEMU_IMG = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking if I understood correctly... If I as an user of the vmimage module want to override qemu-img path then I do this:

import avocado.utils.vmimage
vmimage.QEMU_IMG = '/path/to/qemu-img'

If so then should it be documented because it may not be obvious to some users (like me:)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's why it's commented with #:, it will be documented automatically with that long comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Got it.

@@ -431,7 +438,10 @@ def _take_snapshot(self):
"""
Takes a snapshot from the current image.
"""
qemu_img = utils_path.find_command('qemu-img')
if QEMU_IMG is None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this block. looks good to me!

@clebergnu clebergnu merged commit 5a19093 into avocado-framework:master Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants