Libvirt patches for launching VMs with 'ppc64le' architecture.

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Thank you for taking the time to report this bug and helping to make Ubuntu better. It seems that your bug report is not filed about a specific source package though, rather it is just filed against Ubuntu in general. It is important that bug reports be filed about source packages so that people interested in the package can find the bugs about it. You can find some hints about determining what package your bug might be about at https://wiki.ubuntu.com/Bugs/FindRightPackage. You might also ask for help in the #ubuntu-bugs irc channel on Freenode.

To change the source package that this bug is filed about visit https://bugs.launchpad.net/ubuntu/+bug/1396070/+editstatus and add the package name in the text box next to the word Package.

[This is an automated message. I apologize if it reached you inappropriately; please just reply to this message indicating so.]

Now too late but I had noticed that the provided patch #3 missed two changes from the upstream patch. Not sure how important those would be:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1b8efb1..21309b0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10043,7 +10043,7 @@ virDomainVideoDefaultType(const virDomainDef *def)
             (STREQ(def->os.type, "xen") ||
              STREQ(def->os.type, "linux")))
             return VIR_DOMAIN_VIDEO_TYPE_XEN;
- else if (def->os.arch == VIR_ARCH_PPC64)
+ else if ARCH_IS_PPC64(def->os.arch)
             return VIR_DOMAIN_VIDEO_TYPE_VGA;
         else
             return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
index 67cb9ff..d591c18 100644
--- a/src/cpu/cpu_powerpc.c
+++ b/src/cpu/cpu_powerpc.c
@@ -38,7 +38,7 @@

 VIR_LOG_INIT("cpu.cpu_powerpc");

