On Wed, Jul 08, 2015 at 12:54:11PM -0400, Cole Robinson wrote: > On 07/08/2015 05:33 AM, Pavel Hrdina wrote: > > Instead of hard-coding that ACPI and APIC are enabled by default, detect > > their presence from libvirt capabilities and use it. > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1215692 > > > > Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx> > > --- > > Comments inline > > > diff --git a/virtinst/capabilities.py b/virtinst/capabilities.py > > index a26b4a2..553a430 100644 > > --- a/virtinst/capabilities.py > > +++ b/virtinst/capabilities.py > > @@ -206,6 +206,8 @@ class _CapsGuestFeatures(XMLBuilder): > > _XML_ROOT_NAME = "features" > > > > pae = XMLProperty("./pae", is_bool=True) > > + acpi = XMLProperty("./acpi/@default", is_onoff=True) > > + apic = XMLProperty("./apic/@default", is_onoff=True) > > > > > > class _CapsGuest(XMLBuilder): > > @@ -298,6 +300,16 @@ class _CapsGuest(XMLBuilder): > > return True > > return False > > > > + def supports_acpi(self): > > + if self.features.acpi: > > + return True > > + return False > > + > > + def supports_apic(self): > > + if self.features.apic: > > + return True > > + return False > > + > > > > You can just do: return bool(self.features.apic) > > > ############################ > > # Main capabilities object # > > diff --git a/virtinst/guest.py b/virtinst/guest.py > > index 7ff7bd5..3f91ae9 100644 > > --- a/virtinst/guest.py > > +++ b/virtinst/guest.py > > @@ -840,9 +840,12 @@ class Guest(XMLBuilder): > > default = False > > > > if self.features.acpi == "default": > > - self.features.acpi = self._os_object.supports_acpi(default) > > + if default: > > + self.features.acpi = self.capsinfo.guest.supports_acpi() > > + else: > > + self.features.acpi = False > > if self.features.apic == "default": > > - self.features.apic = self._os_object.supports_apic(default) > > + self.features.apic = self.capsinfo.guest.supports_apic() > > if self.features.pae == "default": > > self.features.pae = self.capsinfo.guest.supports_pae() > > > > You still need to abide the os_object bits. I'd say leave these 'if' > conditionals alone, and set the 'default' variable above to also depend on > capsinfo.guest.supports_*. But you'll probably need to have separate > acpi_default and apic_default vars Actually I've removed the functions from osdict, because the only function was, that in case the host os is "msdos" don't use ACPI/APIC and in all other cases set them as default. I don't think, that this a correct detection. ACPI/APIC are HW features and it should not depend on the OS itself. Pavel > > Thanks, > Cole > > > diff --git a/virtinst/osdict.py b/virtinst/osdict.py > > index 76c2260..bd82ae1 100644 > > --- a/virtinst/osdict.py > > +++ b/virtinst/osdict.py > > @@ -447,14 +447,6 @@ class _OsVariant(object): > > def supports_virtiommio(self): > > return self._is_related_to(["fedora19"]) > > > > - def supports_acpi(self, default): > > - if self._family in ['msdos']: > > - return False > > - return default > > - > > - def supports_apic(self, default): > > - return self.supports_acpi(default) > > - > > def default_netmodel(self): > > """ > > Default non-virtio net-model, since we check for that separately > > > _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
