config: Set Q35 as the default machine type #336
Conversation
| return h.Image | ||
| } | ||
|
|
||
| func (h hypervisor) machineType() string { |
There was a problem hiding this comment.
This is missing a test in config_test.go.
There was a problem hiding this comment.
Ah yes you're right.
| # XXX: Warning: this file is auto-generated from file "@CONFIG_IN@". | ||
|
|
||
| [hypervisor.qemu-lite] | ||
| [hypervisor.q35] |
There was a problem hiding this comment.
We really need to log the machine type in the cc-env output (currently it only shows the path to the hypervisor). Since that is an oversight, I don't think that is necessary for this PR (but it would be nice if you want to add it :). If not, let me know so I can raise an issue.
There was a problem hiding this comment.
I can try to add this.
BTW, I have raised clearcontainers/tests#198 because semaphore is failing because q35 is not installed. I was thinking about installing the q35 version with a different name since we will get current tests failing otherwise (our version of qemu does not support q35 and the q35 version does not support pc-lite). And from this PR, I would specify this different name in the Makefile. What do you think ?
There was a problem hiding this comment.
Thanks! I think you should probably bump formatVersion (https://github.com/clearcontainers/runtime/blob/master/cc-env.go#L33) as adding the machine type is an output format change.
There was a problem hiding this comment.
I think that right now this is hitting semaphore, right? If so, a PR should be added on tests to install q35 qemu instead of the currently qemu-lite.
Although I am not sure if we are providing it as a package in OBS.
There was a problem hiding this comment.
@sboeuf indeed, I don't think we've settled on a name for the q35 qemu - and we should probably call it something else.
I was a little unsure with it living in the 'pc-lite' branch in the repo even.
I can see it is a 'progression of pc-lite', but having the same name will make some of our integrations hard. /cc @anthonyzxu and @mcastelino and @egernst for their thoughts here.
(to note - I renamed mine locally when I was hand testing ;-) )
There was a problem hiding this comment.
I was using this other repo because that was the one pointed by @mcastelino and @egernst to me. If the one you mention is doing the job (support for q35), I think this is great and I can use it.
There was a problem hiding this comment.
Nevermind, I was confused by an email from Anthony but that's fine, our qemu repo works fine.
There was a problem hiding this comment.
My two rusty cents: ideally it would be nice to support both: current upstream QEMU and future version with new shiny features. Runtime then could dynamically detect the QEMU version and enable supported features when it launches a new VM.
I understand that maintaining cost might slightly increase, but this approach could enable CC on platforms with outdated QEMU.
Just an opinion.
There was a problem hiding this comment.
@dvoytik - I agree -- we are looking into adding this configurability.
There was a problem hiding this comment.
Yes I'll work on it today.
|
@sboeuf - note that you'll probably want to update |
This patch implements config.go so that we can really switch between hypervisor machine types such as pc-lite and q35 for qemu hypervisor. It also sets q35 as the default machine type because this is the most upstream and we want this one by default for our runtime. Fixes #329 Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
The purpose of this commit is to add some useful info related to the hypervisor now that we have the machine type available. EnvInfo structure has been modified so that it expects HypervisorInfo structure instead of the generic PathInfo regarding the information about the hypervisor. Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
sboeuf
force-pushed
the
sboeuf/default_q35
branch
from
893bbe8 to
8bdc743
Compare
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
sboeuf
force-pushed
the
sboeuf/default_q35
branch
from
cd4a314 to
162fd2a
Compare
|
Closing this one because it has been partially replaced by #354 and because we don't want to make q35 as the default machine type now. |
…le-name config: Show which config file loaded
This patch implements config.go so that we can really switch between
hypervisor machine types such as pc-lite and q35 for qemu hypervisor.
It also sets q35 as the default machine type because this is the most
upstream and we want this one by default for our runtime.
Fixes #329