[libvirt] [PATCH 1/1] qemu: Remove default memory balloon for PPC64

From: Li Zhang <zhlcindy@linux.vnet.ibm.com> It doesn't need to add a default memory balloon for PPC64. Only if users want it, it can be added explicitly. Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 12 ------------ src/qemu/qemu_domain.c | 12 ++++++++---- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8dc7e43..a1e5387 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11989,18 +11989,6 @@ qemuParseCommandLine(virCapsPtr qemuCaps, def->videos[def->nvideos++] = vid; } - /* - * having a balloon is the default, define one with type="none" to avoid it - */ - if (!def->memballoon) { - virDomainMemballoonDefPtr memballoon; - if (VIR_ALLOC(memballoon) < 0) - goto error; - memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO; - - def->memballoon = memballoon; - } - VIR_FREE(nics); if (virDomainDefAddImplicitControllers(def) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 346fec3..0744e89 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -724,13 +724,17 @@ qemuDomainDefPostParse(virDomainDefPtr def, break; case VIR_ARCH_ARMV7L: - addDefaultUSB = false; - addDefaultMemballoon = false; - break; + addDefaultUSB = false; + addDefaultMemballoon = false; + break; + + case VIR_ARCH_PPC64: + addPCIRoot = true; + addDefaultMemballoon = false; + break; case VIR_ARCH_ALPHA: case VIR_ARCH_PPC: - case VIR_ARCH_PPC64: case VIR_ARCH_PPCEMB: case VIR_ARCH_SH4: case VIR_ARCH_SH4EB: -- 1.8.2.1

On Thu, Nov 21, 2013 at 01:52:02PM +0800, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
It doesn't need to add a default memory balloon for PPC64. Only if users want it, it can be added explicitly.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 12 ------------ src/qemu/qemu_domain.c | 12 ++++++++---- 2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8dc7e43..a1e5387 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11989,18 +11989,6 @@ qemuParseCommandLine(virCapsPtr qemuCaps, def->videos[def->nvideos++] = vid; }
- /* - * having a balloon is the default, define one with type="none" to avoid it - */ - if (!def->memballoon) { - virDomainMemballoonDefPtr memballoon; - if (VIR_ALLOC(memballoon) < 0) - goto error; - memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO; - - def->memballoon = memballoon; - } - VIR_FREE(nics);
if (virDomainDefAddImplicitControllers(def) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 346fec3..0744e89 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -724,13 +724,17 @@ qemuDomainDefPostParse(virDomainDefPtr def, break;
case VIR_ARCH_ARMV7L: - addDefaultUSB = false; - addDefaultMemballoon = false; - break; + addDefaultUSB = false; + addDefaultMemballoon = false; + break; + + case VIR_ARCH_PPC64: + addPCIRoot = true; + addDefaultMemballoon = false; + break;
case VIR_ARCH_ALPHA: case VIR_ARCH_PPC: - case VIR_ARCH_PPC64: case VIR_ARCH_PPCEMB: case VIR_ARCH_SH4: case VIR_ARCH_SH4EB:
Unless this is actively causing failures, then NACK to this patch. In retrospect we should not have enabled the balloon driver by default in the KVM driver. We do however have a way to turn this off in the XML, and I think it is better that the behaviour be consistent across architectures, instead of special casing PPC 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 :|

On 2013年11月21日 19:03, Daniel P. Berrange wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
It doesn't need to add a default memory balloon for PPC64. Only if users want it, it can be added explicitly.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 12 ------------ src/qemu/qemu_domain.c | 12 ++++++++---- 2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8dc7e43..a1e5387 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11989,18 +11989,6 @@ qemuParseCommandLine(virCapsPtr qemuCaps, def->videos[def->nvideos++] = vid; }
- /* - * having a balloon is the default, define one with type="none" to avoid it - */ - if (!def->memballoon) { - virDomainMemballoonDefPtr memballoon; - if (VIR_ALLOC(memballoon) < 0) - goto error; - memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO; - - def->memballoon = memballoon; - } - VIR_FREE(nics);
if (virDomainDefAddImplicitControllers(def) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 346fec3..0744e89 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -724,13 +724,17 @@ qemuDomainDefPostParse(virDomainDefPtr def, break;
case VIR_ARCH_ARMV7L: - addDefaultUSB = false; - addDefaultMemballoon = false; - break; + addDefaultUSB = false; + addDefaultMemballoon = false; + break; + + case VIR_ARCH_PPC64: + addPCIRoot = true; + addDefaultMemballoon = false; + break;
case VIR_ARCH_ALPHA: case VIR_ARCH_PPC: - case VIR_ARCH_PPC64: case VIR_ARCH_PPCEMB: case VIR_ARCH_SH4: case VIR_ARCH_SH4EB: Unless this is actively causing failures, then NACK to this
On Thu, Nov 21, 2013 at 01:52:02PM +0800, Li Zhang wrote: patch.
In retrospect we should not have enabled the balloon driver by default in the KVM driver. We do however have a way to turn this off in the XML, and I think it is better that the behaviour be consistent across architectures, instead of special casing PPC
Thanks for reviewing. Balloon driver is enabled in PPC guest with some distros and doesn't work well. We may need to turned it off by default. According to your comments, I think we need to fix this internally.
Daniel

On Thu, Nov 21, 2013 at 11:03:46AM +0000, Daniel P. Berrange wrote:
On Thu, Nov 21, 2013 at 01:52:02PM +0800, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
It doesn't need to add a default memory balloon for PPC64. Only if users want it, it can be added explicitly.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> [snip]
Unless this is actively causing failures, then NACK to this patch.
Both RHEL6.x and SLES11 SP3 include the virtio-balloon driver in their ppc64 versions, though the driver was never tested on ppc64. Unfortunately, virtio-balloon had endianness and page-size bugs prior to kernel v3.4, with the result that on those distros, any use of the balloon driver on ppc64 will crash the guest kernel. So in that sense this is actively causing failures. Regards, Paul.
participants (3)
-
Daniel P. Berrange
-
Li Zhang
-
Paul Mackerras