-static const virArch archs[] = { VIR_ARCH_PPC64 };
+static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE };

 struct ppc_vendor {
     char *name;

This bug was fixed in the package libvirt - 1.2.8-0ubuntu16

---------------
libvirt (1.2.8-0ubuntu16) vivid; urgency=medium

  * debian/patches/add-ppc64le-support.patch: Added patches needed
    for ppc64le support. (LP: #1396070)
 -- Chuck Short <email address hidden> Thu, 27 Nov 2014 08:57:35 -0500

------- Comment From <email address hidden> 2014-11-28 05:29 EDT-------
(In reply to comment #14)
> Now too late but I had noticed that the provided patch #3 missed two changes
> from the upstream patch. Not sure how important those would be:
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1b8efb1..21309b0 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10043,7 +10043,7 @@ virDomainVideoDefaultType(const virDomainDef *def)
> (STREQ(def->os.type, "xen") ||
> STREQ(def->os.type, "linux")))
> return VIR_DOMAIN_VIDEO_TYPE_XEN;
> - else if (def->os.arch == VIR_ARCH_PPC64)
> + else if ARCH_IS_PPC64(def->os.arch)
> return VIR_DOMAIN_VIDEO_TYPE_VGA;
> else
> return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
> diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
> index 67cb9ff..d591c18 100644
> --- a/src/cpu/cpu_powerpc.c
> +++ b/src/cpu/cpu_powerpc.c
> @@ -38,7 +38,7 @@
>
> VIR_LOG_INIT("cpu.cpu_powerpc");
>
> -static const virArch archs[] = { VIR_ARCH_PPC64 };
> +static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE };
>
> struct ppc_vendor {
> char *name;
>
> This bug was fixed in the package libvirt - 1.2.8-0ubuntu16
>
> ---------------
> libvirt (1.2.8-0ubuntu16) vivid; urgency=medium
>
> * debian/patches/add-ppc64le-support.patch: Added patches needed
> for ppc64le support. (LP: #1396070)
> -- Chuck Short <email address hidden> Thu, 27 Nov 2014 08:57:35 -0500

Yes, these are needed. Wonder how this didnt get flagged in the build test.
In fact the upstream tree 1.2.10+ has these patches in. Is there a chance the upcoming 1.2.11 release could be packaged for Ubuntu ?

------- Comment on attachment From <email address hidden> 2014-11-28 10:06 EDT-------

Needed for cleanly applying patch '003B', which I will upload next.

This is upstream commit :
commit 44e30277d847f1d2e28531b0020f6a5ad8835542
Author: Eric Blake <email address hidden>
Date: Wed Sep 3 13:32:36 2014 -0600

    maint: use consistent if-else braces in qemu

    I'm about to add a syntax check that enforces our documented
    HACKING style of always using matching {} on if-else statements.

    This commit focuses on the qemu driver.

    * src/qemu/qemu_command.c (qemuParseISCSIString)
    (qemuParseCommandLineDisk, qemuParseCommandLine)
    (qemuBuildSmpArgStr, qemuBuildCommandLine)
    (qemuParseCommandLineDisk, qemuParseCommandLineSmp): Correct use
    of {}.
    * src/qemu/qemu_capabilities.c (virQEMUCapsProbeCPUModels):
    Likewise.
    * src/qemu/qemu_driver.c (qemuDomainCoreDumpWithFormat)
    (qemuDomainRestoreFlags, qemuDomainGetInfo)
    (qemuDomainMergeBlkioDevice): Likewise.
    * src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise.
    * src/qemu/qemu_monitor_text.c (qemuMonitorTextCreateSnapshot)

------- Comment From <email address hidden> 2014-11-28 10:16 EDT-------
I have obsoleted the previous patch for adding LE arch support to libvirt CPU driver ("0003-Cpu-Add-support-for-Power-LE-Architecture.patch")

In its place, I have uploaded a fresh version of the patch that includes all changes from upstream.
I have also added another trivial upstream patch that is required for my updated patch to apply.

The series file for libvirt patches now looks like this:

0001-qemu_command-Split-qemuBuildCpuArgStr.patch
0002-qemu-Don-t-compare-CPU-against-host-for-TCG.patch
0003-maint-use-consistent-if-else-braces-in-qemu.patch [Newly added, for 0004 to apply cleanly]
0004-CPU-Add-support-for-Power-LE-Architecture.patch [Updated patch for LE support]
0005-PowerPC-Add-support-for-launching-VM-in-compat-mode.patch
0006-PowerPC-Improve-PVR-handling-to-fall-back-to-cpu-gen.patch
0007-docs-Add-documentation-for-compat-mode.patch
0008-Test-Add-a-testcase-for-PowerPC-compat-mode-cpu-spec.patch

All other patches are unchanged.

------- Comment on attachment From <email address hidden> 2014-11-28 10:12 EDT-------

(NOTE: This patch needs to be applied after " maint: use consistent if-else braces in qemu " )

This is updated patch for adding PPC64LE arch support to libvirt CPU driver.
This obsoletes "0003-Cpu-Add-support-for-Power-LE-Architecture.patch"

Upstream commit :
commit da636d83dc6b1d070a705786b4daef8644eaca13
Author: Prerna Saxena <email address hidden>
Date: Tue Nov 4 22:51:26 2014 +0530

    Cpu: Add support for Power LE Architecture.

    This adds support for PowerPC Little Endian architecture.,
    and allows libvirt to spawn VMs based on 'ppc64le' architecture.

    Signed-off-by: Pradipta Kr. Banerjee <email address hidden>
    Signed-off-by: Prerna Saxena <email address hidden>
    Reviewed-by: Michal Privoznik <email address hidden>

Hm, seems like tha tmight make this not work on ppc64le. Could someone
with the hardware please test and confirm?

Quoting Stefan Bader (<email address hidden>):
> Now too late but I had noticed that the provided patch #3 missed two
> changes from the upstream patch. Not sure how important those would be:
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1b8efb1..21309b0 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10043,7 +10043,7 @@ virDomainVideoDefaultType(const virDomainDef *def)
> (STREQ(def->os.type, "xen") ||
> STREQ(def->os.type, "linux")))
> return VIR_DOMAIN_VIDEO_TYPE_XEN;
> - else if (def->os.arch == VIR_ARCH_PPC64)
> + else if ARCH_IS_PPC64(def->os.arch)
> return VIR_DOMAIN_VIDEO_TYPE_VGA;
> else
> return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
> diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
> index 67cb9ff..d591c18 100644
> --- a/src/cpu/cpu_powerpc.c
> +++ b/src/cpu/cpu_powerpc.c
> @@ -38,7 +38,7 @@
>
> VIR_LOG_INIT("cpu.cpu_powerpc");
>
> -static const virArch archs[] = { VIR_ARCH_PPC64 };
> +static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE };
>
> struct ppc_vendor {
> char *name;

------- Comment From <email address hidden> 2015-02-08 11:54 EDT-------
(In reply to comment #24)
> Hm, seems like that might make this not work on ppc64le. Could someone
> with the hardware please test and confirm?
>
> Quoting Stefan Bader (<email address hidden>):
> > Now too late but I had noticed that the provided patch #3 missed two
> > changes from the upstream patch. Not sure how important those would be:
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 1b8efb1..21309b0 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -10043,7 +10043,7 @@ virDomainVideoDefaultType(const virDomainDef *def)
> > (STREQ(def->os.type, "xen") ||
> > STREQ(def->os.type, "linux")))
> > return VIR_DOMAIN_VIDEO_TYPE_XEN;
> > - else if (def->os.arch == VIR_ARCH_PPC64)
> > + else if ARCH_IS_PPC64(def->os.arch)
> > return VIR_DOMAIN_VIDEO_TYPE_VGA;
> > else
> > return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
> > diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
> > index 67cb9ff..d591c18 100644
> > --- a/src/cpu/cpu_powerpc.c
> > +++ b/src/cpu/cpu_powerpc.c
> > @@ -38,7 +38,7 @@
> >
> > VIR_LOG_INIT("cpu.cpu_powerpc");
> >
> > -static const virArch archs[] = { VIR_ARCH_PPC64 };
> > +static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE };
> >
> > struct ppc_vendor {
> > char *name;

Yes, these two changes are indeed essential. I verified that:
1) These have not been included in the libvirt package shipped with ubuntu 15.04.
2) These are essential to launching guest VMs on ppc64 little endian arch.

