On 10.07.2012 12:14, Christophe Fergeau wrote:
Commit 5e6ce1 moved down detection of the ACPI feature in
qemuParseCommandLine. However, when ACPI is detected, it clears
all feature flags in def->features to only set ACPI. This used to
be fine because this was the first place were def->features was set,
but after the move this is no longer necessarily true because this
block comes before the ACPI check:
if (strstr(def->emulator, "kvm")) {
def->virtType = VIR_DOMAIN_VIRT_KVM;
def->features |= (1 << VIR_DOMAIN_FEATURE_PAE);
}
Since def is allocated in qemuParseCommandLine using VIR_ALLOC, we
can always use |= when modifying def->features
---
This is an issue I noticed while reading this code for other reasons,
I have only compile-tested it and I'm not sure how to test it.
Christophe
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6549f57..0065e83 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7547,7 +7547,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
goto no_memory;
if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch,
"x86_64"))
- def->features = (1 << VIR_DOMAIN_FEATURE_ACPI)
+ def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI)
/*| (1 << VIR_DOMAIN_FEATURE_APIC)*/;
#define WANT_VALUE() \
const char *val = progargv[++i]; \
Cool, so now we don't overwrite PAE feature set just a few lines above (not visible in
this diff though).
ACK with this squashed in:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml
b/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml
index e07c1f6..8abcb51 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml
@@ -8,6 +8,9 @@
<type arch='i686' machine='pc'>hvm</type>
<boot dev='network'/>
</os>
+ <features>
+ <pae/>
+ </features>
<clock offset='utc'>
<timer name='kvmclock' present='no'/>
</clock>
Michal