[libvirt] [PATCH] Fix /domain/features setting in qemuParseCommandLine

[Michal Privoznik]

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