In fact, I had uploaded a modified patch too at the time, but it seems that got missed.
Can these changes be included now ?

It looks to me like tha tis in fact present in 1.2.12. Could you please look at 1.2.12-0ubuntu2 and let us know whichi if any patches are still missing?

Ping - can you please confirm that the patches are missing, and, if so, point to the exact patches still needed?

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

------- Comment on attachment From <email address hidden> 2014-11-28 10:06 EDT-------

Needed for cleanly applying patch '003B', which I will upload next.

This is upstream commit :
commit 44e30277d847f1d2e28531b0020f6a5ad8835542
Author: Eric Blake <email address hidden>
Date: Wed Sep 3 13:32:36 2014 -0600

    maint: use consistent if-else braces in qemu

    I'm about to add a syntax check that enforces our documented
    HACKING style of always using matching {} on if-else statements.

    This commit focuses on the qemu driver.

    * src/qemu/qemu_command.c (qemuParseISCSIString)
    (qemuParseCommandLineDisk, qemuParseCommandLine)
    (qemuBuildSmpArgStr, qemuBuildCommandLine)
    (qemuParseCommandLineDisk, qemuParseCommandLineSmp): Correct use
    of {}.
    * src/qemu/qemu_capabilities.c (virQEMUCapsProbeCPUModels):
    Likewise.
    * src/qemu/qemu_driver.c (qemuDomainCoreDumpWithFormat)
    (qemuDomainRestoreFlags, qemuDomainGetInfo)
    (qemuDomainMergeBlkioDevice): Likewise.
    * src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise.
    * src/qemu/qemu_monitor_text.c (qemuMonitorTextCreateSnapshot)

------- Comment on attachment From <email address hidden> 2014-11-28 10:12 EDT-------

(NOTE: This patch needs to be applied after " maint: use consistent if-else braces in qemu " )

This is updated patch for adding PPC64LE arch support to libvirt CPU driver.
This obsoletes "0003-Cpu-Add-support-for-Power-LE-Architecture.patch"

Upstream commit :
commit da636d83dc6b1d070a705786b4daef8644eaca13
Author: Prerna Saxena <email address hidden>
Date: Tue Nov 4 22:51:26 2014 +0530

    Cpu: Add support for Power LE Architecture.

    This adds support for PowerPC Little Endian architecture.,
    and allows libvirt to spawn VMs based on 'ppc64le' architecture.

    Signed-off-by: Pradipta Kr. Banerjee <email address hidden>
    Signed-off-by: Prerna Saxena <email address hidden>
    Reviewed-by: Michal Privoznik <email address hidden>

