[et-mgmt-tools] virtinst: acpi/apic == FALSE for Windows XP

[Cole Robinson]

Laurent Léonard wrote:
> Le mardi 14 juillet 2009 à 18:16, Cole Robinson a écrit :
>> Cole Robinson wrote:
>>> Laurent Léonard wrote:
>>>> Le mardi 07 juillet 2009 à 18:58, Pasi Kärkkäinen a écrit :
>>>>> On Tue, Jul 07, 2009 at 09:53:45AM +0200, Chris Lalancette wrote:
>>>>>> Pasi Kärkkäinen wrote:
>>>>>>> On Sun, Jul 05, 2009 at 04:50:59PM -0400, Cole Robinson wrote:
>>>>>>>> On 07/05/2009 08:27 AM, Guido Günther wrote:
>>>>>>>>> On Sat, Jul 04, 2009 at 06:09:01PM +0200, Guido Günther wrote:
>>>>>>>>>> Hi,
>>>>>>>>>> virtinst has:
>>>>>>>>>>
>>>>>>>>>> "winxp":{ "label": "Microsoft Windows XP (x86)",
>>>>>>>>>>                   "acpi": False, "apic": False },
>>>>>>>>>>
>>>>>>>>>> I couldn't find any reason for this in the hg logs and
>>>>>>>>>>   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=512439
>>>>>>>>> The correct URL is:
>>>>>>>>>   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=533201
>>>>>>>>> Cheers,
>>>>>>>>>  -- Guido
>>>>>>>> AFAIK this was done because windows installs on xen used to choke if
>>>>>>>> ACPI was enabled. There is an old wiki page corroborating that here,
>>>>>>>> no idea if it is still relevant, or if there were other issues at
>>>>>>>> play:
>>>>>>>>
>>>>>>>> http://wiki.xensource.com/xenwiki/XenWindowsACPI
>>>>>>> I think that's obsolete and nowadays ACPI (and APIC) should be
>>>>>>> enabled for Xen Windows guests.
>>>>>>>
>>>>>>> Please correct me if I'm wrong.
>>>>>> You are probably correct, although testing (especially on things like
>>>>>> RHEL-5 Xen) is in order before making the change for Xen as well.
>>>>> I actually asked about this on xen-devel, and Keir Fraser replied that
>>>>> ACPI with Windows has been working properly at least since Xen 3.1.0
>>>>> days.
>>>> I also installed Windows 2000 Server virtual machines with ACPI enabled
>>>> on Debian Squeeze with Xen 3.2.1 and KVM 85, no problem.
>>>>
>>>> So the attached patch should be applied to virtinst to:
>>>> - Enable ACPI and APIC for Windows XP
>>>> - Enable ACPI and APIC for Windows 2000
>>>> - Merge Windows XP x86 and x86_64 variants because there is no more
>>>> reason to have different variants for the same OS
>>> Hmm, I would like to maintain the current behavior for xen < 3.1.0,
>>> since RHEL/CentOS is still a target and I wouldn't want to regress. We
>>> should be able to get this info from libvirt and the ACPI defaults
>>> accordingly, I'll test it out shortly.
>> I've pushed changes which enable APIC and ACPI for XP and 2000 for
>> everything except Xen < 3.1.0.
> 
> I'm not sure but, why did you re-specify the value for non-Xen <3.1 hosts in 
> each item if the ACPI and APIC are already enabled by default for Windows 
> variants ?
> 
> What about this:
> 
>     },
>     "variants": { \
>         "winxp":{ "label": "Microsoft Windows XP (x86)",
>                   "acpi": [ ("xen", 3001000, False) ],
>                   "apic": [ ("xen", 3001000, False) ] },
>         "winxp64":{ "label": "Microsoft Windows XP (x86_64)" },
>         "win2k": { "label": "Microsoft Windows 2000",
>                   "acpi": [ ("xen", 3001000, False) ],
>                   "apic": [ ("xen", 3001000, False) ] },
>         "win2k3": { "label": "Microsoft Windows 2003" },
>         "win2k8": { "label": "Microsoft Windows 2008" },
>         "vista": { "label": "Microsoft Windows Vista" },
> 
> instead of this:
> 
>     },
>     "variants": { \
>         "winxp":{ "label": "Microsoft Windows XP (x86)",
>                   "acpi": [ ("xen", 3001000, False),
>                             ("all", True ), ],
>                   "apic": [ ("xen", 3001000, False),
>                             ("all", True ), ], },
>         "winxp64":{ "label": "Microsoft Windows XP (x86_64)" },
>         "win2k": { "label": "Microsoft Windows 2000",
>                   "acpi": [ ("xen", 3001000, False),
>                             ("all", True ), ],
>                   "apic": [ ("xen", 3001000, False),
>                             ("all", True ), ], },
>         "win2k3": { "label": "Microsoft Windows 2003" },
>         "win2k8": { "label": "Microsoft Windows 2008" },
>         "vista": { "label": "Microsoft Windows Vista" },
> 

The lookup code is currently dumb in this respect: it only falls back to
the default value if the lookup key (apic, acpi, etc.) isn't present at
all. So the above change won't work at the moment, and indeed causes the
test suite (python setup.py test && python setup.py test_cli) to fail.
I'd take a patch to properly fix it though.

Thanks,
Cole