On Mon, Mar 28, 2016 at 02:45:18PM -0400, John Ferlan wrote:
>
>
> On 03/21/2016 07:55 AM, Bjoern Walk wrote:
> > Since kernel version 4.5, s390 has the 'sie' flag to declare its
> > hardware virtualization support. Let's make virt-host-validate aware so
> > this check is passed correctly.
> >
> > Signed-off-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> > ---
> > tools/virt-host-validate-qemu.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
>
> I've never seen the output of /proc/cpuinfo on an S390, but looking at
> the source code I found it does seem to print with the "features" for a
> cpuinfo on an s390 machine (as opposed the "flags" for a vmx/svm machine).
>
> The virHostValidateHasCPUFlag doesn't really distinguish "flags" or
> "features" field being specified as a label (although I imagine that
> could have internationalization impact if we tried to limit that more).
>
> Anyway, before I push this - I wanted to see if there were any "other"
> thoughts regarding letting this be a bit more generic. My one concern is
> some field name some day has "sie" added (e.g. "easier" or "transient"
> would match).
>
> One change that may help with that would be to check "sie " instead of
> "sie" (similarly "svm" could be "svm " and "vmx" could be "vmx "). The
> source code prints the output as "..."%s ", int_hwcap_str[i]..."
Yes this is a good point, but also its a pre-existing problem with the
virHostValidateHasCPUFlag method, so not a reason to block this patch.
It can be fixed separately as desired.
> > diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c
> > index a9f6c1e..01fb24b 100644
> > --- a/tools/virt-host-validate-qemu.c
> > +++ b/tools/virt-host-validate-qemu.c
> > @@ -31,7 +31,8 @@ int virHostValidateQEMU(void)
> >
> > virHostMsgCheck("QEMU", "%s", ("for hardware virtualization"));
> > if (virHostValidateHasCPUFlag("svm") ||
> > - virHostValidateHasCPUFlag("vmx")) {
> > + virHostValidateHasCPUFlag("vmx") ||
> > + virHostValidateHasCPUFlag("sie")) {
> > virHostMsgPass();
> > if (virHostValidateDeviceExists("QEMU", "/dev/kvm",
> > VIR_HOST_VALIDATE_FAIL,
> >
What we should do here though is check the host architecture before
checking CPU flags.
eg you'll want to make this code do something like this
bool hasHWVirt = false;
switch (virArchFromHost()) {
case VIR_ARCH_I686:
case VIR_ARCH_X86_64:
if (virHostValidateHasCPUFlag("svm") ||
virHostValidateHasCPUFlag("vmx"))
hasHWVirt = true;
break;
case VIR_ARCH_S390:
case VIR_ARCH_S390X:
if (virHostValidateHasCPUFlag("sie"))
hasHWVirt = true;
}
if (hasHWVirt) {
....do the /dev/kvm check...
}
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list