These patches (at least the one in comment #27) are in the vivid package (1.2.12-0ubuntu12).

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

------- Comment on attachment From <email address hidden> 2014-11-28 10:06 EDT-------

Needed for cleanly applying patch '003B', which I will upload next.

This is upstream commit :
commit 44e30277d847f1d2e28531b0020f6a5ad8835542
Author: Eric Blake <email address hidden>
Date: Wed Sep 3 13:32:36 2014 -0600

    maint: use consistent if-else braces in qemu

    I'm about to add a syntax check that enforces our documented
    HACKING style of always using matching {} on if-else statements.

    This commit focuses on the qemu driver.

    * src/qemu/qemu_command.c (qemuParseISCSIString)
    (qemuParseCommandLineDisk, qemuParseCommandLine)
    (qemuBuildSmpArgStr, qemuBuildCommandLine)
    (qemuParseCommandLineDisk, qemuParseCommandLineSmp): Correct use
    of {}.
    * src/qemu/qemu_capabilities.c (virQEMUCapsProbeCPUModels):
    Likewise.
    * src/qemu/qemu_driver.c (qemuDomainCoreDumpWithFormat)
    (qemuDomainRestoreFlags, qemuDomainGetInfo)
    (qemuDomainMergeBlkioDevice): Likewise.
    * src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise.
    * src/qemu/qemu_monitor_text.c (qemuMonitorTextCreateSnapshot)

------- Comment on attachment From <email address hidden> 2014-11-28 10:12 EDT-------

(NOTE: This patch needs to be applied after " maint: use consistent if-else braces in qemu " )

This is updated patch for adding PPC64LE arch support to libvirt CPU driver.
This obsoletes "0003-Cpu-Add-support-for-Power-LE-Architecture.patch"

Upstream commit :
commit da636d83dc6b1d070a705786b4daef8644eaca13
Author: Prerna Saxena <email address hidden>
Date: Tue Nov 4 22:51:26 2014 +0530

    Cpu: Add support for Power LE Architecture.

    This adds support for PowerPC Little Endian architecture.,
    and allows libvirt to spawn VMs based on 'ppc64le' architecture.

    Signed-off-by: Pradipta Kr. Banerjee <email address hidden>
    Signed-off-by: Prerna Saxena <email address hidden>
    Reviewed-by: Michal Privoznik <email address hidden>

I confirmed that the patches were included in vivid. Thanks!

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

Default Comment by Bridge

------- Comment on attachment From <email address hidden> 2014-11-28 10:06 EDT-------

Needed for cleanly applying patch '003B', which I will upload next.

This is upstream commit :
commit 44e30277d847f1d2e28531b0020f6a5ad8835542
Author: Eric Blake <email address hidden>
Date: Wed Sep 3 13:32:36 2014 -0600

    maint: use consistent if-else braces in qemu

    I'm about to add a syntax check that enforces our documented
    HACKING style of always using matching {} on if-else statements.

    This commit focuses on the qemu driver.

    * src/qemu/qemu_command.c (qemuParseISCSIString)
    (qemuParseCommandLineDisk, qemuParseCommandLine)
    (qemuBuildSmpArgStr, qemuBuildCommandLine)
    (qemuParseCommandLineDisk, qemuParseCommandLineSmp): Correct use
    of {}.
    * src/qemu/qemu_capabilities.c (virQEMUCapsProbeCPUModels):
    Likewise.
    * src/qemu/qemu_driver.c (qemuDomainCoreDumpWithFormat)
    (qemuDomainRestoreFlags, qemuDomainGetInfo)
    (qemuDomainMergeBlkioDevice): Likewise.
    * src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise.
    * src/qemu/qemu_monitor_text.c (qemuMonitorTextCreateSnapshot)

------- Comment on attachment From <email address hidden> 2014-11-28 10:12 EDT-------

(NOTE: This patch needs to be applied after " maint: use consistent if-else braces in qemu " )

This is updated patch for adding PPC64LE arch support to libvirt CPU driver.
This obsoletes "0003-Cpu-Add-support-for-Power-LE-Architecture.patch"

Upstream commit :
commit da636d83dc6b1d070a705786b4daef8644eaca13
Author: Prerna Saxena <email address hidden>
Date: Tue Nov 4 22:51:26 2014 +0530

    Cpu: Add support for Power LE Architecture.

    This adds support for PowerPC Little Endian architecture.,
    and allows libvirt to spawn VMs based on 'ppc64le' architecture.

    Signed-off-by: Pradipta Kr. Banerjee <email address hidden>
    Signed-off-by: Prerna Saxena <email address hidden>
    Reviewed-by: Michal Privoznik <email address hidden>

------- Comment From <email address hidden> 2016-02-11 01:04 EDT-------
Closing this bug as the fix has been released.