[libvirt] [RFC v3 PATCH 4/5] PowerPC : Clean up qemuBuildCommandLine to remove x86-specific assumptions from generic code.

[Daniel P. Berrange]

On Tue, Nov 29, 2011 at 08:31:05PM +0530, Prerna Saxena wrote:
> From: Prerna Saxena <prerna at linux.vnet.ibm.com>
> Date: Mon, 21 Nov 2011 18:20:42 +0530
> Subject: [PATCH 4/5] Clean up qemuBuildCommandLine to remove x86-specific
>  assumptions from generic code.
> 
> This implements the minimal set of changes needed in libvirt to launch a
> PowerPC-KVM based guest.
> It removes x86-specific assumptions about choice of serial driver backend
> from generic qemu guest commandline generation code.
> It also restricts the ACPI capability to be available for an x86 or
> x86_64 domain.
> This is not a complete solution -- it still does not guarantee libvirt
> the capability to flag non-supported options in guest XML. (Eg, an ACPI
> specification in a PowerPC guest XML will still get processed, even
> though qemu-system-ppc64 does not support it while qemu-system-x86_64 does.)
> This drawback exists because libvirt falls back on qemu to query supported
> features, and qemu '-h' blindly lists all capabilities -- irrespective
> of whether they are available while emulating a given architecture or not.
> The long-term solution would be for qemu to list out capabilities based
> on architecture and platform -- so that libvirt can cleanly make out what
> devices are supported on an arch (say 'ppc64') and platform (say, 'mac99').
> 
> Signed-off-by: Prerna Saxena <prerna at linux.vnet.ibm.com>
> ---
>  src/qemu/qemu_command.c |   39 ++++++++++++++++++++++++++++++++++++---
>  src/qemu/qemu_command.h |    6 ++++++
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 

>  /*
>   * This method takes a string representing a QEMU command line ARGV set
> @@ -6649,7 +6680,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>      def->maxvcpus = 1;
>      def->vcpus = 1;
>      def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
> -    def->features = (1 << VIR_DOMAIN_FEATURE_ACPI)
> +
> +    if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64"))
> +        def->features = (1 << VIR_DOMAIN_FEATURE_ACPI)
>          /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/;
>      def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART;
>      def->onCrash = VIR_DOMAIN_LIFECYCLE_DESTROY;

This is the cause of one of the tes case failures Stefan mentions.
At this point in the function, we have not set  def->os.arch to
any value.  You'll need to move this code right down to the end
of this function instead, and add a check for def->os.arch
being NULL too for safety.

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 :|