8:55 a.m.
Recent development in KVM for 64-bit Power ISA Book3S machines, allows
users to run multiple KVM guest instances on POWER7 and PPC970
processor based systems. Also qemu-system-ppc64 has been enhanced to
support a new machine type "pseries" suitable for Power Book3S machines.
This addition effectively brings the KVM+qemu combination to run
multiple guest instances on a Power Book3S machine.
Libvirt continues to be the key interface to configure and manage the
KVM guest instances on x86. This patch set is an effort to enable
libvirt to support KVM guest configuration and management on Power Book3S
machines.
Based on community discussion around the earlier version, this patch
series augments the present 'kvm' driver to support PowerPC-KVM based
guests.Since some of the supported devices vary between architectures,
libvirt must be capable of choosing supported device backends and
defaults for each architecture in qemu.
To check if qemu supports a certain feature, libvirt at present parses
the -help string which is generated by running the qemu binary with the
'-h' argument. This approach is gated by QEMU's inherent limitation.
When generating the list of allowed options with the '-h' flags, qemu
today blindly lists all options defined for any architecture/platform
instead of doing any arch-specific checking. This tricks libvirt into
assuming a much bigger set of host capabilities than is actually
available.
Ideally, it would be good to have qemu specify a list of devices for a
given architecture and platform which libvirt can parse to understand
supported capabilities for that guest.
As a part of this patchset, there is an attempt to cleanly bifurcate
libvirt code and to remove x86-specific assumptions from generic qemu
commandline code.
Series Description:
-------------------
This patch series consists of 5 patches :
Patch 1/5 : Use sysfs to gather host topology in place of /proc/cpuinfo.
Patch 2/5 : Add PowerPC CPU Driver
Patch 3/5 : Add support for qemu-system-ppc64
Patch 4/5 : Clean up x86-specific assumptions from generic qemu code.
Patch 5/5 : Add address family "spapr-vio"
Changelog:
---------
** v1->v2 :
* Patches 1,2,3 unchanged ; The hacks in Patch 4 of v1 replaced by a
new patch to neatly select arch-specific features.
** v2->v3 :
* Patches 1,2,3 have minor cleanups ; Patch 4 no longer has an
arch-specific handler routine. It is now replaced by a much simpler
patch that merely removes x86/pc-specific assumptions from libvirt.
** v3->v4 :
* Patches 1,2,3,4 unchanged ; patch 5 is a new addition from Michael Ellerman
that adds a new device-tree based addressing mechanism for the 'pseries'
guest.
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
8:56 a.m.
New subject: [libvirt] [RFC v3 PATCH 1/5] PowerPC : Use sysfs to gather host topology, in place of /proc/cpuinfo
From: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 05:45:30 -0700
Subject: [PATCH 1/5] Use sysfs to gather host topology, in place of
/proc/cpuinfo
Libvirt at present depends on /proc/cpuinfo to gather host
details such as CPUs, cores, threads, etc. This is an architecture-
dependent approach. An alternative is to use 'Sysfs', which provides
a platform-agnostic interface to parse host CPU topology.
Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
---
src/nodeinfo.c | 114 +++++++++++++++++++-------------------------------------
1 files changed, 39 insertions(+), 75 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 6448b79..f70654a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -30,6 +30,7 @@
#include <errno.h>
#include <dirent.h>
#include <sys/utsname.h>
+#include <sched.h>
#if HAVE_NUMACTL
# define NUMA_VERSION1_COMPATIBILITY 1
@@ -67,8 +68,7 @@
/* NB, this is not static as we need to call it from the testsuite */
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- virNodeInfoPtr nodeinfo,
- bool need_hyperthreads);
+ virNodeInfoPtr nodeinfo);
static int linuxNodeGetCPUStats(FILE *procstat,
int cpuNum,
@@ -191,23 +191,26 @@ static int parse_socket(unsigned int cpu)
return ret;
}
+static int parse_core(unsigned int cpu)
+{
+ return get_cpu_value(cpu, "topology/core_id", false);
+}
+
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- virNodeInfoPtr nodeinfo,
- bool need_hyperthreads)
+ virNodeInfoPtr nodeinfo)
{
char line[1024];
DIR *cpudir = NULL;
struct dirent *cpudirent = NULL;
unsigned int cpu;
- unsigned long cur_threads;
- int socket;
- unsigned long long socket_mask = 0;
- unsigned int remaining;
+ unsigned long core, socket, cur_threads;
+ cpu_set_t core_mask;
+ cpu_set_t socket_mask;
int online;
nodeinfo->cpus = 0;
nodeinfo->mhz = 0;
- nodeinfo->cores = 1;
+ nodeinfo->cores = 0;
nodeinfo->nodes = 1;
# if HAVE_NUMACTL
@@ -221,20 +224,10 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* NOTE: hyperthreads are ignored here; they are parsed out of /sys */
while (fgets(line, sizeof(line), cpuinfo) != NULL) {
char *buf = line;
- if (STRPREFIX(buf, "processor")) { /* aka a single logical CPU */
- buf += 9;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':') {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("parsing cpuinfo
processor"));
- return -1;
- }
- nodeinfo->cpus++;
# if defined(__x86_64__) || \
defined(__amd64__) || \
defined(__i386__)
- } else if (STRPREFIX(buf, "cpu MHz")) {
+ if (STRPREFIX(buf, "cpu MHz")) {
char *p;
unsigned int ui;
buf += 9;
@@ -249,24 +242,9 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* Accept trailing fractional part. */
&& (*p == '\0' || *p == '.' || c_isspace(*p)))
nodeinfo->mhz = ui;
- } else if (STRPREFIX(buf, "cpu cores")) { /* aka cores */
- char *p;
- unsigned int id;
- buf += 9;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':' || !buf[1]) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- _("parsing cpuinfo cpu cores %c"), *buf);
- return -1;
- }
- if (virStrToLong_ui(buf+1, &p, 10, &id) == 0
- && (*p == '\0' || c_isspace(*p))
- && id > nodeinfo->cores)
- nodeinfo->cores = id;
# elif defined(__powerpc__) || \
defined(__powerpc64__)
- } else if (STRPREFIX(buf, "clock")) {
+ if (STRPREFIX(buf, "clock")) {
char *p;
unsigned int ui;
buf += 5;
@@ -281,53 +259,30 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* Accept trailing fractional part. */
&& (*p == '\0' || *p == '.' || c_isspace(*p)))
nodeinfo->mhz = ui;
-# elif defined(__s390__) || \
- defined(__s390x__)
- } else if (STRPREFIX(buf, "# processors")) {
- char *p;
- unsigned int ui;
- buf += 12;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':' || !buf[1]) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- _("parsing number of processors %c"), *buf);
- return -1;
- }
- if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0
- && (*p == '\0' || c_isspace(*p)))
- nodeinfo->cpus = ui;
/* No other interesting infos are available in /proc/cpuinfo.
* However, there is a line identifying processor's version,
* identification and machine, but we don't want it to be caught
* and parsed in next iteration, because it is not in expected
* format and thus lead to error. */
- break;
# else
# warning Parser for /proc/cpuinfo needs to be adapted for your architecture
# endif
}
}
- if (!nodeinfo->cpus) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("no cpus found"));
- return -1;
- }
-
- if (!need_hyperthreads)
- return 0;
-
- /* OK, we've parsed what we can out of /proc/cpuinfo. Get the socket
- * and thread information from /sys
+ /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the core, socket
+ * thread and topology information from /sys
*/
- remaining = nodeinfo->cpus;
cpudir = opendir(CPU_SYS_PATH);
if (cpudir == NULL) {
virReportSystemError(errno, _("cannot opendir %s"), CPU_SYS_PATH);
return -1;
}
- while ((errno = 0), remaining && (cpudirent = readdir(cpudir))) {
+
+ CPU_ZERO(&core_mask);
+ CPU_ZERO(&socket_mask);
+
+ while ((cpudirent = readdir(cpudir))) {
if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
continue;
@@ -338,15 +293,19 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
}
if (!online)
continue;
- remaining--;
+ nodeinfo->cpus++;
- socket = parse_socket(cpu);
- if (socket < 0) {
- closedir(cpudir);
- return -1;
+ /* Parse core */
+ core = parse_core(cpu);
+ if (!CPU_ISSET(core, &core_mask)) {
+ CPU_SET(core, &core_mask);
+ nodeinfo->cores++;
}
- if (!(socket_mask & (1 << socket))) {
- socket_mask |= (1 << socket);
+
+ /* Parse socket */
+ socket = parse_socket(cpu);
+ if (!CPU_ISSET(socket, &socket_mask)) {
+ CPU_SET(socket, &socket_mask);
nodeinfo->sockets++;
}
@@ -367,7 +326,12 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
closedir(cpudir);
- /* there should always be at least one socket and one thread */
+ /* there should always be at least one cpu, socket and one thread */
+ if (nodeinfo->cpus == 0) {
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("no CPUs found"));
+ return -1;
+ }
if (nodeinfo->sockets == 0) {
nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("no sockets found"));
@@ -617,7 +581,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr
nodeinfo) {
_("cannot open %s"), CPUINFO_PATH);
return -1;
}
- ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo, true);
+ ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo);
VIR_FORCE_FCLOSE(cpuinfo);
if (ret < 0)
return -1;
--
1.7.7
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
6:30 a.m.
New subject: [libvirt] [RFC v3 PATCH 1/5] PowerPC : Use sysfs to gather host topology, in place of /proc/cpuinfo
On Tue, Nov 29, 2011 at 08:26:57PM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 05:45:30 -0700
Subject: [PATCH 1/5] Use sysfs to gather host topology, in place of
/proc/cpuinfo
Libvirt at present depends on /proc/cpuinfo to gather host
details such as CPUs, cores, threads, etc. This is an architecture-
dependent approach. An alternative is to use 'Sysfs', which provides
a platform-agnostic interface to parse host CPU topology.
Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
---
src/nodeinfo.c | 114 +++++++++++++++++++-------------------------------------
1 files changed, 39 insertions(+), 75 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 6448b79..f70654a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -30,6 +30,7 @@
#include <errno.h>
#include <dirent.h>
#include <sys/utsname.h>
+#include <sched.h>
#if HAVE_NUMACTL
# define NUMA_VERSION1_COMPATIBILITY 1
@@ -67,8 +68,7 @@
/* NB, this is not static as we need to call it from the testsuite */
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- virNodeInfoPtr nodeinfo,
- bool need_hyperthreads);
+ virNodeInfoPtr nodeinfo);
static int linuxNodeGetCPUStats(FILE *procstat,
int cpuNum,
@@ -191,23 +191,26 @@ static int parse_socket(unsigned int cpu)
return ret;
}
+static int parse_core(unsigned int cpu)
+{
+ return get_cpu_value(cpu, "topology/core_id", false);
+}
+
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- virNodeInfoPtr nodeinfo,
- bool need_hyperthreads)
+ virNodeInfoPtr nodeinfo)
{
char line[1024];
DIR *cpudir = NULL;
struct dirent *cpudirent = NULL;
unsigned int cpu;
- unsigned long cur_threads;
- int socket;
- unsigned long long socket_mask = 0;
- unsigned int remaining;
+ unsigned long core, socket, cur_threads;
+ cpu_set_t core_mask;
+ cpu_set_t socket_mask;
int online;
nodeinfo->cpus = 0;
nodeinfo->mhz = 0;
- nodeinfo->cores = 1;
+ nodeinfo->cores = 0;
nodeinfo->nodes = 1;
# if HAVE_NUMACTL
@@ -221,20 +224,10 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* NOTE: hyperthreads are ignored here; they are parsed out of /sys */
while (fgets(line, sizeof(line), cpuinfo) != NULL) {
char *buf = line;
- if (STRPREFIX(buf, "processor")) { /* aka a single logical CPU */
- buf += 9;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':') {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("parsing cpuinfo
processor"));
- return -1;
- }
- nodeinfo->cpus++;
# if defined(__x86_64__) || \
defined(__amd64__) || \
defined(__i386__)
- } else if (STRPREFIX(buf, "cpu MHz")) {
+ if (STRPREFIX(buf, "cpu MHz")) {
char *p;
unsigned int ui;
buf += 9;
@@ -249,24 +242,9 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* Accept trailing fractional part. */
&& (*p == '\0' || *p == '.' || c_isspace(*p)))
nodeinfo->mhz = ui;
- } else if (STRPREFIX(buf, "cpu cores")) { /* aka cores */
- char *p;
- unsigned int id;
- buf += 9;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':' || !buf[1]) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- _("parsing cpuinfo cpu cores %c"), *buf);
- return -1;
- }
- if (virStrToLong_ui(buf+1, &p, 10, &id) == 0
- && (*p == '\0' || c_isspace(*p))
- && id > nodeinfo->cores)
- nodeinfo->cores = id;
# elif defined(__powerpc__) || \
defined(__powerpc64__)
- } else if (STRPREFIX(buf, "clock")) {
+ if (STRPREFIX(buf, "clock")) {
char *p;
unsigned int ui;
buf += 5;
@@ -281,53 +259,30 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* Accept trailing fractional part. */
&& (*p == '\0' || *p == '.' || c_isspace(*p)))
nodeinfo->mhz = ui;
-# elif defined(__s390__) || \
- defined(__s390x__)
- } else if (STRPREFIX(buf, "# processors")) {
- char *p;
- unsigned int ui;
- buf += 12;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':' || !buf[1]) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- _("parsing number of processors %c"), *buf);
- return -1;
- }
- if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0
- && (*p == '\0' || c_isspace(*p)))
- nodeinfo->cpus = ui;
/* No other interesting infos are available in /proc/cpuinfo.
* However, there is a line identifying processor's version,
* identification and machine, but we don't want it to be caught
* and parsed in next iteration, because it is not in expected
* format and thus lead to error. */
- break;
# else
# warning Parser for /proc/cpuinfo needs to be adapted for your architecture
# endif
}
}
- if (!nodeinfo->cpus) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("no cpus found"));
- return -1;
- }
-
- if (!need_hyperthreads)
- return 0;
-
- /* OK, we've parsed what we can out of /proc/cpuinfo. Get the socket
- * and thread information from /sys
+ /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the core, socket
+ * thread and topology information from /sys
*/
- remaining = nodeinfo->cpus;
cpudir = opendir(CPU_SYS_PATH);
if (cpudir == NULL) {
virReportSystemError(errno, _("cannot opendir %s"), CPU_SYS_PATH);
return -1;
}
- while ((errno = 0), remaining && (cpudirent = readdir(cpudir))) {
+
+ CPU_ZERO(&core_mask);
+ CPU_ZERO(&socket_mask);
+
+ while ((cpudirent = readdir(cpudir))) {
if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
continue;
@@ -338,15 +293,19 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
}
if (!online)
continue;
- remaining--;
+ nodeinfo->cpus++;
- socket = parse_socket(cpu);
- if (socket < 0) {
- closedir(cpudir);
- return -1;
+ /* Parse core */
+ core = parse_core(cpu);
+ if (!CPU_ISSET(core, &core_mask)) {
+ CPU_SET(core, &core_mask);
+ nodeinfo->cores++;
}
- if (!(socket_mask & (1 << socket))) {
- socket_mask |= (1 << socket);
+
+ /* Parse socket */
+ socket = parse_socket(cpu);
+ if (!CPU_ISSET(socket, &socket_mask)) {
+ CPU_SET(socket, &socket_mask);
nodeinfo->sockets++;
}
@@ -367,7 +326,12 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
closedir(cpudir);
- /* there should always be at least one socket and one thread */
+ /* there should always be at least one cpu, socket and one thread */
+ if (nodeinfo->cpus == 0) {
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("no CPUs found"));
+ return -1;
+ }
if (nodeinfo->sockets == 0) {
nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("no sockets found"));
@@ -617,7 +581,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr
nodeinfo) {
_("cannot open %s"), CPUINFO_PATH);
return -1;
}
- ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo, true);
+ ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo);
VIR_FORCE_FCLOSE(cpuinfo);
if (ret < 0)
return -1;
There's nothing particularly wrong with the idea here, but as Stefan
mentions this has broken the test suite, because the nodeinfotest.c
now tries to read /sys from the test machine, instead of reading
our dummy data files in tests/nodeinfodata/
I think we'd probably need to create some dummy sysfs trees under
tests/nodeinfodata/test1/sys/..../ and make sure we can pass in
a path to the linuxNodeInfoCPUPopulate API so we cna point the
test at our sysfs, instead of the real machine
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 :|
6:34 a.m.
New subject: [libvirt] [RFC v3 PATCH 1/5] PowerPC : Use sysfs to gather host topology, in place of /proc/cpuinfo
Hi Daniel,
Thanks for taking a look.
On 11/30/2011 06:00 PM, Daniel P. Berrange wrote:
On Tue, Nov 29, 2011 at 08:26:57PM +0530, Prerna Saxena wrote:
> From: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
> Date: Mon, 3 Oct 2011 05:45:30 -0700
> Subject: [PATCH 1/5] Use sysfs to gather host topology, in place of
> /proc/cpuinfo
>
> Libvirt at present depends on /proc/cpuinfo to gather host
> details such as CPUs, cores, threads, etc. This is an architecture-
> dependent approach. An alternative is to use 'Sysfs', which provides
> a platform-agnostic interface to parse host CPU topology.
>
> Signed-off-by: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
> ---
> src/nodeinfo.c | 114 +++++++++++++++++++-------------------------------------
> 1 files changed, 39 insertions(+), 75 deletions(-)
>
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 6448b79..f70654a 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
[...snip...]
There's nothing particularly wrong with the idea here, but as Stefan
mentions this has broken the test suite, because the nodeinfotest.c
now tries to read /sys from the test machine, instead of reading
our dummy data files in tests/nodeinfodata/
I think we'd probably need to create some dummy sysfs trees under
tests/nodeinfodata/test1/sys/..../ and make sure we can pass in
a path to the linuxNodeInfoCPUPopulate API so we cna point the
test at our sysfs, instead of the real machine
Daniel
Thanks for the pointer. I'll now modify this test to use sysfs.
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
8:58 a.m.
New subject: [libvirt] [RFC v3 PATCH 2/5] PowerPC : Add PPC CPU driver
From: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 05:56:20 -0700
Subject: [PATCH 2/5] Add PPC cpu driver.
To add support for running libvirt on PowerPC, a CPU driver for the
PowerPC platform must be added.
Most generic cpu driver routines such as CPU compare, decode, etc
are based on CPUID comparison and are not relevant for non-x86
platforms.
Here, we introduce stubs for relevant PowerPC routines invoked by libvirt.
Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Signed-off-by: Anton Blanchard <anton(a)au.ibm.com>
---
src/Makefile.am | 3 +-
src/cpu/cpu.c | 2 +
src/cpu/cpu_powerpc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
src/cpu/cpu_powerpc.h | 32 +++++++++++++++++++
4 files changed, 117 insertions(+), 1 deletions(-)
create mode 100644 src/cpu/cpu_powerpc.c
create mode 100644 src/cpu/cpu_powerpc.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 33a32a8..aea3ddd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -533,7 +533,8 @@ CPU_SOURCES = \
cpu/cpu.h cpu/cpu.c \
cpu/cpu_generic.h cpu/cpu_generic.c \
cpu/cpu_x86.h cpu/cpu_x86.c cpu/cpu_x86_data.h \
- cpu/cpu_map.h cpu/cpu_map.c
+ cpu/cpu_map.h cpu/cpu_map.c cpu/cpu_powerpc.h \
+ cpu/cpu_powerpc.c
VMX_SOURCES = \
vmx/vmx.c vmx/vmx.h
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index b919b6e..e4149e2 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -28,6 +28,7 @@
#include "xml.h"
#include "cpu.h"
#include "cpu_x86.h"
+#include "cpu_powerpc.h"
#include "cpu_generic.h"
@@ -36,6 +37,7 @@
static struct cpuArchDriver *drivers[] = {
&cpuDriverX86,
+ &cpuDriverPowerPC,
/* generic driver must always be the last one */
&cpuDriverGeneric
};
diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
new file mode 100644
index 0000000..ed81694
--- /dev/null
+++ b/src/cpu/cpu_powerpc.c
@@ -0,0 +1,81 @@
+/*
+ * cpu_powerpc.h: CPU driver for PowerPC CPUs
+ *
+ * Copyright (C) Copyright (C) IBM Corporation, 2010
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Authors:
+ * Anton Blanchard <anton(a)au.ibm.com>
+ * Prerna Saxena <prerna(a)linux.vnet.ibm.com>
+ */
+
+#include <config.h>
+
+#include "memory.h"
+#include "cpu.h"
+
+
+#define VIR_FROM_THIS VIR_FROM_CPU
+
+static const char *archs[] = { "ppc64" };
+
+static union cpuData *
+PowerPCNodeData(void)
+{
+ union cpuData *data;
+
+ if (VIR_ALLOC(data) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ return data;
+}
+
+
+static int
+PowerPCDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED,
+ const union cpuData *data ATTRIBUTE_UNUSED,
+ const char **models ATTRIBUTE_UNUSED,
+ unsigned int nmodels ATTRIBUTE_UNUSED,
+ const char *preferred ATTRIBUTE_UNUSED)
+{
+ return 0;
+}
+
+static void
+PowerPCDataFree(union cpuData *data)
+{
+ if (data == NULL)
+ return;
+
+ VIR_FREE(data);
+}
+
+struct cpuArchDriver cpuDriverPowerPC = {
+ .name = "ppc64",
+ .arch = archs,
+ .narch = ARRAY_CARDINALITY(archs),
+ .compare = NULL,
+ .decode = PowerPCDecode,
+ .encode = NULL,
+ .free = PowerPCDataFree,
+ .nodeData = PowerPCNodeData,
+ .guestData = NULL,
+ .baseline = NULL,
+ .update = NULL,
+ .hasFeature = NULL,
+};
diff --git a/src/cpu/cpu_powerpc.h b/src/cpu/cpu_powerpc.h
new file mode 100644
index 0000000..2e0c1a5
--- /dev/null
+++ b/src/cpu/cpu_powerpc.h
@@ -0,0 +1,32 @@
+/*
+ * cpu_powerpc.h: CPU driver for PowerPC CPUs
+ *
+ * Copyright (C) Copyright (C) IBM Corporation, 2010
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Authors:
+ * Anton Blanchard <anton(a)au.ibm.com>
+ * Prerna Saxena <prerna(a)linux.vnet.ibm.com>
+ */
+
+#ifndef __VIR_CPU_POWERPC_H__
+# define __VIR_CPU_POWERPC_H__
+
+# include "cpu.h"
+
+extern struct cpuArchDriver cpuDriverPowerPC;
+
+#endif /* __VIR_CPU_POWERPC_H__ */
--
1.7.7
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
11:29 a.m.
New subject: [libvirt] [RFC v3 PATCH 2/5] PowerPC : Add PPC CPU driver
On 11/29/2011 09:58 AM, Prerna Saxena wrote:
From: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 05:56:20 -0700
Subject: [PATCH 2/5] Add PPC cpu driver.
To add support for running libvirt on PowerPC, a CPU driver for the
PowerPC platform must be added.
Most generic cpu driver routines such as CPU compare, decode, etc
are based on CPUID comparison and are not relevant for non-x86
platforms.
Here, we introduce stubs for relevant PowerPC routines invoked by libvirt.
Signed-off-by: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
Signed-off-by: Anton Blanchard<anton(a)au.ibm.com>
ACK
6:34 a.m.
New subject: [libvirt] [RFC v3 PATCH 2/5] PowerPC : Add PPC CPU driver
On Tue, Nov 29, 2011 at 08:28:55PM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 05:56:20 -0700
Subject: [PATCH 2/5] Add PPC cpu driver.
To add support for running libvirt on PowerPC, a CPU driver for the
PowerPC platform must be added.
Most generic cpu driver routines such as CPU compare, decode, etc
are based on CPUID comparison and are not relevant for non-x86
platforms.
Here, we introduce stubs for relevant PowerPC routines invoked by libvirt.
Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Signed-off-by: Anton Blanchard <anton(a)au.ibm.com>
---
src/Makefile.am | 3 +-
src/cpu/cpu.c | 2 +
src/cpu/cpu_powerpc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
src/cpu/cpu_powerpc.h | 32 +++++++++++++++++++
4 files changed, 117 insertions(+), 1 deletions(-)
create mode 100644 src/cpu/cpu_powerpc.c
create mode 100644 src/cpu/cpu_powerpc.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 33a32a8..aea3ddd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -533,7 +533,8 @@ CPU_SOURCES = \
cpu/cpu.h cpu/cpu.c \
cpu/cpu_generic.h cpu/cpu_generic.c \
cpu/cpu_x86.h cpu/cpu_x86.c cpu/cpu_x86_data.h \
- cpu/cpu_map.h cpu/cpu_map.c
+ cpu/cpu_map.h cpu/cpu_map.c cpu/cpu_powerpc.h \
+ cpu/cpu_powerpc.c
VMX_SOURCES = \
vmx/vmx.c vmx/vmx.h
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index b919b6e..e4149e2 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -28,6 +28,7 @@
#include "xml.h"
#include "cpu.h"
#include "cpu_x86.h"
+#include "cpu_powerpc.h"
#include "cpu_generic.h"
@@ -36,6 +37,7 @@
static struct cpuArchDriver *drivers[] = {
&cpuDriverX86,
+ &cpuDriverPowerPC,
/* generic driver must always be the last one */
&cpuDriverGeneric
};
diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
new file mode 100644
index 0000000..ed81694
--- /dev/null
+++ b/src/cpu/cpu_powerpc.c
@@ -0,0 +1,81 @@
+/*
+ * cpu_powerpc.h: CPU driver for PowerPC CPUs
+ *
+ * Copyright (C) Copyright (C) IBM Corporation, 2010
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Authors:
+ * Anton Blanchard <anton(a)au.ibm.com>
+ * Prerna Saxena <prerna(a)linux.vnet.ibm.com>
+ */
+
+#include <config.h>
+
+#include "memory.h"
+#include "cpu.h"
+
+
+#define VIR_FROM_THIS VIR_FROM_CPU
+
+static const char *archs[] = { "ppc64" };
+
+static union cpuData *
+PowerPCNodeData(void)
+{
+ union cpuData *data;
+
+ if (VIR_ALLOC(data) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ return data;
+}
+
+
+static int
+PowerPCDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED,
+ const union cpuData *data ATTRIBUTE_UNUSED,
+ const char **models ATTRIBUTE_UNUSED,
+ unsigned int nmodels ATTRIBUTE_UNUSED,
+ const char *preferred ATTRIBUTE_UNUSED)
+{
+ return 0;
+}
+
+static void
+PowerPCDataFree(union cpuData *data)
+{
+ if (data == NULL)
+ return;
+
+ VIR_FREE(data);
+}
+
+struct cpuArchDriver cpuDriverPowerPC = {
+ .name = "ppc64",
+ .arch = archs,
+ .narch = ARRAY_CARDINALITY(archs),
+ .compare = NULL,
+ .decode = PowerPCDecode,
+ .encode = NULL,
+ .free = PowerPCDataFree,
+ .nodeData = PowerPCNodeData,
+ .guestData = NULL,
+ .baseline = NULL,
+ .update = NULL,
+ .hasFeature = NULL,
+};
diff --git a/src/cpu/cpu_powerpc.h b/src/cpu/cpu_powerpc.h
new file mode 100644
index 0000000..2e0c1a5
--- /dev/null
+++ b/src/cpu/cpu_powerpc.h
@@ -0,0 +1,32 @@
+/*
+ * cpu_powerpc.h: CPU driver for PowerPC CPUs
+ *
+ * Copyright (C) Copyright (C) IBM Corporation, 2010
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Authors:
+ * Anton Blanchard <anton(a)au.ibm.com>
+ * Prerna Saxena <prerna(a)linux.vnet.ibm.com>
+ */
+
+#ifndef __VIR_CPU_POWERPC_H__
+# define __VIR_CPU_POWERPC_H__
+
+# include "cpu.h"
+
+extern struct cpuArchDriver cpuDriverPowerPC;
+
+#endif /* __VIR_CPU_POWERPC_H__ */
ACK
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 :|
1:40 p.m.
New subject: [libvirt] [RFC v3 PATCH 2/5] PowerPC : Add PPC CPU driver
On 11/30/2011 05:34 AM, Daniel P. Berrange wrote:
On Tue, Nov 29, 2011 at 08:28:55PM +0530, Prerna Saxena wrote:
> From: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
> Date: Mon, 3 Oct 2011 05:56:20 -0700
> Subject: [PATCH 2/5] Add PPC cpu driver.
>
> To add support for running libvirt on PowerPC, a CPU driver for the
> PowerPC platform must be added.
> Most generic cpu driver routines such as CPU compare, decode, etc
> are based on CPUID comparison and are not relevant for non-x86
> platforms.
> Here, we introduce stubs for relevant PowerPC routines invoked by libvirt.
>
ACK
Pushed, as well as adding you to AUTHORS. I'm still waiting for v4 on
1/5 that addresses Daniel's comments. Let me know if I need to adjust
any preferred spelling.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:59 a.m.
New subject: [libvirt] [RFC v3 PATCH 3/5] PowerPC : Add support for ppc64 qemu
From: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 06:01:33 -0700
Subject: [PATCH 3/5] Add support for ppc64 qemu
This enables libvirt to select the correct qemu binary (qemu-system-ppc64)
for a guest vm based on arch 'ppc64'.
Also, libvirt is enabled to correctly parse the list of supported PowerPC
CPUs, generated by running 'qemu-system-ppc64 -cpu ?'
Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
---
src/qemu/qemu_capabilities.c | 64 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c5fe41d..c2d3d93 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -185,6 +185,7 @@ static const struct qemu_arch_info const arch_info_hvm[] = {
{ "mipsel", 32, NULL, "qemu-system-mipsel", NULL, NULL, 0 },
{ "sparc", 32, NULL, "qemu-system-sparc", NULL, NULL, 0 },
{ "ppc", 32, NULL, "qemu-system-ppc", NULL, NULL, 0 },
+ { "ppc64", 64, NULL, "qemu-system-ppc64", NULL, NULL, 0
},
{ "itanium", 64, NULL, "qemu-system-ia64", NULL, NULL, 0 },
{ "s390x", 64, NULL, "qemu-system-s390x", NULL, NULL, 0 },
};
@@ -477,6 +478,67 @@ error:
return -1;
}
+/* ppc64 parser.
+ * Format : PowerPC <machine> <description>
+ */
+static int
+qemuCapsParsePPCModels(const char *output,
+ unsigned int *retcount,
+ const char ***retcpus)
+{
+ const char *p = output;
+ const char *next;
+ unsigned int count = 0;
+ const char **cpus = NULL;
+ int i;
+ do {
+ const char *t;
+
+ if ((next = strchr(p, '\n')))
+ next++;
+
+ if (!STRPREFIX(p, "PowerPC "))
+ continue;
+
+ /* Skip the preceding sub-string "PowerPC " */
+ p += 8;
+
+ /*Malformed string, does not obey the format 'PowerPC <model>
<desc>'*/
+ if (!(t = strchr(p, ' ')) || (next && t >= next))
+ continue;
+
+ if (*p == '\0' || *p == '\n')
+ continue;
+
+ if (retcpus) {
+ unsigned int len;
+
+ if (VIR_REALLOC_N(cpus, count + 1) < 0)
+ goto error;
+
+ if (t)
+ len = t - p - 1;
+
+ if (!(cpus[count] = strndup(p, len)))
+ goto error;
+ }
+ count++;
+ } while ((p = next));
+
+ if (retcount)
+ *retcount = count;
+ if (retcpus)
+ *retcpus = cpus;
+ return 0;
+
+error:
+ if (cpus) {
+ for (i = 0; i < count; i++)
+ VIR_FREE(cpus[i]);
+ }
+ VIR_FREE(cpus);
+ return -1;
+}
int
qemuCapsProbeCPUModels(const char *qemu,
@@ -497,6 +559,8 @@ qemuCapsProbeCPUModels(const char *qemu,
if (STREQ(arch, "i686") || STREQ(arch, "x86_64"))
parse = qemuCapsParseX86Models;
+ else if (STREQ(arch, "ppc64"))
+ parse = qemuCapsParsePPCModels;
else {
VIR_DEBUG("don't know how to parse %s CPU models", arch);
return 0;
--
1.7.7
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
10:55 a.m.
New subject: [libvirt] [RFC v3 PATCH 3/5] PowerPC : Add support for ppc64 qemu
On 11/29/2011 09:59 AM, Prerna Saxena wrote:
From: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 06:01:33 -0700
Subject: [PATCH 3/5] Add support for ppc64 qemu
This enables libvirt to select the correct qemu binary (qemu-system-ppc64)
for a guest vm based on arch 'ppc64'.
Also, libvirt is enabled to correctly parse the list of supported PowerPC
CPUs, generated by running 'qemu-system-ppc64 -cpu ?'
Signed-off-by: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
---
src/qemu/qemu_capabilities.c | 64 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c5fe41d..c2d3d93 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -185,6 +185,7 @@ static const struct qemu_arch_info const arch_info_hvm[] = {
{ "mipsel", 32, NULL, "qemu-system-mipsel", NULL, NULL, 0 },
{ "sparc", 32, NULL, "qemu-system-sparc", NULL, NULL, 0 },
{ "ppc", 32, NULL, "qemu-system-ppc", NULL, NULL, 0 },
+ { "ppc64", 64, NULL, "qemu-system-ppc64", NULL, NULL, 0
},
{ "itanium", 64, NULL, "qemu-system-ia64", NULL, NULL, 0 },
{ "s390x", 64, NULL, "qemu-system-s390x", NULL, NULL, 0 },
};
@@ -477,6 +478,67 @@ error:
return -1;
}
+/* ppc64 parser.
+ * Format : PowerPC<machine> <description>
+ */
+static int
+qemuCapsParsePPCModels(const char *output,
+ unsigned int *retcount,
+ const char ***retcpus)
+{
+ const char *p = output;
+ const char *next;
+ unsigned int count = 0;
+ const char **cpus = NULL;
+ int i;
+ do {
+ const char *t;
+
+ if ((next = strchr(p, '\n')))
+ next++;
+
+ if (!STRPREFIX(p, "PowerPC "))
+ continue;
If what you are parsing here is indeed corrupted, you may
end up in an
endless loop with p not moving anymore. You have to advance 'p' at least
by 1 and have a check before whether *p == '\0' to get out of it. You
may also want to put this check before the calculation of 'next'.
+
+ /* Skip the preceding sub-string "PowerPC " */
+ p += 8;
+
+ /*Malformed string, does not obey the format 'PowerPC<model>
<desc>'*/
+ if (!(t = strchr(p, ' ')) || (next&& t>= next))
+ continue;
+
+ if (*p == '\0' || *p == '\n')
+ continue;
*p = '\0' presumably is end-of-string --
shouldn't we 'break' here ?
+
+ if (retcpus) {
if retcpus is not provided you may want to return -1
earlier
+ unsigned int len;
+
+ if (VIR_REALLOC_N(cpus, count + 1)< 0)
virReportOOMError();
+ goto error;
+
+ if (t)
+ len = t - p - 1;
len would remain uninitialized if t is not set,
but t at this point must
be set due to the check above. Remove 'if(t)'.
+
+ if (!(cpus[count] = strndup(p, len)))
virReportOOMError();
+ goto error;
+ }
+ count++;
+ } while ((p = next));
+
+ if (retcount)
+ *retcount = count;
+ if (retcpus)
+ *retcpus = cpus;
+ return 0;
+
+error:
+ if (cpus) {
+ for (i = 0; i< count; i++)
+ VIR_FREE(cpus[i]);
+ }
+ VIR_FREE(cpus);
+ return -1;
+}
int
qemuCapsProbeCPUModels(const char *qemu,
@@ -497,6 +559,8 @@ qemuCapsProbeCPUModels(const char *qemu,
if (STREQ(arch, "i686") || STREQ(arch, "x86_64"))
parse = qemuCapsParseX86Models;
+ else if (STREQ(arch, "ppc64"))
+ parse = qemuCapsParsePPCModels;
else {
VIR_DEBUG("don't know how to parse %s CPU models", arch);
return 0;
6:21 a.m.
New subject: [libvirt] [RFC v3 PATCH 3/5] PowerPC : Add support for ppc64 qemu
Hi Stefan,
Thanks for reviewing the patch.
On 11/29/2011 10:25 PM, Stefan Berger wrote:
On 11/29/2011 09:59 AM, Prerna Saxena wrote:
> From: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
> Date: Mon, 3 Oct 2011 06:01:33 -0700
> Subject: [PATCH 3/5] Add support for ppc64 qemu
>
> This enables libvirt to select the correct qemu binary
> (qemu-system-ppc64)
> for a guest vm based on arch 'ppc64'.
> Also, libvirt is enabled to correctly parse the list of supported PowerPC
> CPUs, generated by running 'qemu-system-ppc64 -cpu ?'
>
> Signed-off-by: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
> ---
> src/qemu/qemu_capabilities.c | 64
> ++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 64 insertions(+), 0 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index c5fe41d..c2d3d93 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -185,6 +185,7 @@ static const struct qemu_arch_info const
> arch_info_hvm[] = {
> { "mipsel", 32, NULL, "qemu-system-mipsel", NULL, NULL, 0 },
> { "sparc", 32, NULL, "qemu-system-sparc", NULL, NULL, 0 },
> { "ppc", 32, NULL, "qemu-system-ppc", NULL, NULL, 0 },
> + { "ppc64", 64, NULL, "qemu-system-ppc64", NULL, NULL, 0 },
> { "itanium", 64, NULL, "qemu-system-ia64", NULL, NULL, 0 },
> { "s390x", 64, NULL, "qemu-system-s390x", NULL, NULL, 0 },
> };
> @@ -477,6 +478,67 @@ error:
> return -1;
> }
>
> +/* ppc64 parser.
> + * Format : PowerPC<machine> <description>
> + */
> +static int
> +qemuCapsParsePPCModels(const char *output,
> + unsigned int *retcount,
> + const char ***retcpus)
> +{
> + const char *p = output;
> + const char *next;
> + unsigned int count = 0;
> + const char **cpus = NULL;
> + int i;
> + do {
> + const char *t;
> +
> + if ((next = strchr(p, '\n')))
> + next++;
> +
> + if (!STRPREFIX(p, "PowerPC "))
> + continue;
If what you are parsing here is indeed corrupted, you may end up in an
endless loop with p not moving anymore. You have to advance 'p' at least
by 1 and have a check before whether *p == '\0' to get out of it. You
may also want to put this check before the calculation of 'next'.
When the execution gets to a 'continue', the do..while loop will resume
execution from next iteration after processing the conditional check. In
this case, whe condition in 'while' loop is :
while ((p =next))
which automatically advances 'p'.
> +
> + /* Skip the preceding sub-string "PowerPC " */
> + p += 8;
> +
> + /*Malformed string, does not obey the format 'PowerPC<model>
<desc>'*/
> + if (!(t = strchr(p, ' ')) || (next&& t>= next))
> + continue;
> +
> + if (*p == '\0' || *p == '\n')
> + continue;
*p = '\0' presumably is end-of-string -- shouldn't we 'break' here ?
Agree. The reason it works fine in its present form is 'cos when p
becomes empty, 'next' is empty too, and the loop condition fails which
causes the do..while loop to terminate.
> +
> + if (retcpus) {
if retcpus is not provided you may want to return -1 earlier
> + unsigned int len;
> +
> + if (VIR_REALLOC_N(cpus, count + 1)< 0)
virReportOOMError();
> + goto error;
> +
> + if (t)
> + len = t - p - 1;
len would remain uninitialized if t is not set, but t at this point must
be set due to the check above. Remove 'if(t)'.
Will do.
> +
> + if (!(cpus[count] = strndup(p, len)))
virReportOOMError();
I need to explicitly free cpus[] array, hence hadnt invoked this routine.
I'll call this just before return '-1'.
> + goto error;
> + }
> + count++;
> + } while ((p = next));
> +
> + if (retcount)
> + *retcount = count;
> + if (retcpus)
> + *retcpus = cpus;
> + return 0;
> +
> +error:
> + if (cpus) {
> + for (i = 0; i< count; i++)
> + VIR_FREE(cpus[i]);
> + }
> + VIR_FREE(cpus);
> + return -1;
> +}
>
> int
> qemuCapsProbeCPUModels(const char *qemu,
> @@ -497,6 +559,8 @@ qemuCapsProbeCPUModels(const char *qemu,
>
> if (STREQ(arch, "i686") || STREQ(arch, "x86_64"))
> parse = qemuCapsParseX86Models;
> + else if (STREQ(arch, "ppc64"))
> + parse = qemuCapsParsePPCModels;
> else {
> VIR_DEBUG("don't know how to parse %s CPU models", arch);
> return 0;
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
6:37 a.m.
New subject: [libvirt] [RFC v3 PATCH 3/5] PowerPC : Add support for ppc64 qemu
On Tue, Nov 29, 2011 at 08:29:56PM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 06:01:33 -0700
Subject: [PATCH 3/5] Add support for ppc64 qemu
This enables libvirt to select the correct qemu binary (qemu-system-ppc64)
for a guest vm based on arch 'ppc64'.
Also, libvirt is enabled to correctly parse the list of supported PowerPC
CPUs, generated by running 'qemu-system-ppc64 -cpu ?'
Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
---
src/qemu/qemu_capabilities.c | 64 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c5fe41d..c2d3d93 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -185,6 +185,7 @@ static const struct qemu_arch_info const arch_info_hvm[] = {
{ "mipsel", 32, NULL, "qemu-system-mipsel", NULL, NULL, 0 },
{ "sparc", 32, NULL, "qemu-system-sparc", NULL, NULL, 0 },
{ "ppc", 32, NULL, "qemu-system-ppc", NULL, NULL, 0 },
+ { "ppc64", 64, NULL, "qemu-system-ppc64", NULL, NULL, 0
},
{ "itanium", 64, NULL, "qemu-system-ia64", NULL, NULL, 0 },
{ "s390x", 64, NULL, "qemu-system-s390x", NULL, NULL, 0 },
};
@@ -477,6 +478,67 @@ error:
return -1;
}
+/* ppc64 parser.
+ * Format : PowerPC <machine> <description>
+ */
+static int
+qemuCapsParsePPCModels(const char *output,
+ unsigned int *retcount,
+ const char ***retcpus)
+{
+ const char *p = output;
+ const char *next;
+ unsigned int count = 0;
+ const char **cpus = NULL;
+ int i;
+ do {
+ const char *t;
+
+ if ((next = strchr(p, '\n')))
+ next++;
+
+ if (!STRPREFIX(p, "PowerPC "))
+ continue;
+
+ /* Skip the preceding sub-string "PowerPC " */
+ p += 8;
+
+ /*Malformed string, does not obey the format 'PowerPC <model>
<desc>'*/
+ if (!(t = strchr(p, ' ')) || (next && t >= next))
+ continue;
+
+ if (*p == '\0' || *p == '\n')
+ continue;
+
+ if (retcpus) {
+ unsigned int len;
+
+ if (VIR_REALLOC_N(cpus, count + 1) < 0)
+ goto error;
Need a virReportOOMError() call here
+
+ if (t)
+ len = t - p - 1;
+
+ if (!(cpus[count] = strndup(p, len)))
+ goto error;
Need a virReportOOMError() call here too
+ }
+ count++;
+ } while ((p = next));
+
+ if (retcount)
+ *retcount = count;
+ if (retcpus)
+ *retcpus = cpus;
+ return 0;
+
+error:
+ if (cpus) {
+ for (i = 0; i < count; i++)
+ VIR_FREE(cpus[i]);
+ }
+ VIR_FREE(cpus);
+ return -1;
+}
int
qemuCapsProbeCPUModels(const char *qemu,
@@ -497,6 +559,8 @@ qemuCapsProbeCPUModels(const char *qemu,
if (STREQ(arch, "i686") || STREQ(arch, "x86_64"))
parse = qemuCapsParseX86Models;
+ else if (STREQ(arch, "ppc64"))
+ parse = qemuCapsParsePPCModels;
else {
VIR_DEBUG("don't know how to parse %s CPU models", arch);
return 0;
I know we don't have any test case for the x86 CPU parser here either,
but it would be desirable to create a test case for the PPC parser
while doing this. eg, save the QEMU -cpu? output into a text file
in the tests/ directory, and invoke qemuCapsParsePPCModels() on it
and validate the result.
Regards,
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 :|
9:01 a.m.
New subject: [libvirt] [RFC v3 PATCH 4/5] PowerPC : Clean up qemuBuildCommandLine to remove x86-specific assumptions from generic code.
From: Prerna Saxena <prerna(a)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(a)linux.vnet.ibm.com>
---
src/qemu/qemu_command.c | 39 ++++++++++++++++++++++++++++++++++++---
src/qemu/qemu_command.h | 6 ++++++
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 85b34bb..57b25d6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4550,8 +4550,11 @@ qemuBuildCommandLine(virConnectPtr conn,
VIR_FREE(devstr);
virCommandAddArg(cmd, "-device");
- virCommandAddArgFormat(cmd, "isa-serial,chardev=char%s,id=%s",
- serial->info.alias, serial->info.alias);
+ if (!(devstr = qemuBuildChrDeviceStr(serial, def->os.arch,
+ def->os.machine)))
+ goto error;
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
} else {
virCommandAddArg(cmd, "-serial");
if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL)))
@@ -5438,6 +5441,34 @@ qemuBuildCommandLine(virConnectPtr conn,
return NULL;
}
+/* This function generates the correct '-device' string for character
+ * devices of each architecture.
+ */
+char *
+qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
+ char *os_arch,
+ char *machine)
+{
+ virBuffer cmd = VIR_BUFFER_INITIALIZER;
+
+ if (STREQ(os_arch, "ppc64") && STREQ(machine,
"pseries"))
+ virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s",
+ serial->info.alias);
+ else
+ virBufferAsprintf(&cmd, "isa-serial,chardev=char%s,id=%s",
+ serial->info.alias, serial->info.alias);
+
+ if (virBufferError(&cmd)) {
+ virReportOOMError();
+ goto error;
+ }
+
+ return virBufferContentAndReset(&cmd);
+
+ error:
+ virBufferFreeAndReset(&cmd);
+ return NULL;
+}
/*
* 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;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index dbe2fb2..1fe0394 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -53,6 +53,12 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
enum virNetDevVPortProfileOp vmop)
ATTRIBUTE_NONNULL(1);
+/* Generate string for arch-specific '-device' parameter */
+char *
+qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
+ char *os_arch,
+ char *machine);
+
/* With vlan == -1, use netdev syntax, else old hostnet */
char * qemuBuildHostNetStr(virDomainNetDefPtr net,
char type_sep,
--
1.7.7
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
10:57 a.m.
New subject: [libvirt] [RFC v3 PATCH 4/5] PowerPC : Clean up qemuBuildCommandLine to remove x86-specific assumptions from generic code.
On 11/29/2011 10:01 AM, Prerna Saxena wrote:
From: Prerna Saxena<prerna(a)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(a)linux.vnet.ibm.com>
---
src/qemu/qemu_command.c | 39 ++++++++++++++++++++++++++++++++++++---
src/qemu/qemu_command.h | 6 ++++++
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 85b34bb..57b25d6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4550,8 +4550,11 @@ qemuBuildCommandLine(virConnectPtr conn,
VIR_FREE(devstr);
virCommandAddArg(cmd, "-device");
- virCommandAddArgFormat(cmd,
"isa-serial,chardev=char%s,id=%s",
- serial->info.alias, serial->info.alias);
+ if (!(devstr = qemuBuildChrDeviceStr(serial, def->os.arch,
+ def->os.machine)))
+ goto error;
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
} else {
virCommandAddArg(cmd, "-serial");
if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL)))
@@ -5438,6 +5441,34 @@ qemuBuildCommandLine(virConnectPtr conn,
return NULL;
}
+/* This function generates the correct '-device' string for character
+ * devices of each architecture.
+ */
+char *
+qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
+ char *os_arch,
+ char *machine)
+{
+ virBuffer cmd = VIR_BUFFER_INITIALIZER;
+
+ if (STREQ(os_arch, "ppc64")&& STREQ(machine,
"pseries"))
+ virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s",
+ serial->info.alias);
+ else
+ virBufferAsprintf(&cmd, "isa-serial,chardev=char%s,id=%s",
+ serial->info.alias, serial->info.alias);
+
+ if (virBufferError(&cmd)) {
+ virReportOOMError();
+ goto error;
+ }
+
+ return virBufferContentAndReset(&cmd);
+
+ error:
+ virBufferFreeAndReset(&cmd);
+ return NULL;
+}
/*
* 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;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index dbe2fb2..1fe0394 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -53,6 +53,12 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
enum virNetDevVPortProfileOp vmop)
ATTRIBUTE_NONNULL(1);
+/* Generate string for arch-specific '-device' parameter */
+char *
+qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
+ char *os_arch,
+ char *machine);
Remove the space before '('.
+
/* With vlan == -1, use netdev syntax, else old hostnet */
char * qemuBuildHostNetStr(virDomainNetDefPtr net,
char type_sep,
6:22 a.m.
New subject: [libvirt] [RFC v3 PATCH 4/5] PowerPC : Clean up qemuBuildCommandLine to remove x86-specific assumptions from generic code.
On 11/29/2011 10:27 PM, Stefan Berger wrote:
On 11/29/2011 10:01 AM, Prerna Saxena wrote:
> From: Prerna Saxena<prerna(a)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(a)linux.vnet.ibm.com>
> ---
> src/qemu/qemu_command.c | 39 ++++++++++++++++++++++++++++++++++++---
> src/qemu/qemu_command.h | 6 ++++++
> 2 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 85b34bb..57b25d6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4550,8 +4550,11 @@ qemuBuildCommandLine(virConnectPtr conn,
> VIR_FREE(devstr);
>
> virCommandAddArg(cmd, "-device");
> - virCommandAddArgFormat(cmd, "isa-serial,chardev=char%s,id=%s",
> - serial->info.alias, serial->info.alias);
> + if (!(devstr = qemuBuildChrDeviceStr(serial, def->os.arch,
> + def->os.machine)))
> + goto error;
> + virCommandAddArg(cmd, devstr);
> + VIR_FREE(devstr);
> } else {
> virCommandAddArg(cmd, "-serial");
> if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL)))
> @@ -5438,6 +5441,34 @@ qemuBuildCommandLine(virConnectPtr conn,
> return NULL;
> }
>
> +/* This function generates the correct '-device' string for character
> + * devices of each architecture.
> + */
> +char *
> +qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
> + char *os_arch,
> + char *machine)
> +{
> + virBuffer cmd = VIR_BUFFER_INITIALIZER;
> +
> + if (STREQ(os_arch, "ppc64")&& STREQ(machine,
"pseries"))
> + virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s",
> + serial->info.alias);
> + else
> + virBufferAsprintf(&cmd, "isa-serial,chardev=char%s,id=%s",
> + serial->info.alias, serial->info.alias);
> +
> + if (virBufferError(&cmd)) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + return virBufferContentAndReset(&cmd);
> +
> + error:
> + virBufferFreeAndReset(&cmd);
> + return NULL;
> +}
>
> /*
> * 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;
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index dbe2fb2..1fe0394 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -53,6 +53,12 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
> enum virNetDevVPortProfileOp vmop)
> ATTRIBUTE_NONNULL(1);
>
> +/* Generate string for arch-specific '-device' parameter */
> +char *
> +qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
> + char *os_arch,
> + char *machine);
Remove the space before '('.
Thanks for pointing this out.. it had escaped 'make syntax-check' too !
I'll fix this.
> +
> /* With vlan == -1, use netdev syntax, else old hostnet */
> char * qemuBuildHostNetStr(virDomainNetDefPtr net,
> char type_sep,
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
6:40 a.m.
New subject: [libvirt] [RFC v3 PATCH 4/5] PowerPC : Clean up qemuBuildCommandLine to remove x86-specific assumptions from generic code.
On Tue, Nov 29, 2011 at 08:31:05PM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna(a)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(a)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 :|
9:03 a.m.
New subject: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
From: Michael Ellerman <michael(a)ellerman.id.au>
Date: Tue, 29 Nov 2011 13:48:09 +0530
Subject: [PATCH 5/5] Add address-type "spapr-vio" for devices based on
the same bus.
For QEMU PPC64 we have a machine type ("pseries") which
has a virtual bus called "spapr-vio". We need to be
able to create devices on this bus, and as such need a
way to specify the address for those devices.
This patch adds a new address type "spapr-vio", which achieves this.
The addressing is specified with a "reg" property in the address
definition. The reg is optional, if it is not specified QEMU will
auto-assign an address for the device.
Additionally, this patch forces an "spapr-vscsi" controller to be
selected for a guest running on architecture 'ppc64' and platform
'pseries'.
Signed-off-by: Michael Ellerman <michael(a)ellerman.id.au>
Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 9 +++++++++
src/qemu/qemu_command.c | 18 ++++++++++++++----
src/qemu/qemu_command.h | 3 ++-
src/qemu/qemu_hotplug.c | 2 +-
5 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ea99f7..5c3809e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -139,7 +139,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
"drive",
"virtio-serial",
"ccid",
- "usb")
+ "usb",
+ "spapr-vio")
VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti,
VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST,
@@ -1844,6 +1845,11 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
info->addr.usb.port);
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ if (info->addr.spaprvio.has_reg)
+ virBufferAsprintf(buf, " reg='%#llx'",
info->addr.spaprvio.reg);
+ break;
+
default:
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown address type '%d'"),
info->type);
@@ -2103,6 +2109,34 @@ cleanup:
}
static int
+virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node,
+ virDomainDeviceSpaprVioAddressPtr addr)
+{
+ char *reg;
+ int ret;
+
+ memset(addr, 0, sizeof(*addr));
+ addr->has_reg = false;
+
+ reg = virXMLPropString(node, "reg");
+ if (reg) {
+ if (virStrToLong_ull(reg, NULL, 16, &addr->reg) < 0) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'reg'
attribute"));
+ ret = -1;
+ goto cleanup;
+ }
+
+ addr->has_reg = true;
+ }
+
+ ret = 0;
+cleanup:
+ VIR_FREE(reg);
+ return ret;
+}
+
+static int
virDomainDeviceUSBMasterParseXML(xmlNodePtr node,
virDomainDeviceUSBMasterPtr master)
{
@@ -2215,6 +2249,11 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
goto cleanup;
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ if (virDomainDeviceSpaprVioAddressParseXML(address, &info->addr.spaprvio)
< 0)
+ goto cleanup;
+ break;
+
default:
/* Should not happen */
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3048,6 +3087,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
}
if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO &&
def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Controllers must use the 'pci' address
type"));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4439f55..b1f9260 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -69,6 +69,7 @@ enum virDomainDeviceAddressType {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
};
@@ -121,6 +122,13 @@ struct _virDomainDeviceUSBAddress {
char *port;
};
+typedef struct _virDomainDeviceSpaprVioAddress virDomainDeviceSpaprVioAddress;
+typedef virDomainDeviceSpaprVioAddress *virDomainDeviceSpaprVioAddressPtr;
+struct _virDomainDeviceSpaprVioAddress {
+ unsigned long long reg;
+ bool has_reg;
+};
+
enum virDomainControllerMaster {
VIR_DOMAIN_CONTROLLER_MASTER_NONE,
VIR_DOMAIN_CONTROLLER_MASTER_USB,
@@ -145,6 +153,7 @@ struct _virDomainDeviceInfo {
virDomainDeviceVirtioSerialAddress vioserial;
virDomainDeviceCcidAddress ccid;
virDomainDeviceUSBAddress usb;
+ virDomainDeviceSpaprVioAddress spaprvio;
} addr;
int mastertype;
union {
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 57b25d6..0d03fbc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1253,6 +1253,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
qemuDomainPCIAddressSetPtr addrs)
def->controllers[i]->idx == 0)
continue;
+ if (def->controllers[i]->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
+ continue;
if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
continue;
if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info)
< 0)
@@ -1403,6 +1405,9 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
virBufferAsprintf(buf, ",bus=");
qemuUsbId(buf, info->addr.usb.bus);
virBufferAsprintf(buf, ".0,port=%s", info->addr.usb.port);
+ } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) {
+ if (info->addr.spaprvio.has_reg)
+ virBufferAsprintf(buf, ",reg=%#llx", info->addr.spaprvio.reg);
}
return 0;
@@ -2087,7 +2092,8 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
}
char *
-qemuBuildControllerDevStr(virDomainControllerDefPtr def,
+qemuBuildControllerDevStr(virDomainDefPtr domainDef,
+ virDomainControllerDefPtr def,
virBitmapPtr qemuCaps,
int *nusbcontroller)
{
@@ -2095,7 +2101,11 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def,
switch (def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
- virBufferAddLit(&buf, "lsi");
+ if (STREQ(domainDef->os.arch, "ppc64") &&
STREQ(domainDef->os.machine, "pseries")) {
+ virBufferAddLit(&buf, "spapr-vscsi");
+ } else {
+ virBufferAddLit(&buf, "lsi");
+ }
virBufferAsprintf(&buf, ",id=scsi%d", def->idx);
break;
@@ -4009,7 +4019,7 @@ qemuBuildCommandLine(virConnectPtr conn,
char *devstr;
virCommandAddArg(cmd, "-device");
- if (!(devstr = qemuBuildControllerDevStr(cont, qemuCaps, NULL)))
+ if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps,
NULL)))
goto error;
virCommandAddArg(cmd, devstr);
@@ -4028,7 +4038,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArg(cmd, "-device");
char *devstr;
- if (!(devstr = qemuBuildControllerDevStr(def->controllers[i],
qemuCaps,
+ if (!(devstr = qemuBuildControllerDevStr(def, def->controllers[i],
qemuCaps,
&usbcontroller)))
goto error;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 1fe0394..3978b2b 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -95,7 +95,8 @@ char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk,
char * qemuBuildFSDevStr(virDomainFSDefPtr fs,
virBitmapPtr qemuCaps);
/* Current, best practice */
-char * qemuBuildControllerDevStr(virDomainControllerDefPtr def,
+char * qemuBuildControllerDevStr(virDomainDefPtr domainDef,
+ virDomainControllerDefPtr def,
virBitmapPtr qemuCaps,
int *nusbcontroller);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 96c0070..eabfeaa 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -329,7 +329,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver,
goto cleanup;
}
- if (!(devstr = qemuBuildControllerDevStr(controller, priv->qemuCaps, NULL)))
{
+ if (!(devstr = qemuBuildControllerDevStr(vm->def, controller,
priv->qemuCaps, NULL))) {
goto cleanup;
}
}
--
1.7.7
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
11:16 a.m.
New subject: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
On 11/29/2011 10:03 AM, Prerna Saxena wrote:
From: Michael Ellerman<michael(a)ellerman.id.au>
Date: Tue, 29 Nov 2011 13:48:09 +0530
Subject: [PATCH 5/5] Add address-type "spapr-vio" for devices based on
the same bus.
For QEMU PPC64 we have a machine type ("pseries") which
has a virtual bus called "spapr-vio". We need to be
able to create devices on this bus, and as such need a
way to specify the address for those devices.
This patch adds a new address type "spapr-vio", which achieves this.
The addressing is specified with a "reg" property in the address
definition. The reg is optional, if it is not specified QEMU will
auto-assign an address for the device.
Additionally, this patch forces an "spapr-vscsi" controller to be
selected for a guest running on architecture 'ppc64' and platform
'pseries'.
Signed-off-by: Michael Ellerman<michael(a)ellerman.id.au>
Signed-off-by: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 9 +++++++++
src/qemu/qemu_command.c | 18 ++++++++++++++----
src/qemu/qemu_command.h | 3 ++-
src/qemu/qemu_hotplug.c | 2 +-
5 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ea99f7..5c3809e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -139,7 +139,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
"drive",
"virtio-serial",
"ccid",
- "usb")
+ "usb",
+ "spapr-vio")
VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti,
VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST,
@@ -1844,6 +1845,11 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
info->addr.usb.port);
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ if (info->addr.spaprvio.has_reg)
+ virBufferAsprintf(buf, " reg='%#llx'",
info->addr.spaprvio.reg);
+ break;
+
default:
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown address type '%d'"),
info->type);
@@ -2103,6 +2109,34 @@ cleanup:
}
static int
+virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node,
+ virDomainDeviceSpaprVioAddressPtr addr)
+{
+ char *reg;
+ int ret;
+
+ memset(addr, 0, sizeof(*addr));
+ addr->has_reg = false;
+
+ reg = virXMLPropString(node, "reg");
+ if (reg) {
+ if (virStrToLong_ull(reg, NULL, 16,&addr->reg)< 0) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse<address> 'reg'
attribute"));
+ ret = -1;
+ goto cleanup;
+ }
+
+ addr->has_reg = true;
+ }
+
+ ret = 0;
+cleanup:
+ VIR_FREE(reg);
+ return ret;
+}
+
+static int
virDomainDeviceUSBMasterParseXML(xmlNodePtr node,
virDomainDeviceUSBMasterPtr master)
{
@@ -2215,6 +2249,11 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
goto cleanup;
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ if
(virDomainDeviceSpaprVioAddressParseXML(address,&info->addr.spaprvio)< 0)
+ goto cleanup;
+ break;
+
default:
/* Should not happen */
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3048,6 +3087,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
}
if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE&&
+ def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO&&
def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Controllers must use the 'pci' address
type"));
Is this still the proper error message for this type of bus? You may
want to look at def->os.arch / def->os.machine to see what is expected
here in terms of bus and then have a more specific error message.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4439f55..b1f9260 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -69,6 +69,7 @@ enum virDomainDeviceAddressType {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
};
@@ -121,6 +122,13 @@ struct _virDomainDeviceUSBAddress {
char *port;
};
+typedef struct _virDomainDeviceSpaprVioAddress virDomainDeviceSpaprVioAddress;
+typedef virDomainDeviceSpaprVioAddress *virDomainDeviceSpaprVioAddressPtr;
+struct _virDomainDeviceSpaprVioAddress {
+ unsigned long long reg;
+ bool has_reg;
+};
+
enum virDomainControllerMaster {
VIR_DOMAIN_CONTROLLER_MASTER_NONE,
VIR_DOMAIN_CONTROLLER_MASTER_USB,
@@ -145,6 +153,7 @@ struct _virDomainDeviceInfo {
virDomainDeviceVirtioSerialAddress vioserial;
virDomainDeviceCcidAddress ccid;
virDomainDeviceUSBAddress usb;
+ virDomainDeviceSpaprVioAddress spaprvio;
} addr;
int mastertype;
union {
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 57b25d6..0d03fbc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1253,6 +1253,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
qemuDomainPCIAddressSetPtr addrs)
def->controllers[i]->idx == 0)
continue;
+ if (def->controllers[i]->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
+ continue;
if (def->controllers[i]->info.type !=
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
continue;
if
(qemuDomainPCIAddressSetNextAddr(addrs,&def->controllers[i]->info)< 0)
@@ -1403,6 +1405,9 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
virBufferAsprintf(buf, ",bus=");
qemuUsbId(buf, info->addr.usb.bus);
virBufferAsprintf(buf, ".0,port=%s", info->addr.usb.port);
+ } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) {
+ if (info->addr.spaprvio.has_reg)
+ virBufferAsprintf(buf, ",reg=%#llx", info->addr.spaprvio.reg);
}
return 0;
@@ -2087,7 +2092,8 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
}
char *
-qemuBuildControllerDevStr(virDomainControllerDefPtr def,
+qemuBuildControllerDevStr(virDomainDefPtr domainDef,
+ virDomainControllerDefPtr def,
virBitmapPtr qemuCaps,
int *nusbcontroller)
{
@@ -2095,7 +2101,11 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def,
switch (def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
- virBufferAddLit(&buf, "lsi");
+ if (STREQ(domainDef->os.arch, "ppc64")&&
STREQ(domainDef->os.machine, "pseries")) {
+ virBufferAddLit(&buf, "spapr-vscsi");
+ } else {
+ virBufferAddLit(&buf, "lsi");
+ }
virBufferAsprintf(&buf, ",id=scsi%d", def->idx);
break;
@@ -4009,7 +4019,7 @@ qemuBuildCommandLine(virConnectPtr conn,
char *devstr;
virCommandAddArg(cmd, "-device");
- if (!(devstr = qemuBuildControllerDevStr(cont, qemuCaps, NULL)))
+ if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps,
NULL)))
goto error;
virCommandAddArg(cmd, devstr);
@@ -4028,7 +4038,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArg(cmd, "-device");
char *devstr;
- if (!(devstr = qemuBuildControllerDevStr(def->controllers[i],
qemuCaps,
+ if (!(devstr = qemuBuildControllerDevStr(def, def->controllers[i],
qemuCaps,
&usbcontroller)))
goto error;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 1fe0394..3978b2b 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -95,7 +95,8 @@ char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk,
char * qemuBuildFSDevStr(virDomainFSDefPtr fs,
virBitmapPtr qemuCaps);
/* Current, best practice */
-char * qemuBuildControllerDevStr(virDomainControllerDefPtr def,
+char * qemuBuildControllerDevStr(virDomainDefPtr domainDef,
+ virDomainControllerDefPtr def,
virBitmapPtr qemuCaps,
int *nusbcontroller);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 96c0070..eabfeaa 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -329,7 +329,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver,
goto cleanup;
}
- if (!(devstr = qemuBuildControllerDevStr(controller, priv->qemuCaps, NULL)))
{
+ if (!(devstr = qemuBuildControllerDevStr(vm->def, controller,
priv->qemuCaps, NULL))) {
goto cleanup;
}
}
Otherwise looks good to me.
6:23 a.m.
New subject: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
On 11/29/2011 10:46 PM, Stefan Berger wrote:
On 11/29/2011 10:03 AM, Prerna Saxena wrote:
> From: Michael Ellerman<michael(a)ellerman.id.au>
> Date: Tue, 29 Nov 2011 13:48:09 +0530
> Subject: [PATCH 5/5] Add address-type "spapr-vio" for devices based on
> the same bus.
>
> For QEMU PPC64 we have a machine type ("pseries") which
> has a virtual bus called "spapr-vio". We need to be
> able to create devices on this bus, and as such need a
> way to specify the address for those devices.
>
> This patch adds a new address type "spapr-vio", which achieves this.
>
> The addressing is specified with a "reg" property in the address
> definition. The reg is optional, if it is not specified QEMU will
> auto-assign an address for the device.
>
> Additionally, this patch forces an "spapr-vscsi" controller to be
> selected for a guest running on architecture 'ppc64' and platform
> 'pseries'.
>
> Signed-off-by: Michael Ellerman<michael(a)ellerman.id.au>
> Signed-off-by: Prerna Saxena<prerna(a)linux.vnet.ibm.com>
> ---
> src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 9 +++++++++
> src/qemu/qemu_command.c | 18 ++++++++++++++----
> src/qemu/qemu_command.h | 3 ++-
> src/qemu/qemu_hotplug.c | 2 +-
> 5 files changed, 67 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3ea99f7..5c3809e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -139,7 +139,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress,
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
> "drive",
> "virtio-serial",
> "ccid",
> - "usb")
> + "usb",
> + "spapr-vio")
>
> VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti,
> VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST,
> @@ -1844,6 +1845,11 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
> info->addr.usb.port);
> break;
>
> + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
> + if (info->addr.spaprvio.has_reg)
> + virBufferAsprintf(buf, " reg='%#llx'",
info->addr.spaprvio.reg);
> + break;
> +
> default:
> virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> _("unknown address type '%d'"), info->type);
> @@ -2103,6 +2109,34 @@ cleanup:
> }
>
> static int
> +virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node,
> + virDomainDeviceSpaprVioAddressPtr addr)
> +{
> + char *reg;
> + int ret;
> +
> + memset(addr, 0, sizeof(*addr));
> + addr->has_reg = false;
> +
> + reg = virXMLPropString(node, "reg");
> + if (reg) {
> + if (virStrToLong_ull(reg, NULL, 16,&addr->reg)< 0) {
> + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Cannot parse<address> 'reg' attribute"));
> + ret = -1;
> + goto cleanup;
> + }
> +
> + addr->has_reg = true;
> + }
> +
> + ret = 0;
> +cleanup:
> + VIR_FREE(reg);
> + return ret;
> +}
> +
> +static int
> virDomainDeviceUSBMasterParseXML(xmlNodePtr node,
> virDomainDeviceUSBMasterPtr master)
> {
> @@ -2215,6 +2249,11 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
> goto cleanup;
> break;
>
> + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
> + if
> (virDomainDeviceSpaprVioAddressParseXML(address,&info->addr.spaprvio)< 0)
> + goto cleanup;
> + break;
> +
> default:
> /* Should not happen */
> virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -3048,6 +3087,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> }
>
> if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE&&
> + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO&&
> def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Controllers must use the 'pci' address type"));
Is this still the proper error message for this type of bus? You may
want to look at def->os.arch / def->os.machine to see what is expected
here in terms of bus and then have a more specific error message.
Will do.
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 4439f55..b1f9260 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -69,6 +69,7 @@ enum virDomainDeviceAddressType {
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL,
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID,
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB,
> + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO,
>
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
> };
> @@ -121,6 +122,13 @@ struct _virDomainDeviceUSBAddress {
> char *port;
> };
>
> +typedef struct _virDomainDeviceSpaprVioAddress
> virDomainDeviceSpaprVioAddress;
> +typedef virDomainDeviceSpaprVioAddress
> *virDomainDeviceSpaprVioAddressPtr;
> +struct _virDomainDeviceSpaprVioAddress {
> + unsigned long long reg;
> + bool has_reg;
> +};
> +
> enum virDomainControllerMaster {
> VIR_DOMAIN_CONTROLLER_MASTER_NONE,
> VIR_DOMAIN_CONTROLLER_MASTER_USB,
> @@ -145,6 +153,7 @@ struct _virDomainDeviceInfo {
> virDomainDeviceVirtioSerialAddress vioserial;
> virDomainDeviceCcidAddress ccid;
> virDomainDeviceUSBAddress usb;
> + virDomainDeviceSpaprVioAddress spaprvio;
> } addr;
> int mastertype;
> union {
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 57b25d6..0d03fbc 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1253,6 +1253,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
> qemuDomainPCIAddressSetPtr addrs)
> def->controllers[i]->idx == 0)
> continue;
>
> + if (def->controllers[i]->info.type ==
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
> + continue;
> if (def->controllers[i]->info.type !=
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> continue;
> if (qemuDomainPCIAddressSetNextAddr(addrs,&def->controllers[i]->info)<
0)
> @@ -1403,6 +1405,9 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
> virBufferAsprintf(buf, ",bus=");
> qemuUsbId(buf, info->addr.usb.bus);
> virBufferAsprintf(buf, ".0,port=%s", info->addr.usb.port);
> + } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) {
> + if (info->addr.spaprvio.has_reg)
> + virBufferAsprintf(buf, ",reg=%#llx", info->addr.spaprvio.reg);
> }
>
> return 0;
> @@ -2087,7 +2092,8 @@
> qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
> }
>
> char *
> -qemuBuildControllerDevStr(virDomainControllerDefPtr def,
> +qemuBuildControllerDevStr(virDomainDefPtr domainDef,
> + virDomainControllerDefPtr def,
> virBitmapPtr qemuCaps,
> int *nusbcontroller)
> {
> @@ -2095,7 +2101,11 @@
> qemuBuildControllerDevStr(virDomainControllerDefPtr def,
>
> switch (def->type) {
> case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> - virBufferAddLit(&buf, "lsi");
> + if (STREQ(domainDef->os.arch, "ppc64")&&
> STREQ(domainDef->os.machine, "pseries")) {
> + virBufferAddLit(&buf, "spapr-vscsi");
> + } else {
> + virBufferAddLit(&buf, "lsi");
> + }
> virBufferAsprintf(&buf, ",id=scsi%d", def->idx);
> break;
>
> @@ -4009,7 +4019,7 @@ qemuBuildCommandLine(virConnectPtr conn,
> char *devstr;
>
> virCommandAddArg(cmd, "-device");
> - if (!(devstr = qemuBuildControllerDevStr(cont, qemuCaps, NULL)))
> + if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps, NULL)))
> goto error;
>
> virCommandAddArg(cmd, devstr);
> @@ -4028,7 +4038,7 @@ qemuBuildCommandLine(virConnectPtr conn,
> virCommandAddArg(cmd, "-device");
>
> char *devstr;
> - if (!(devstr = qemuBuildControllerDevStr(def->controllers[i], qemuCaps,
> + if (!(devstr = qemuBuildControllerDevStr(def, def->controllers[i],
> qemuCaps,
> &usbcontroller)))
> goto error;
>
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 1fe0394..3978b2b 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -95,7 +95,8 @@ char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk,
> char * qemuBuildFSDevStr(virDomainFSDefPtr fs,
> virBitmapPtr qemuCaps);
> /* Current, best practice */
> -char * qemuBuildControllerDevStr(virDomainControllerDefPtr def,
> +char * qemuBuildControllerDevStr(virDomainDefPtr domainDef,
> + virDomainControllerDefPtr def,
> virBitmapPtr qemuCaps,
> int *nusbcontroller);
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 96c0070..eabfeaa 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -329,7 +329,7 @@ int qemuDomainAttachPciControllerDevice(struct
> qemud_driver *driver,
> goto cleanup;
> }
>
> - if (!(devstr = qemuBuildControllerDevStr(controller, priv->qemuCaps,
> NULL))) {
> + if (!(devstr = qemuBuildControllerDevStr(vm->def, controller,
> priv->qemuCaps, NULL))) {
> goto cleanup;
> }
> }
Otherwise looks good to me.
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
8:10 p.m.
New subject: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
On Tue, 2011-11-29 at 12:16 -0500, Stefan Berger wrote:
On 11/29/2011 10:03 AM, Prerna Saxena wrote:
> @@ -3048,6 +3087,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> }
>
> if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO &&
> def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Controllers must use the 'pci'
address type"));
Is this still the proper error message for this type of bus? You may
want to look at def->os.arch / def->os.machine to see what is expected
here in terms of bus and then have a more specific error message.
No it's not the right message, which is why we're adding that check to
skip the message if the controller has an spapr-vio address. So I don't
think we need to anything else with arch/machine.
cheers
6:44 a.m.
New subject: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
On Tue, Nov 29, 2011 at 08:33:09PM +0530, Prerna Saxena wrote:
From: Michael Ellerman <michael(a)ellerman.id.au>
Date: Tue, 29 Nov 2011 13:48:09 +0530
Subject: [PATCH 5/5] Add address-type "spapr-vio" for devices based on
the same bus.
For QEMU PPC64 we have a machine type ("pseries") which
has a virtual bus called "spapr-vio". We need to be
able to create devices on this bus, and as such need a
way to specify the address for those devices.
This patch adds a new address type "spapr-vio", which achieves this.
The addressing is specified with a "reg" property in the address
definition. The reg is optional, if it is not specified QEMU will
auto-assign an address for the device.
In libvirt, we need to guarantee that addressing is stable across
QEMU releases. So if the 'reg' property is not set in the XML,
libvirt must auto-assign one itself before starting QEMU.
You'll notice we have a qemuDomainAssignPCIAddresses() API.
We will need to have some kind of equivalent for sparpr-vio
"reg" addresses. Ideally libvirt will auto-assign using the
same algorithm as QEMU would have, just for sake of sanity.
Regards,
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 :|
12:38 a.m.
New subject: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
On Wed, 2011-11-30 at 12:44 +0000, Daniel P. Berrange wrote:
On Tue, Nov 29, 2011 at 08:33:09PM +0530, Prerna Saxena wrote:
> From: Michael Ellerman <michael(a)ellerman.id.au>
> Date: Tue, 29 Nov 2011 13:48:09 +0530
> Subject: [PATCH 5/5] Add address-type "spapr-vio" for devices based on
> the same bus.
>
> For QEMU PPC64 we have a machine type ("pseries") which
> has a virtual bus called "spapr-vio". We need to be
> able to create devices on this bus, and as such need a
> way to specify the address for those devices.
>
> This patch adds a new address type "spapr-vio", which achieves this.
>
> The addressing is specified with a "reg" property in the address
> definition. The reg is optional, if it is not specified QEMU will
> auto-assign an address for the device.
In libvirt, we need to guarantee that addressing is stable across
QEMU releases. So if the 'reg' property is not set in the XML,
libvirt must auto-assign one itself before starting QEMU.
Must we really? IMHO if the user doesn't specify anything then that is
essentially asking for "the default", whatever that is.
My reasoning for putting the address assignment code in QEMU in the
first place was that it has the most knowledge and so can make the best
decision.
You'll notice we have a qemuDomainAssignPCIAddresses() API.
We will need to have some kind of equivalent for sparpr-vio
"reg" addresses. Ideally libvirt will auto-assign using the
same algorithm as QEMU would have, just for sake of sanity.
Yeah I've seen it. I'd rather not do anything similar for spapr-vio,
precisely because we will end up with identical duplicate code in
libvirt & QEMU.
But if it's a hard requirement in your view then I'll do it.
cheers
3:52 a.m.
New subject: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
On Thu, Dec 01, 2011 at 05:38:31PM +1100, Michael Ellerman wrote:
On Wed, 2011-11-30 at 12:44 +0000, Daniel P. Berrange wrote:
> On Tue, Nov 29, 2011 at 08:33:09PM +0530, Prerna Saxena wrote:
> > From: Michael Ellerman <michael(a)ellerman.id.au>
> > Date: Tue, 29 Nov 2011 13:48:09 +0530
> > Subject: [PATCH 5/5] Add address-type "spapr-vio" for devices based
on
> > the same bus.
> >
> > For QEMU PPC64 we have a machine type ("pseries") which
> > has a virtual bus called "spapr-vio". We need to be
> > able to create devices on this bus, and as such need a
> > way to specify the address for those devices.
> >
> > This patch adds a new address type "spapr-vio", which achieves this.
> >
> > The addressing is specified with a "reg" property in the address
> > definition. The reg is optional, if it is not specified QEMU will
> > auto-assign an address for the device.
>
> In libvirt, we need to guarantee that addressing is stable across
> QEMU releases. So if the 'reg' property is not set in the XML,
> libvirt must auto-assign one itself before starting QEMU.
Must we really? IMHO if the user doesn't specify anything then that is
essentially asking for "the default", whatever that is.
My reasoning for putting the address assignment code in QEMU in the
first place was that it has the most knowledge and so can make the best
decision.
> You'll notice we have a qemuDomainAssignPCIAddresses() API.
> We will need to have some kind of equivalent for sparpr-vio
> "reg" addresses. Ideally libvirt will auto-assign using the
> same algorithm as QEMU would have, just for sake of sanity.
Yeah I've seen it. I'd rather not do anything similar for spapr-vio,
precisely because we will end up with identical duplicate code in
libvirt & QEMU.
But if it's a hard requirement in your view then I'll do it.
It is the only way to ensure stable device addresses every time the
guest is started.
eg, consider what happens if you don't assign addresses
qemu -drive foo -drive bar
QEMU assigns foo reg=1, bar reg=2. Now delete hotunplug a device
(qemu) drive_del foo
Now migrate QEMU to a new host, which means it'll be started with
qemu -drive bar
So QEMU will assign bar reg=1, whereas the guest OS currently
thinks it has reg=2.
libvirt *must* define addresses for every single device it passes to
QEMU at all times. Even if we ignore hotplug, and just consider that
we cold unpluged 'foo', we still want 'bar' to have the same address,
in case the guest OS had configured something based on address, or
worse yet done an OS license/activation check based on hardware config.
Regards,
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 :|
1:34 a.m.
New subject: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
On Thu, 2011-12-01 at 09:52 +0000, Daniel P. Berrange wrote:
On Thu, Dec 01, 2011 at 05:38:31PM +1100, Michael Ellerman wrote:
> But if it's a hard requirement in your view then I'll do it.
It is the only way to ensure stable device addresses every time the
guest is started.
eg, consider what happens if you don't assign addresses
qemu -drive foo -drive bar
QEMU assigns foo reg=1, bar reg=2. Now delete hotunplug a device
(qemu) drive_del foo
Now migrate QEMU to a new host, which means it'll be started with
qemu -drive bar
So QEMU will assign bar reg=1, whereas the guest OS currently
thinks it has reg=2.
libvirt *must* define addresses for every single device it passes to
QEMU at all times. Even if we ignore hotplug, and just consider that
we cold unpluged 'foo', we still want 'bar' to have the same address,
in case the guest OS had configured something based on address, or
worse yet done an OS license/activation check based on hardware config.
OK. Currently I don't think either of those examples actually apply to
us, but I can see that it is liable to bite us in the future if we don't
assign addresses.
Below is a first cut at a patch to do this. It seems to work but it has
a few problems.
Firstly there are calls in qemuDomainUpdateDeviceConfig()
to qemuDomainAssignPCIAddresses(), I'm not sure if I need to also add
calls to qemuAssignSpaprVIOAddresses() in there. Can you explain to me
when qemuDomainUpdateDeviceConfig() is called and what it is doing?
Secondly, and this is a real problem, if the user specifies a serial
with no address type, we don't see it, because it doesn't have a
spapr-vio address, and so we don't take it into account in the address
allocation. But then because it ends becoming a spapr-vty in
qemuBuildChrDeviceStr() (which Prerna added), it clashes with the other
serial we created.
Example config is:
<serial type="pty"/>
<serial type="pty">
<address type="spapr-vio"/>
</serial>
So we need some way of knowing that the first serial will end up on the
spapr-vio bus, so we can take it into account in the address allocation.
I haven't thought of a good way to fix that yet. We could just say that
it's unsupported for pseries, ie. you MUST specify the address type, but
that would be a pity.
cheers
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index eceae35..dfd92ad 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -684,6 +684,63 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps)
return -1;
}
+static int qemuSpaprVIOFindByReg(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info, void *opaque)
+{
+ virDomainDeviceInfoPtr target = (virDomainDeviceInfoPtr)opaque;
+
+ if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
+ return 0;
+
+ /* Match a dev that has a reg, is not us, and has a matching reg */
+ if (info->addr.spaprvio.has_reg && info != target &&
+ info->addr.spaprvio.reg == target->addr.spaprvio.reg)
+ /* Has to be < 0 so virDomainDeviceInfoIterate() will exit */
+ return -1;
+
+ return 0;
+}
+
+static void qemuAssignSpaprVIOAddress(virDomainDefPtr def,
+ virDomainDeviceInfoPtr info,
+ unsigned long long default_reg)
+{
+ int rc;
+
+ if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
+ return;
+
+ if (info->addr.spaprvio.has_reg)
+ return;
+
+ info->addr.spaprvio.has_reg = true;
+ info->addr.spaprvio.reg = default_reg;
+
+ rc = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info);
+ while (rc != 0) {
+ /* We hit another device, move along by 4K and search again */
+ info->addr.spaprvio.reg += 0x1000;
+ rc = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info);
+ }
+}
+
+int qemuAssignSpaprVIOAddresses(virDomainDefPtr def)
+{
+ int i;
+
+ for (i = 0 ; i < def->nnets; i++)
+ qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, 0x1000ul);
+
+ for (i = 0 ; i < def->ncontrollers; i++)
+ qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, 0x2000ul);
+
+ for (i = 0 ; i < def->nserials; i++)
+ qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, 0x30000000ul);
+
+ /* No other devices are currently supported on spapr-vio */
+
+ return 0;
+}
#define QEMU_PCI_ADDRESS_LAST_SLOT 31
#define QEMU_PCI_ADDRESS_LAST_FUNCTION 8
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 8b51ef3..f748c2b 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -203,6 +203,8 @@ int qemuAssignDeviceHostdevAlias(virDomainDefPtr def,
virDomainHostdevDefPtr hos
int qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller);
int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev,
int idx);
+int qemuAssignSpaprVIOAddresses(virDomainDefPtr def);
+
int
qemuParseKeywords(const char *str,
char ***retkeywords,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 105bdde..6c47516 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1317,6 +1317,8 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char
*xml,
if (qemuDomainAssignPCIAddresses(def) < 0)
goto cleanup;
+ qemuAssignSpaprVIOAddresses(def);
+
if (!(vm = virDomainAssignDef(driver->caps,
&driver->domains,
def, false)))
@@ -4903,6 +4905,8 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char
*xml) {
if (qemuDomainAssignPCIAddresses(def) < 0)
goto cleanup;
+ qemuAssignSpaprVIOAddresses(def);
+
if (!(vm = virDomainAssignDef(driver->caps,
&driver->domains,
def, false))) {
@@ -10748,6 +10752,8 @@ static virDomainPtr qemuDomainAttach(virConnectPtr conn,
if (qemuDomainAssignPCIAddresses(def) < 0)
goto cleanup;
+ qemuAssignSpaprVIOAddresses(def);
+
if (!(vm = virDomainAssignDef(driver->caps,
&driver->domains,
def, false)))
--
1.7.7.3
10:39 a.m.
On 11/29/2011 09:55 AM, Prerna Saxena wrote:
Recent development in KVM for 64-bit Power ISA Book3S machines,
allows
users to run multiple KVM guest instances on POWER7 and PPC970
processor based systems. Also qemu-system-ppc64 has been enhanced to
support a new machine type "pseries" suitable for Power Book3S machines.
This addition effectively brings the KVM+qemu combination to run
multiple guest instances on a Power Book3S machine.
I applied this patch series on an x86 machine and ran 'make check'. I
now have two tests failing due to it not making use of the proc
filesystem entries anymore...
TEST: nodeinfotest
Expect 39 'CPUs: 2, MHz: 2800, Nodes: 1, Cores: 2
'
Actual 39 'CPUs: 8, MHz: 2800, Nodes: 1, Cores: 4
'
!Expect 39 'CPUs: 2, MHz: 2211, Nodes: 1, Cores: 2
'
Actual 39 'CPUs: 8, MHz: 2211, Nodes: 1, Cores: 4
'
!Expect 39 'CPUs: 4, MHz: 1595, Nodes: 1, Cores: 2
'
Actual 39 'CPUs: 8, MHz: 1595, Nodes: 1, Cores: 4
'
!Expect 39 'CPUs: 4, MHz: 1000, Nodes: 1, Cores: 4
'
Actual 39 'CPUs: 8, MHz: 1000, Nodes: 1, Cores: 4
'
!Expect 39 'CPUs: 4, MHz: 2814, Nodes: 1, Cores: 2
'
Actual 39 'CPUs: 8, MHz: 2814, Nodes: 1, Cores: 4
'
!Expect 39 'CPUs: 4, MHz: 1000, Nodes: 1, Cores: 2
'
Actual 39 'CPUs: 8, MHz: 1000, Nodes: 1, Cores: 4
'
! 6 FAIL
FAIL: nodeinfotest
[...]
TEST: qemuargv2xmltest
/bin/sh: line 5: 875 Segmentation fault (core dumped)
abs_top_builddir=`cd '..'; pwd` abs_top_srcdir=`cd '..'; pwd`
abs_builddir=`pwd` abs_srcdir=`cd '.'; pwd` CONFIG_HEADER="`cd '..';
pwd`/config.h" PATH="`cd '..'; pwd`/daemon:`cd '..';
pwd`/tools:`cd
'..'; pwd`/tests:$PATH" SHELL="/bin/sh"
LIBVIRT_DRIVER_DIR="/root/tmp/libvirt-acl/src/.libs" LC_ALL=C ${dir}$tst
FAIL: qemuargv2xmltest
I had 'export DEBUG_TESTS=1' set to see the debugging info on the
nodeinfo test.
The qemuargv2xmltest fails due to the following:
0x00000038ec32e334 in __strcmp_ssse3 () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install
augeas-libs-0.9.0-1.fc14.x86_64 avahi-libs-0.6.27-8.fc14.x86_64
cyrus-sasl-lib-2.1.23-12.fc14.x86_64 dbus-libs-1.4.0-3.fc14.x86_64
device-mapper-libs-1.02.63-2.fc14.x86_64 glibc-2.13-2.x86_64
gnutls-2.8.6-2.fc14.x86_64 keyutils-libs-1.2-6.fc12.x86_64
krb5-libs-1.8.4-2.fc14.x86_64 libcap-ng-0.6.5-1.fc14.x86_64
libcom_err-1.41.12-6.fc14.x86_64 libcurl-7.21.0-10.fc14.x86_64
libgcc-4.5.1-4.fc14.x86_64 libgcrypt-1.4.5-4.fc13.x86_64
libgpg-error-1.9-1.fc14.x86_64 libidn-1.18-1.fc14.x86_64
libpcap-1.1.1-3.fc14.x86_64 libselinux-2.0.96-6.fc14.1.x86_64
libsepol-2.0.41-3.fc14.x86_64 libssh2-1.2.4-1.fc14.x86_64
libtasn1-2.7-1.fc14.x86_64 libudev-161-10.fc14.x86_64
libxml2-2.7.7-3.fc14.x86_64 libxslt-1.1.26-3.fc14.x86_64
netcf-libs-0.1.9-1.fc14.x86_64 nspr-4.8.8-1.fc14.x86_64
nss-3.12.10-4.fc14.x86_64 nss-softokn-freebl-3.12.10-1.fc14.x86_64
nss-util-3.12.10-1.fc14.x86_64 numactl-2.0.3-8.fc13.x86_64
openldap-2.4.23-10.fc14.x86_64 openssl-1.0.0e-1.fc14.x86_64
xen-libs-4.0.2-1.fc14.x86_64 yajl-1.0.7-3.fc13.x86_64
zlib-1.2.5-2.fc14.x86_64
(gdb) up
#1 0x000000000042d56c in qemuParseCommandLine (caps=0x9b94a0,
progenv=0x9badc0, progargv=0x9bae00, pidfile=0x0, monConfig=0x0,
monJSON=0x0)
at qemu/qemu_command.c:6694
6694 if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch,
"x86_64"))
(gdb) print def->os
$1 = {type = 0x0, arch = 0x0, machine = 0x0, nBootDevs = 0, bootDevs =
{0, 0, 0, 0}, bootmenu = 0, init = 0x0, kernel = 0x0, initrd = 0x0,
cmdline = 0x0, root = 0x0, loader = 0x0, bootloader = 0x0,
bootloaderArgs = 0x0, smbios_mode = 0, bios = {useserial = 0}}
I looked on a RHEL 5 machine (i686). The sysfs at least is there.
Stefan
Libvirt continues to be the key interface to configure and manage
the
KVM guest instances on x86. This patch set is an effort to enable
libvirt to support KVM guest configuration and management on Power Book3S
machines.
Based on community discussion around the earlier version, this patch
series augments the present 'kvm' driver to support PowerPC-KVM based
guests.Since some of the supported devices vary between architectures,
libvirt must be capable of choosing supported device backends and
defaults for each architecture in qemu.
To check if qemu supports a certain feature, libvirt at present parses
the -help string which is generated by running the qemu binary with the
'-h' argument. This approach is gated by QEMU's inherent limitation.
When generating the list of allowed options with the '-h' flags, qemu
today blindly lists all options defined for any architecture/platform
instead of doing any arch-specific checking. This tricks libvirt into
assuming a much bigger set of host capabilities than is actually
available.
Ideally, it would be good to have qemu specify a list of devices for a
given architecture and platform which libvirt can parse to understand
supported capabilities for that guest.
As a part of this patchset, there is an attempt to cleanly bifurcate
libvirt code and to remove x86-specific assumptions from generic qemu
commandline code.
Series Description:
-------------------
This patch series consists of 5 patches :
Patch 1/5 : Use sysfs to gather host topology in place of /proc/cpuinfo.
Patch 2/5 : Add PowerPC CPU Driver
Patch 3/5 : Add support for qemu-system-ppc64
Patch 4/5 : Clean up x86-specific assumptions from generic qemu code.
Patch 5/5 : Add address family "spapr-vio"
Changelog:
---------
** v1->v2 :
* Patches 1,2,3 unchanged ; The hacks in Patch 4 of v1 replaced by a
new patch to neatly select arch-specific features.
** v2->v3 :
* Patches 1,2,3 have minor cleanups ; Patch 4 no longer has an
arch-specific handler routine. It is now replaced by a much simpler
patch that merely removes x86/pc-specific assumptions from libvirt.
** v3->v4 :
* Patches 1,2,3,4 unchanged ; patch 5 is a new addition from Michael Ellerman
that adds a new device-tree based addressing mechanism for the 'pseries'
guest.
4739
days inactive
4742
days old
24 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Michael Ellerman -
Prerna Saxena -
Stefan Berger