scripts: Add script to generate hypervisor configure options. #45
Conversation
|
lgtm |
jodh-intel
force-pushed
the
add-hypervisor-build-options-script
branch
from
9992d53 to
cf12ad6
Compare
scripts/README.md
Outdated
|
|
||
| ## `configure-hypervisor.sh` | ||
|
|
||
| This script generates the official set of hypervisor build configuration |
There was a problem hiding this comment.
Hi. given the script generates QEMU, and only QEMU, arguments right now, maybe we could be more explicit about that in the text.
For instance, I ran (for a laugh...) ./configure-hypervisor.sh graham, and was surprised it ran and generated output :-)
Given we are not filtering on the name, and the output is very QEMU specific, I reckon we can be more explicit. you think?
There was a problem hiding this comment.
When you say, "in the text", do you mean the script header or the README? The former mentions qemu and the latter gives and example. I'm open to suggestions here ;)
There was a problem hiding this comment.
Again, I purposely didn't validate the input, but I can add a check to ensure it is qemu-lite if you like. But we should probably permit qemu too? Also, we'll need to update it for qemu-cc once #34 lands.
There was a problem hiding this comment.
I was thinking in the README - currently we say 'hypervisor', in a rather neutral way, implying to me that it might either understand which hypervisor you are asking for, or support multiple hypervisors. Just changing that to QEMU would work for me, just to be explicit that this is just supporting QEMU and not filtering on the arguments right now.
I'm fine on not filtering right now, the script does exactly what we need it to do for our integrations, just wanted it a bit more explicit that it will spit out QEMU settings, always, right now.
| qemu_options+=(--disable-libiscsi) | ||
| qemu_options+=(--disable-libnfs) | ||
| qemu_options+=(--disable-libssh2) | ||
| qemu_options+=(--disable-rbd) |
There was a problem hiding this comment.
/me will remember that for if I pick up that ceph work again ;-)
There was a problem hiding this comment.
Minor doc nit, does not have to block.
Lovely work on documenting the arguments @jodh-intel
@dvoytik - don't know if you or somebody else would like to review those settings now we have them nicely laid out :-)
|
lgtm |
|
@anthonyzxu can you take a look? |
To avoid a proliferation of scripts and config files from specifying different sets of hypervisor configuration options, this script must be run to generate the correct set of options. Each option group has been documented explaining why it has been specified. The script currently only supports qemu-based hypervisors. Fixes clearcontainers#44. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
jodh-intel
force-pushed
the
add-hypervisor-build-options-script
branch
from
cf12ad6 to
0fc437f
Compare
|
looks good to me |
Add the version of config and patches we are using in a package. Kernel version before: 4.14.22-128 Now: 4.14.22.1-128 Fixes: clearcontainers#45 Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
To avoid a proliferation of scripts and config files from specifying
different sets of hypervisor configuration options, this script must
be run to generate the correct set of options.
Each option group has been documented explaining why it has been
specified.
The script currently only supports qemu-based hypervisors.
Fixes #44.
Signed-off-by: James O. D. Hunt james.o.hunt@intel.com