[libvirt] [PATCH 0/6] Use enum for tracking valid architectures

Different operating systems have different names for architectures. eg x86_64 x64, amd64. This can cause inconsistency in libvirt code dealing with architectures. To deal with this define an enum for valid architectures and a canonical string mapping. Update all code internally to use the enum instead of 'char *' for arch data.

From: "Daniel P. Berrange" <berrange@redhat.com> Introduce a 'virArch' enum for CPU architectures. Include data type providing wordsize and endianness, and APIs to query this info and convert to/from enum and string form. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/Makefile.am | 1 + src/util/virarch.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virarch.h | 81 ++++++++++++++++++++++++ 3 files changed, 259 insertions(+) create mode 100644 src/util/virarch.c create mode 100644 src/util/virarch.h diff --git a/src/Makefile.am b/src/Makefile.am index 1a2f94f..4edfb7f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -79,6 +79,7 @@ UTIL_SOURCES = \ util/threadpool.c util/threadpool.h \ util/uuid.c util/uuid.h \ util/util.c util/util.h \ + util/virarch.h util/virarch.c \ util/viratomic.h util/viratomic.c \ util/viraudit.c util/viraudit.h \ util/virauth.c util/virauth.h \ diff --git a/src/util/virarch.c b/src/util/virarch.c new file mode 100644 index 0000000..8b7bec8 --- /dev/null +++ b/src/util/virarch.c @@ -0,0 +1,177 @@ +/* + * virarch.c: architecture handling + * + * Copyright (C) 2012 Red Hat, Inc. + * + * 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, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include <sys/utsname.h> + +#include "logging.h" +#include "virarch.h" +#include "verify.h" + +/* The canonical names are used in XML documents. ie ABI sensitive */ +static const struct virArchData { + const char *name; + unsigned int wordsize; + virArchEndian endian; +} virArchData[] = { + { "", 0, VIR_ARCH_LITTLE_ENDIAN }, + { "alpha", 64, VIR_ARCH_BIG_ENDIAN }, + { "armv7l", 32, VIR_ARCH_LITTLE_ENDIAN }, + { "cris", 32, VIR_ARCH_LITTLE_ENDIAN }, + { "i686", 32, VIR_ARCH_LITTLE_ENDIAN }, + + { "itanium", 64, VIR_ARCH_LITTLE_ENDIAN }, + { "lm32", 32, VIR_ARCH_BIG_ENDIAN }, + { "m68k", 32, VIR_ARCH_BIG_ENDIAN }, + { "microblaze", 32, VIR_ARCH_BIG_ENDIAN }, + { "microblazeel", 32, VIR_ARCH_LITTLE_ENDIAN}, + + { "mips", 32, VIR_ARCH_BIG_ENDIAN }, + { "mipsel", 32, VIR_ARCH_LITTLE_ENDIAN }, + { "mips64", 64, VIR_ARCH_BIG_ENDIAN }, + { "mips64el", 64, VIR_ARCH_LITTLE_ENDIAN }, + { "openrisc", 32, VIR_ARCH_BIG_ENDIAN }, + + { "parisc", 32, VIR_ARCH_BIG_ENDIAN }, + { "parisc64", 64, VIR_ARCH_BIG_ENDIAN }, + { "ppc", 32, VIR_ARCH_BIG_ENDIAN }, + { "ppc64", 64, VIR_ARCH_BIG_ENDIAN }, + { "ppcemb", 32, VIR_ARCH_BIG_ENDIAN }, + + { "s390", 32, VIR_ARCH_BIG_ENDIAN }, + { "s390x", 64, VIR_ARCH_BIG_ENDIAN }, + { "sh4", 32, VIR_ARCH_LITTLE_ENDIAN }, + { "sh4eb", 64, VIR_ARCH_BIG_ENDIAN }, + { "sparc", 32, VIR_ARCH_BIG_ENDIAN }, + + { "sparc64", 64, VIR_ARCH_BIG_ENDIAN }, + { "unicore32", 32, VIR_ARCH_LITTLE_ENDIAN }, + { "x86_64", 64, VIR_ARCH_LITTLE_ENDIAN }, + { "xtensa", 32, VIR_ARCH_LITTLE_ENDIAN }, + { "xtensaeb", 32, VIR_ARCH_BIG_ENDIAN }, +}; + +verify(ARRAY_CARDINALITY(virArchData) == VIR_ARCH_LAST); + + +/** + * virArchGetWordSize: + * @arch: the CPU architecture + * + * Return the wordsize of the CPU architecture (32 or 64) + */ +unsigned int virArchGetWordSize(virArch arch) +{ + if (arch >= VIR_ARCH_LAST) + arch = VIR_ARCH_NONE; + + return virArchData[arch].wordsize; +} + +/** + * virArchGetEndian: + * @arch: the CPU architecture + * + * Return the endian-ness of the CPU architecture + * (VIR_ARCH_LITTLE_ENDIAN or VIR_ARCH_BIG_ENDIAN) + */ +virArchEndian virArchGetEndian(virArch arch) +{ + if (arch >= VIR_ARCH_LAST) + arch = VIR_ARCH_NONE; + + return virArchData[arch].endian; +} + +/** + * virArchToString: + * @arch: the CPU architecture + * + * Return the string name of the architecture + */ +const char *virArchToString(virArch arch) +{ + if (arch >= VIR_ARCH_LAST) + arch = VIR_ARCH_NONE; + + return virArchData[arch].name; +} + + +/** + * virArchFromString: + * @archstr: the CPU architecture string + * + * Return the architecture matching @archstr, + * defaulting to VIR_ARCH_I686 if unidentified + */ +virArch virArchFromString(const char *archstr) +{ + size_t i; + for (i = 1 ; i < VIR_ARCH_LAST ; i++) { + if (STREQ(virArchData[i].name, archstr)) + return i; + } + + VIR_DEBUG("Unknown arch %s", archstr); + return VIR_ARCH_NONE; +} + + +/** + * virArchFromHost: + * + * Return the host architecture. Prefer this to the + * uname 'machine' field, since this will canonicalize + * architecture names like 'amd64' into 'x86_64'. + */ +virArch virArchFromHost(void) +{ + struct utsname ut; + virArch arch; + + uname(&ut); + + /* Some special cases we need to handle first + * for non-canonical names */ + if (ut.machine[0] == 'i' && + ut.machine[2] == '8' && + ut.machine[3] == '6' && + ut.machine[4] == '\0') { + arch = VIR_ARCH_I686; + } else if (STREQ(ut.machine, "ia64")) { + arch = VIR_ARCH_ITANIUM; + } else if (STREQ(ut.machine, "amd64")) { + arch = VIR_ARCH_X86_64; + } else { + /* Otherwise assume the canonical name */ + if ((arch = virArchFromString(ut.machine)) == VIR_ARCH_NONE) { + VIR_WARN("Unknown host arch %s, report to libvir-list@redhat.com", + ut.machine); + } + } + + VIR_DEBUG("Mapped %s to %d (%s)", + ut.machine, arch, virArchToString(arch)); + + return arch; +} diff --git a/src/util/virarch.h b/src/util/virarch.h new file mode 100644 index 0000000..d29d7ef --- /dev/null +++ b/src/util/virarch.h @@ -0,0 +1,81 @@ +/* + * virarch.h: architecture handling + * + * Copyright (C) 2012 Red Hat, Inc. + * + * 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, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __VIR_ARCH_H__ +# define __VIR_ARCH_H__ + +# include "internal.h" +# include "util.h" + +typedef enum { + VIR_ARCH_NONE, + VIR_ARCH_ALPHA, /* Alpha 64 BE http://en.wikipedia.org/wiki/DEC_Alpha */ + VIR_ARCH_ARMV7L, /* ARMv7 32 LE http://en.wikipedia.org/wiki/ARM_architecture */ + VIR_ARCH_CRIS, /* ETRAX 32 LE http://en.wikipedia.org/wiki/ETRAX_CRIS */ + VIR_ARCH_I686, /* x86 32 LE http://en.wikipedia.org/wiki/X86 */ + + VIR_ARCH_ITANIUM, /* Itanium 64 LE http://en.wikipedia.org/wiki/Itanium */ + VIR_ARCH_LM32, /* MilkyMist 32 BE http://en.wikipedia.org/wiki/Milkymist */ + VIR_ARCH_M68K, /* m68k 32 BE http://en.wikipedia.org/wiki/Motorola_68000_family */ + VIR_ARCH_MICROBLAZE, /* Microblaze 32 BE http://en.wikipedia.org/wiki/MicroBlaze */ + VIR_ARCH_MICROBLAZEEL, /* Microblaze 32 LE http://en.wikipedia.org/wiki/MicroBlaze */ + + VIR_ARCH_MIPS, /* MIPS 32 BE http://en.wikipedia.org/wiki/MIPS_architecture */ + VIR_ARCH_MIPSEL, /* MIPS 32 LE http://en.wikipedia.org/wiki/MIPS_architecture */ + VIR_ARCH_MIPS64, /* MIPS 64 BE http://en.wikipedia.org/wiki/MIPS_architecture */ + VIR_ARCH_MIPS64EL, /* MIPS 64 LE http://en.wikipedia.org/wiki/MIPS_architecture */ + VIR_ARCH_OR32, /* OpenRisc 32 BE http://en.wikipedia.org/wiki/OpenRISC#QEMU_support*/ + + VIR_ARCH_PARISC, /* PA-Risc 32 BE http://en.wikipedia.org/wiki/PA-RISC */ + VIR_ARCH_PARISC64, /* PA-Risc 64 BE http://en.wikipedia.org/wiki/PA-RISC */ + VIR_ARCH_PPC, /* PowerPC 32 BE http://en.wikipedia.org/wiki/PowerPC */ + VIR_ARCH_PPC64, /* PowerPC 64 BE http://en.wikipedia.org/wiki/PowerPC */ + VIR_ARCH_PPCEMB, /* PowerPC 32 BE http://en.wikipedia.org/wiki/PowerPC */ + + VIR_ARCH_S390, /* S390 32 BE http://en.wikipedia.org/wiki/S390 */ + VIR_ARCH_S390X, /* S390 64 BE http://en.wikipedia.org/wiki/S390x */ + VIR_ARCH_SH4, /* SuperH4 32 LE http://en.wikipedia.org/wiki/SuperH */ + VIR_ARCH_SH4EB, /* SuperH4 32 BE http://en.wikipedia.org/wiki/SuperH */ + VIR_ARCH_SPARC, /* Sparc 32 BE http://en.wikipedia.org/wiki/Sparc */ + + VIR_ARCH_SPARC64, /* Sparc 64 BE http://en.wikipedia.org/wiki/Sparc */ + VIR_ARCH_UNICORE32, /* UniCore 32 LE http://en.wikipedia.org/wiki/Unicore*/ + VIR_ARCH_X86_64, /* x86 64 LE http://en.wikipedia.org/wiki/X86 */ + VIR_ARCH_XTENSA, /* XTensa 32 LE http://en.wikipedia.org/wiki/Xtensa#Processor_Cores */ + VIR_ARCH_XTENSAEB, /* XTensa 32 BE http://en.wikipedia.org/wiki/Xtensa#Processor_Cores */ + + VIR_ARCH_LAST, +} virArch; + + +typedef enum { + VIR_ARCH_LITTLE_ENDIAN, + VIR_ARCH_BIG_ENDIAN, +} virArchEndian; + +unsigned int virArchGetWordSize(virArch arch); +virArchEndian virArchGetEndian(virArch arch); +const char *virArchToString(virArch arch); +virArch virArchFromString(const char *name); + +virArch virArchFromHost(void); + +#endif /* __VIR_ARCH_H__ */ -- 1.7.11.7

On 12/11/2012 07:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a 'virArch' enum for CPU architectures. Include data type providing wordsize and endianness, and APIs to query this info and convert to/from enum and string form.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- +/** + * virArchFromString: + * @archstr: the CPU architecture string + * + * Return the architecture matching @archstr, + * defaulting to VIR_ARCH_I686 if unidentified
Really?
+ */ +virArch virArchFromString(const char *archstr) +{ + size_t i; + for (i = 1 ; i < VIR_ARCH_LAST ; i++) { + if (STREQ(virArchData[i].name, archstr)) + return i; + } + + VIR_DEBUG("Unknown arch %s", archstr); + return VIR_ARCH_NONE;
Looks like you return NONE instead. Other than the mixed-up comment, looks good to go after 1.0.1. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Dec 11, 2012 at 14:53:36 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a 'virArch' enum for CPU architectures. Include data type providing wordsize and endianness, and APIs to query this info and convert to/from enum and string form. ... diff --git a/src/util/virarch.c b/src/util/virarch.c new file mode 100644 index 0000000..8b7bec8 --- /dev/null +++ b/src/util/virarch.c @@ -0,0 +1,177 @@ ... +/** + * virArchFromString: + * @archstr: the CPU architecture string + * + * Return the architecture matching @archstr, + * defaulting to VIR_ARCH_I686 if unidentified
This should be VIR_ARCH_NONE as Eric already noted.
+ */ +virArch virArchFromString(const char *archstr) +{ + size_t i; + for (i = 1 ; i < VIR_ARCH_LAST ; i++) { + if (STREQ(virArchData[i].name, archstr)) + return i; + } + + VIR_DEBUG("Unknown arch %s", archstr); + return VIR_ARCH_NONE; +} + + +/** + * virArchFromHost: + * + * Return the host architecture. Prefer this to the + * uname 'machine' field, since this will canonicalize + * architecture names like 'amd64' into 'x86_64'. + */ +virArch virArchFromHost(void) +{ + struct utsname ut; + virArch arch; + + uname(&ut); + + /* Some special cases we need to handle first + * for non-canonical names */ + if (ut.machine[0] == 'i' && + ut.machine[2] == '8' && + ut.machine[3] == '6' && + ut.machine[4] == '\0') { + arch = VIR_ARCH_I686;
This could access undefined memory in the unlikely case of ut.machine being just "i". Insert the ut.machine[1] != '\0' test to match the original code in qemu_command.c.
+ } else if (STREQ(ut.machine, "ia64")) { + arch = VIR_ARCH_ITANIUM; + } else if (STREQ(ut.machine, "amd64")) { + arch = VIR_ARCH_X86_64; + } else { + /* Otherwise assume the canonical name */ + if ((arch = virArchFromString(ut.machine)) == VIR_ARCH_NONE) { + VIR_WARN("Unknown host arch %s, report to libvir-list@redhat.com", + ut.machine); + } + } + + VIR_DEBUG("Mapped %s to %d (%s)", + ut.machine, arch, virArchToString(arch)); + + return arch; +} diff --git a/src/util/virarch.h b/src/util/virarch.h new file mode 100644 index 0000000..d29d7ef --- /dev/null +++ b/src/util/virarch.h @@ -0,0 +1,81 @@ ... +#ifndef __VIR_ARCH_H__ +# define __VIR_ARCH_H__ + +# include "internal.h" +# include "util.h"
Looks like nothing from util.h is used in this header file. ... ACK with the nits fixed. Jirka

On Tue, Dec 18, 2012 at 10:24:06AM +0100, Jiri Denemark wrote:
On Tue, Dec 11, 2012 at 14:53:36 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a 'virArch' enum for CPU architectures. Include data type providing wordsize and endianness, and APIs to query this info and convert to/from enum and string form. ... diff --git a/src/util/virarch.c b/src/util/virarch.c new file mode 100644 index 0000000..8b7bec8 --- /dev/null +++ b/src/util/virarch.c @@ -0,0 +1,177 @@ ... +/** + * virArchFromString: + * @archstr: the CPU architecture string + * + * Return the architecture matching @archstr, + * defaulting to VIR_ARCH_I686 if unidentified
This should be VIR_ARCH_NONE as Eric already noted.
+ */ +virArch virArchFromString(const char *archstr) +{ + size_t i; + for (i = 1 ; i < VIR_ARCH_LAST ; i++) { + if (STREQ(virArchData[i].name, archstr)) + return i; + } + + VIR_DEBUG("Unknown arch %s", archstr); + return VIR_ARCH_NONE; +} + + +/** + * virArchFromHost: + * + * Return the host architecture. Prefer this to the + * uname 'machine' field, since this will canonicalize + * architecture names like 'amd64' into 'x86_64'. + */ +virArch virArchFromHost(void) +{ + struct utsname ut; + virArch arch; + + uname(&ut); + + /* Some special cases we need to handle first + * for non-canonical names */ + if (ut.machine[0] == 'i' && + ut.machine[2] == '8' && + ut.machine[3] == '6' && + ut.machine[4] == '\0') { + arch = VIR_ARCH_I686;
This could access undefined memory in the unlikely case of ut.machine being just "i". Insert the ut.machine[1] != '\0' test to match the original code in qemu_command.c.
+ } else if (STREQ(ut.machine, "ia64")) { + arch = VIR_ARCH_ITANIUM; + } else if (STREQ(ut.machine, "amd64")) { + arch = VIR_ARCH_X86_64; + } else { + /* Otherwise assume the canonical name */ + if ((arch = virArchFromString(ut.machine)) == VIR_ARCH_NONE) { + VIR_WARN("Unknown host arch %s, report to libvir-list@redhat.com", + ut.machine); + } + } + + VIR_DEBUG("Mapped %s to %d (%s)", + ut.machine, arch, virArchToString(arch)); + + return arch; +} diff --git a/src/util/virarch.h b/src/util/virarch.h new file mode 100644 index 0000000..d29d7ef --- /dev/null +++ b/src/util/virarch.h @@ -0,0 +1,81 @@ ... +#ifndef __VIR_ARCH_H__ +# define __VIR_ARCH_H__ + +# include "internal.h" +# include "util.h"
Looks like nothing from util.h is used in this header file.
Oh true. Originally I used the VIR_ENUM stuff, but I removed that in the end Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Dec 18, 2012 at 10:24:06AM +0100, Jiri Denemark wrote:
On Tue, Dec 11, 2012 at 14:53:36 +0000, Daniel P. Berrange wrote:
+virArch virArchFromHost(void) +{ + struct utsname ut; + virArch arch; + + uname(&ut); + + /* Some special cases we need to handle first + * for non-canonical names */ + if (ut.machine[0] == 'i' && + ut.machine[2] == '8' && + ut.machine[3] == '6' && + ut.machine[4] == '\0') { + arch = VIR_ARCH_I686;
This could access undefined memory in the unlikely case of ut.machine being just "i". Insert the ut.machine[1] != '\0' test to match the original code in qemu_command.c.
I decided it is better just to add a 'strlen(ut.machine) == 4' test as the first thing in the conditional Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Dec 11, 2012 at 02:53:36PM +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a 'virArch' enum for CPU architectures. Include data type providing wordsize and endianness, and APIs to query this info and convert to/from enum and string form.
It'd be so nice if this sort of thing were more generally available as a simple library. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top

From: "Daniel P. Berrange" <berrange@redhat.com> Replace use of uname in nodeGetInfo with virArch APIs to provide canonicalization of host architecture name Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nodeinfo.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 096000b..89322a4 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -47,6 +47,7 @@ #include "virterror_internal.h" #include "count-one-bits.h" #include "intprops.h" +#include "virarch.h" #include "virfile.h" #include "virtypedparam.h" @@ -841,13 +842,11 @@ error: } #endif -int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { - struct utsname info; - - memset(nodeinfo, 0, sizeof(*nodeinfo)); - uname(&info); +int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) +{ + virArch hostarch = virArchFromHost(); - if (virStrcpyStatic(nodeinfo->model, info.machine) == NULL) + if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) == NULL) return -1; #ifdef __linux__ -- 1.7.11.7

On Tue, Dec 11, 2012 at 14:53:37 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Replace use of uname in nodeGetInfo with virArch APIs to provide canonicalization of host architecture name
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nodeinfo.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
ACK Jirka

From: "Daniel P. Berrange" <berrange@redhat.com> Use virArch APIs to determine host architecture when launching QEMU. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9009bd2..6968f74 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -32,6 +32,7 @@ #include "logging.h" #include "virterror_internal.h" #include "util.h" +#include "virarch.h" #include "virfile.h" #include "uuid.h" #include "c-ctype.h" @@ -121,21 +122,6 @@ VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "handle"); -static void -uname_normalize(struct utsname *ut) -{ - uname(ut); - - /* Map i386, i486, i586 to i686. */ - if (ut->machine[0] == 'i' && - ut->machine[1] != '\0' && - ut->machine[2] == '8' && - ut->machine[3] == '6' && - ut->machine[4] == '\0') - ut->machine[1] = '6'; -} - - /** * qemuPhysIfaceConnect: * @def: the definition of the VM (needed by 802.1Qbh and audit) @@ -4281,7 +4267,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, const virDomainDefPtr def, const char *emulator, qemuCapsPtr caps, - const struct utsname *ut, + virArch hostarch, char **opt, bool *hasHwVirt, bool migrating) @@ -4414,7 +4400,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, * 2. emulator is qemu-system-x86_64 */ if (STREQ(def->os.arch, "i686") && - ((STREQ(ut->machine, "x86_64") && + (((hostarch == VIR_ARCH_X86_64) && strstr(emulator, "kvm")) || strstr(emulator, "x86_64"))) { virBufferAdd(&buf, default_model, -1); @@ -4959,7 +4945,6 @@ qemuBuildCommandLine(virConnectPtr conn, enum virNetDevVPortProfileOp vmop) { int i, j; - struct utsname ut; int disableKQEMU = 0; int enableKQEMU = 0; int disableKVM = 0; @@ -4977,7 +4962,6 @@ qemuBuildCommandLine(virConnectPtr conn, int spice = 0; int usbcontroller = 0; bool usblegacy = false; - uname_normalize(&ut); int contOrder[] = { /* We don't add an explicit IDE or FD controller because the * provided PIIX4 device already includes one. It isn't possible to @@ -4988,6 +4972,7 @@ qemuBuildCommandLine(virConnectPtr conn, VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, VIR_DOMAIN_CONTROLLER_TYPE_CCID, }; + virArch hostarch = virArchFromHost(); VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " "caps=%p migrateFrom=%s migrateFD=%d " @@ -5074,7 +5059,7 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; if (qemuBuildCpuArgStr(driver, def, emulator, caps, - &ut, &cpu, &hasHwVirt, !!migrateFrom) < 0) + hostarch, &cpu, &hasHwVirt, !!migrateFrom) < 0) goto error; if (cpu) { -- 1.7.11.7

On Tue, Dec 11, 2012 at 14:53:38 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Use virArch APIs to determine host architecture when launching QEMU.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9009bd2..6968f74 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c ... @@ -4414,7 +4400,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, * 2. emulator is qemu-system-x86_64 */ if (STREQ(def->os.arch, "i686") && - ((STREQ(ut->machine, "x86_64") && + (((hostarch == VIR_ARCH_X86_64) &&
Unneeded ().
strstr(emulator, "kvm")) || strstr(emulator, "x86_64"))) { virBufferAdd(&buf, default_model, -1);
ACK Jirka

From: "Daniel P. Berrange" <berrange@redhat.com> Convert the host capabilities and domain config structs to use the virArch datatype. Update the parsers and all drivers to take account of datatype change Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/conf/capabilities.c | 86 +++++++++++++++++----------------- src/conf/capabilities.h | 24 +++++----- src/conf/domain_conf.c | 36 +++++++++------ src/conf/domain_conf.h | 2 +- src/esx/esx_driver.c | 14 ++++-- src/libxl/libxl_conf.c | 38 ++++++--------- src/libxl/libxl_driver.c | 11 ++--- src/lxc/lxc_conf.c | 17 ++----- src/lxc/lxc_container.c | 32 +++++++------ src/lxc/lxc_container.h | 2 +- src/lxc/lxc_controller.c | 13 ++---- src/openvz/openvz_conf.c | 11 ++--- src/openvz/openvz_driver.c | 1 - src/parallels/parallels_driver.c | 24 +++++----- src/phyp/phyp_driver.c | 12 ++--- src/qemu/qemu_capabilities.c | 99 +++++++++++++++++----------------------- src/qemu/qemu_command.c | 40 ++++++++-------- src/qemu/qemu_command.h | 2 +- src/qemu/qemu_conf.c | 1 - src/qemu/qemu_driver.c | 1 - src/security/virt-aa-helper.c | 29 ++++-------- src/test/test_driver.c | 7 ++- src/uml/uml_conf.c | 15 ++---- src/vbox/vbox_tmpl.c | 20 +++----- src/vmware/vmware_conf.c | 34 +++++++------- src/vmx/vmx.c | 16 +++---- src/xen/xen_hypervisor.c | 66 ++++++++++----------------- src/xen/xen_hypervisor.h | 2 +- src/xenxs/xen_xm.c | 8 ++-- 29 files changed, 284 insertions(+), 379 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index a8ee2cf..97d1b80 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -42,14 +42,14 @@ VIR_ENUM_IMPL(virCapsHostPMTarget, VIR_NODE_SUSPEND_TARGET_LAST, /** * virCapabilitiesNew: - * @arch: host machine architecture + * @hostarch: host machine architecture * @offlineMigrate: non-zero if offline migration is available * @liveMigrate: non-zero if live migration is available * * Allocate a new capabilities object */ virCapsPtr -virCapabilitiesNew(const char *arch, +virCapabilitiesNew(virArch hostarch, int offlineMigrate, int liveMigrate) { @@ -58,16 +58,11 @@ virCapabilitiesNew(const char *arch, if (VIR_ALLOC(caps) < 0) return NULL; - if ((caps->host.arch = strdup(arch)) == NULL) - goto no_memory; + caps->host.arch = hostarch; caps->host.offlineMigrate = offlineMigrate; caps->host.liveMigrate = liveMigrate; return caps; - - no_memory: - virCapabilitiesFree(caps); - return NULL; } static void @@ -125,7 +120,6 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest) VIR_FREE(guest->ostype); - VIR_FREE(guest->arch.name); VIR_FREE(guest->arch.defaultInfo.emulator); VIR_FREE(guest->arch.defaultInfo.loader); for (i = 0 ; i < guest->arch.defaultInfo.nmachines ; i++) @@ -180,8 +174,6 @@ virCapabilitiesFree(virCapsPtr caps) { VIR_FREE(caps->host.migrateTrans[i]); VIR_FREE(caps->host.migrateTrans); - VIR_FREE(caps->host.arch); - for (i = 0; i < caps->host.nsecModels; i++) { VIR_FREE(caps->host.secModels[i].model); VIR_FREE(caps->host.secModels[i].doi); @@ -353,7 +345,7 @@ virCapabilitiesFreeMachines(virCapsGuestMachinePtr *machines, * virCapabilitiesAddGuest: * @caps: capabilities to extend * @ostype: guest operating system type ('hvm' or 'xen') - * @arch: guest CPU architecture ('i686', or 'x86_64', etc) + * @arch: guest CPU architecture * @wordsize: number of bits in CPU word * @emulator: path to default device emulator for arch/ostype * @loader: path to default BIOS loader for arch/ostype @@ -367,8 +359,7 @@ virCapabilitiesFreeMachines(virCapsGuestMachinePtr *machines, virCapsGuestPtr virCapabilitiesAddGuest(virCapsPtr caps, const char *ostype, - const char *arch, - int wordsize, + virArch arch, const char *emulator, const char *loader, int nmachines, @@ -382,9 +373,8 @@ virCapabilitiesAddGuest(virCapsPtr caps, if ((guest->ostype = strdup(ostype)) == NULL) goto no_memory; - if ((guest->arch.name = strdup(arch)) == NULL) - goto no_memory; - guest->arch.wordsize = wordsize; + guest->arch.id = arch; + guest->arch.wordsize = virArchGetWordSize(arch); if (emulator && (guest->arch.defaultInfo.emulator = strdup(emulator)) == NULL) @@ -505,18 +495,18 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest, /** * virCapabilitiesSupportsGuestArch: * @caps: capabilities to query - * @arch: Architecture to search for (eg, 'i686', 'x86_64') + * @arch: Architecture to search for * * Returns non-zero if the capabilities support the * requested architecture */ extern int virCapabilitiesSupportsGuestArch(virCapsPtr caps, - const char *arch) + virArch arch) { int i; for (i = 0 ; i < caps->nguests ; i++) { - if (STREQ(caps->guests[i]->arch.name, arch)) + if (caps->guests[i]->arch.id == arch) return 1; } return 0; @@ -548,7 +538,7 @@ virCapabilitiesSupportsGuestOSType(virCapsPtr caps, * virCapabilitiesSupportsGuestOSTypeArch: * @caps: capabilities to query * @ostype: OS type to search for (eg 'hvm', 'xen') - * @arch: Architecture to search for (eg, 'i686', 'x86_64') + * @arch: Architecture to search for * * Returns non-zero if the capabilities support the * requested operating system type @@ -556,12 +546,12 @@ virCapabilitiesSupportsGuestOSType(virCapsPtr caps, extern int virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps, const char *ostype, - const char *arch) + virArch arch) { int i; for (i = 0 ; i < caps->nguests ; i++) { if (STREQ(caps->guests[i]->ostype, ostype) && - STREQ(caps->guests[i]->arch.name, arch)) + (caps->guests[i]->arch.id == arch)) return 1; } return 0; @@ -576,28 +566,35 @@ virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps, * Returns the first architecture able to run the * requested operating system type */ -extern const char * +extern virArch virCapabilitiesDefaultGuestArch(virCapsPtr caps, const char *ostype, const char *domain) { int i, j; - const char *arch = NULL; + + /* First try to find one matching host arch */ for (i = 0 ; i < caps->nguests ; i++) { if (STREQ(caps->guests[i]->ostype, ostype)) { for (j = 0 ; j < caps->guests[i]->arch.ndomains ; j++) { - if (STREQ(caps->guests[i]->arch.domains[j]->type, domain)) { - /* Use the first match... */ - if (!arch) - arch = caps->guests[i]->arch.name; - /* ...unless we can match the host's architecture. */ - if (STREQ(caps->guests[i]->arch.name, caps->host.arch)) - return caps->guests[i]->arch.name; - } + if (STREQ(caps->guests[i]->arch.domains[j]->type, domain) && + (caps->guests[i]->arch.id == caps->host.arch)) + return caps->guests[i]->arch.id; } } } - return arch; + + /* Otherwise find the first match */ + for (i = 0 ; i < caps->nguests ; i++) { + if (STREQ(caps->guests[i]->ostype, ostype)) { + for (j = 0 ; j < caps->guests[i]->arch.ndomains ; j++) { + if (STREQ(caps->guests[i]->arch.domains[j]->type, domain)) + return caps->guests[i]->arch.id; + } + } + } + + return VIR_ARCH_I686; } /** @@ -614,7 +611,7 @@ virCapabilitiesDefaultGuestArch(virCapsPtr caps, extern const char * virCapabilitiesDefaultGuestMachine(virCapsPtr caps, const char *ostype, - const char *arch, + virArch arch, const char *domain) { int i; @@ -623,11 +620,12 @@ virCapabilitiesDefaultGuestMachine(virCapsPtr caps, virCapsGuestPtr guest = caps->guests[i]; int j; - if (!STREQ(guest->ostype, ostype) || !STREQ(guest->arch.name, arch)) + if (!STREQ(guest->ostype, ostype) || + (guest->arch.id != arch)) continue; for (j = 0; j < guest->arch.ndomains; j++) { - virCapsGuestDomainPtr dom= guest->arch.domains[j]; + virCapsGuestDomainPtr dom = guest->arch.domains[j]; if (!STREQ(dom->type, domain)) continue; @@ -659,14 +657,14 @@ virCapabilitiesDefaultGuestMachine(virCapsPtr caps, extern const char * virCapabilitiesDefaultGuestEmulator(virCapsPtr caps, const char *ostype, - const char *arch, + virArch arch, const char *domain) { int i, j; for (i = 0 ; i < caps->nguests ; i++) { char *emulator; if (STREQ(caps->guests[i]->ostype, ostype) && - STREQ(caps->guests[i]->arch.name, arch)) { + (caps->guests[i]->arch.id == arch)) { emulator = caps->guests[i]->arch.defaultInfo.emulator; for (j = 0 ; j < caps->guests[i]->arch.ndomains ; j++) { if (STREQ(caps->guests[i]->arch.domains[j]->type, domain)) { @@ -703,8 +701,9 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAsprintf(&xml," <uuid>%s</uuid>\n", host_uuid); } virBufferAddLit(&xml, " <cpu>\n"); - virBufferAsprintf(&xml, " <arch>%s</arch>\n", - caps->host.arch); + if (caps->host.arch) + virBufferAsprintf(&xml, " <arch>%s</arch>\n", + virArchToString(caps->host.arch)); if (caps->host.nfeatures) { virBufferAddLit(&xml, " <features>\n"); @@ -788,8 +787,9 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " <guest>\n"); virBufferAsprintf(&xml, " <os_type>%s</os_type>\n", caps->guests[i]->ostype); - virBufferAsprintf(&xml, " <arch name='%s'>\n", - caps->guests[i]->arch.name); + if (caps->guests[i]->arch.id) + virBufferAsprintf(&xml, " <arch name='%s'>\n", + virArchToString(caps->guests[i]->arch.id)); virBufferAsprintf(&xml, " <wordsize>%d</wordsize>\n", caps->guests[i]->arch.wordsize); if (caps->guests[i]->arch.defaultInfo.emulator) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 641f279..3cdbcaa 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -27,6 +27,7 @@ # include "internal.h" # include "buf.h" # include "cpu_conf.h" +# include "virarch.h" # include "virmacaddr.h" # include <libxml/xpath.h> @@ -65,8 +66,8 @@ struct _virCapsGuestDomain { typedef struct _virCapsGuestArch virCapsGuestArch; typedef virCapsGuestArch *virCapsGuestArchptr; struct _virCapsGuestArch { - char *name; - int wordsize; + virArch id; + unsigned int wordsize; virCapsGuestDomainInfo defaultInfo; size_t ndomains; size_t ndomains_max; @@ -101,7 +102,7 @@ struct _virCapsHostSecModel { typedef struct _virCapsHost virCapsHost; typedef virCapsHost *virCapsHostPtr; struct _virCapsHost { - char *arch; + virArch arch; size_t nfeatures; size_t nfeatures_max; char **features; @@ -150,7 +151,7 @@ struct _virCaps { unsigned int emulatorRequired : 1; const char *defaultDiskDriverName; int defaultDiskDriverType; /* enum virStorageFileFormat */ - int (*defaultConsoleTargetType)(const char *ostype, const char *arch); + int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); void *(*privateDataAllocFunc)(void); void (*privateDataFreeFunc)(void *); int (*privateDataXMLFormat)(virBufferPtr, void *); @@ -163,7 +164,7 @@ struct _virCaps { extern virCapsPtr -virCapabilitiesNew(const char *arch, +virCapabilitiesNew(virArch hostarch, int offlineMigrate, int liveMigrate); @@ -218,8 +219,7 @@ virCapabilitiesFreeMachines(virCapsGuestMachinePtr *machines, extern virCapsGuestPtr virCapabilitiesAddGuest(virCapsPtr caps, const char *ostype, - const char *arch, - int wordsize, + virArch arch, const char *emulator, const char *loader, int nmachines, @@ -241,29 +241,29 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest, extern int virCapabilitiesSupportsGuestArch(virCapsPtr caps, - const char *arch); + virArch arch); extern int virCapabilitiesSupportsGuestOSType(virCapsPtr caps, const char *ostype); extern int virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps, const char *ostype, - const char *arch); + virArch arch); -extern const char * +extern virArch virCapabilitiesDefaultGuestArch(virCapsPtr caps, const char *ostype, const char *domain); extern const char * virCapabilitiesDefaultGuestMachine(virCapsPtr caps, const char *ostype, - const char *arch, + virArch arch, const char *domain); extern const char * virCapabilitiesDefaultGuestEmulator(virCapsPtr caps, const char *ostype, - const char *arch, + virArch arch, const char *domain); extern char * diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6aa5f79..1f978db 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1694,7 +1694,6 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->redirdevs); VIR_FREE(def->os.type); - VIR_FREE(def->os.arch); VIR_FREE(def->os.machine); VIR_FREE(def->os.init); for (i = 0 ; def->os.initargv && def->os.initargv[i] ; i++) @@ -8357,7 +8356,7 @@ static char *virDomainDefDefaultEmulator(virDomainDefPtr def, if (!emulator) { virReportError(VIR_ERR_INTERNAL_ERROR, _("no emulator for domain %s os type %s on architecture %s"), - type, def->os.type, def->os.arch); + type, def->os.type, virArchToString(def->os.arch)); return NULL; } @@ -9412,12 +9411,20 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } - def->os.arch = virXPathString("string(./os/type[1]/@arch)", ctxt); - if (def->os.arch) { + tmp = virXPathString("string(./os/type[1]/@arch)", ctxt); + if (tmp) { + virArch arch = virArchFromString(tmp); + if (!arch) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown architecture %s"), + tmp); + goto error; + } + if (!virCapabilitiesSupportsGuestArch(caps, def->os.arch)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No guest options available for arch '%s'"), - def->os.arch); + virArchToString(def->os.arch)); goto error; } @@ -9426,20 +9433,17 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->os.arch)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No os type '%s' available for arch '%s'"), - def->os.type, def->os.arch); + def->os.type, virArchToString(def->os.arch)); goto error; } } else { - const char *defaultArch = virCapabilitiesDefaultGuestArch(caps, def->os.type, virDomainVirtTypeToString(def->virtType)); - if (defaultArch == NULL) { + def->os.arch = virCapabilitiesDefaultGuestArch(caps, def->os.type, virDomainVirtTypeToString(def->virtType)); + if (!def->os.arch) { virReportError(VIR_ERR_INTERNAL_ERROR, _("no supported architecture for os type '%s'"), def->os.type); goto error; } - if (!(def->os.arch = strdup(defaultArch))) { - goto no_memory; - } } def->os.machine = virXPathString("string(./os/type[1]/@machine)", ctxt); @@ -10050,7 +10054,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->memballoon = memballoon; VIR_FREE(nodes); - } else if (!STREQ(def->os.arch,"s390x")) { + } else if ((def->os.arch != VIR_ARCH_S390) && + (def->os.arch != VIR_ARCH_S390X)) { /* TODO: currently no balloon support on s390 -> no default balloon */ if (def->virtType == VIR_DOMAIN_VIRT_XEN || def->virtType == VIR_DOMAIN_VIRT_QEMU || @@ -11222,10 +11227,11 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, dst->os.type, src->os.type); goto cleanup; } - if (STRNEQ(src->os.arch, dst->os.arch)) { + if (src->os.arch != dst->os.arch){ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain architecture %s does not match source %s"), - dst->os.arch, src->os.arch); + virArchToString(dst->os.arch), + virArchToString(src->os.arch)); goto cleanup; } if (STRNEQ(src->os.machine, dst->os.machine)) { @@ -13935,7 +13941,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " <type"); if (def->os.arch) - virBufferAsprintf(buf, " arch='%s'", def->os.arch); + virBufferAsprintf(buf, " arch='%s'", virArchToString(def->os.arch)); if (def->os.machine) virBufferAsprintf(buf, " machine='%s'", def->os.machine); /* diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7ad5377..ce36472 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1479,7 +1479,7 @@ typedef struct _virDomainOSDef virDomainOSDef; typedef virDomainOSDef *virDomainOSDefPtr; struct _virDomainOSDef { char *type; - char *arch; + virArch arch; char *machine; size_t nBootDevs; int bootDevs[VIR_DOMAIN_BOOT_LAST]; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index a858298..4171beb 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -569,7 +569,7 @@ esxLookupHostSystemBiosUuid(esxPrivate *priv, unsigned char *uuid) static int esxDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } @@ -587,9 +587,9 @@ esxCapsInit(esxPrivate *priv) } if (supportsLongMode == esxVI_Boolean_True) { - caps = virCapabilitiesNew("x86_64", 1, 1); + caps = virCapabilitiesNew(VIR_ARCH_X86_64, 1, 1); } else { - caps = virCapabilitiesNew("i686", 1, 1); + caps = virCapabilitiesNew(VIR_ARCH_I686, 1, 1); } if (caps == NULL) { @@ -608,7 +608,9 @@ esxCapsInit(esxPrivate *priv) } /* i686 */ - guest = virCapabilitiesAddGuest(caps, "hvm", "i686", 32, NULL, NULL, 0, + guest = virCapabilitiesAddGuest(caps, "hvm", + VIR_ARCH_I686, + NULL, NULL, 0, NULL); if (guest == NULL) { @@ -622,7 +624,9 @@ esxCapsInit(esxPrivate *priv) /* x86_64 */ if (supportsLongMode == esxVI_Boolean_True) { - guest = virCapabilitiesAddGuest(caps, "hvm", "x86_64", 64, NULL, NULL, + guest = virCapabilitiesAddGuest(caps, "hvm", + VIR_ARCH_X86_64, + NULL, NULL, 0, NULL); if (guest == NULL) { diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 5b6d6fb..e456b1e 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -29,7 +29,6 @@ #include <libxl.h> #include <sys/types.h> #include <sys/socket.h> -#include <sys/utsname.h> #include "internal.h" #include "logging.h" @@ -53,7 +52,7 @@ struct guest_arch { - const char *model; + virArch arch; int bits; int hvm; int pae; @@ -156,9 +155,8 @@ libxlBuildCapabilities(const char *hostmachine, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", - guest_archs[i].model, - guest_archs[i].bits, - (STREQ(hostmachine, "x86_64") ? + guest_archs[i].arch, + ((hostarch == VIR_ARCH_X86_64) ? "/usr/lib64/xen/bin/qemu-dm" : "/usr/lib/xen/bin/qemu-dm"), (guest_archs[i].hvm ? @@ -230,7 +228,7 @@ libxlBuildCapabilities(const char *hostmachine, } static virCapsPtr -libxlMakeCapabilitiesInternal(const char *hostmachine, +libxlMakeCapabilitiesInternal(virArch hostarch, libxl_physinfo *phy_info, char *capabilities) { @@ -285,12 +283,11 @@ libxlMakeCapabilitiesInternal(const char *hostmachine, if (regexec(&xen_cap_rec, token, sizeof(subs) / sizeof(subs[0]), subs, 0) == 0) { int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); - const char *model; - int bits, pae = 0, nonpae = 0, ia64_be = 0; + virArch arch; + int pae = 0, nonpae = 0, ia64_be = 0; if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { - model = "i686"; - bits = 32; + arch = VIR_ARCH_I686; if (subs[3].rm_so != -1 && STRPREFIX(&token[subs[3].rm_so], "p")) pae = 1; @@ -298,26 +295,24 @@ libxlMakeCapabilitiesInternal(const char *hostmachine, nonpae = 1; } else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) { - model = "x86_64"; - bits = 64; + arch = VIR_ARCH_X86_64; } else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) { - model = "ia64"; - bits = 64; + arch = VIR_ARCH_ITANIUM; if (subs[3].rm_so != -1 && STRPREFIX(&token[subs[3].rm_so], "be")) ia64_be = 1; } else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) { - model = "ppc64"; - bits = 64; + arch = VIR_ARCH_PPC64; } else { + /* XXX arm ? */ continue; } /* Search for existing matching (model,hvm) tuple */ for (i = 0 ; i < nr_guest_archs ; i++) { - if (STREQ(guest_archs[i].model, model) && + if ((guest_archs[i].arch == arch) && guest_archs[i].hvm == hvm) { break; } @@ -330,7 +325,7 @@ libxlMakeCapabilitiesInternal(const char *hostmachine, if (i == nr_guest_archs) nr_guest_archs++; - guest_archs[i].model = model; + guest_archs[i].arch = arch; guest_archs[i].bits = bits; guest_archs[i].hvm = hvm; @@ -347,7 +342,7 @@ libxlMakeCapabilitiesInternal(const char *hostmachine, } } - if ((caps = libxlBuildCapabilities(hostmachine, + if ((caps = libxlBuildCapabilities(hostarch, host_pae, guest_archs, nr_guest_archs)) == NULL) @@ -795,7 +790,6 @@ libxlMakeCapabilities(libxl_ctx *ctx) { libxl_physinfo phy_info; const libxl_version_info *ver_info; - struct utsname utsname; regcomp(&xen_cap_rec, xen_cap_re, REG_EXTENDED); @@ -811,9 +805,7 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; } - uname(&utsname); - - return libxlMakeCapabilitiesInternal(utsname.machine, + return libxlMakeCapabilitiesInternal(virArchFromHost(), &phy_info, ver_info->capabilities); } diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 302f81c..c02cb19 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -26,7 +26,6 @@ #include <config.h> -#include <sys/utsname.h> #include <math.h> #include <libxl.h> #include <fcntl.h> @@ -320,7 +319,7 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) { libxl_physinfo phy_info; const libxl_version_info* ver_info; - struct utsname utsname; + virArch hostarch = virArchFromHost(); if (libxl_get_physinfo(driver->ctx, &phy_info)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -334,14 +333,10 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) return -1; } - uname(&utsname); - if (virStrncpy(info->model, - utsname.machine, - strlen(utsname.machine), - sizeof(info->model)) == NULL) { + if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("machine type %s too big for destination"), - utsname.machine); + virArchToString(hostarch)); return -1; } diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index e512b8f..9b89ceb 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -26,8 +26,6 @@ /* includes */ #include <config.h> -#include <sys/utsname.h> - #include "lxc_conf.h" #include "nodeinfo.h" #include "virterror_internal.h" @@ -43,7 +41,7 @@ #define VIR_FROM_THIS VIR_FROM_LXC static int lxcDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC; } @@ -52,14 +50,11 @@ static int lxcDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, /* Functions */ virCapsPtr lxcCapsInit(virLXCDriverPtr driver) { - struct utsname utsname; virCapsPtr caps; virCapsGuestPtr guest; - const char *altArch; - - uname(&utsname); + virArch altArch; - if ((caps = virCapabilitiesNew(utsname.machine, + if ((caps = virCapabilitiesNew(virArchFromHost(), 0, 0)) == NULL) goto error; @@ -88,8 +83,7 @@ virCapsPtr lxcCapsInit(virLXCDriverPtr driver) if ((guest = virCapabilitiesAddGuest(caps, "exe", - utsname.machine, - sizeof(void*) == 4 ? 32 : 64, + caps->host.arch, LIBEXECDIR "/libvirt_lxc", NULL, 0, @@ -105,11 +99,10 @@ virCapsPtr lxcCapsInit(virLXCDriverPtr driver) goto error; /* On 64-bit hosts, we can use personality() to request a 32bit process */ - if ((altArch = lxcContainerGetAlt32bitArch(utsname.machine)) != NULL) { + if ((altArch = lxcContainerGetAlt32bitArch(caps->host.arch)) != VIR_ARCH_NONE) { if ((guest = virCapabilitiesAddGuest(caps, "exe", altArch, - 32, LIBEXECDIR "/libvirt_lxc", NULL, 0, diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 3014564..8875a92 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1931,24 +1931,26 @@ static int userns_supported(void) #endif } -const char *lxcContainerGetAlt32bitArch(const char *arch) +virArch lxcContainerGetAlt32bitArch(virArch arch) { /* Any Linux 64bit arch which has a 32bit * personality available should be listed here */ - if (STREQ(arch, "x86_64")) - return "i686"; - if (STREQ(arch, "s390x")) - return "s390"; - if (STREQ(arch, "ppc64")) - return "ppc"; - if (STREQ(arch, "parisc64")) - return "parisc"; - if (STREQ(arch, "sparc64")) - return "sparc"; - if (STREQ(arch, "mips64")) - return "mips"; - - return NULL; + if (arch == VIR_ARCH_X86_64) + return VIR_ARCH_I686; + if (arch == VIR_ARCH_S390X) + return VIR_ARCH_S390; + if (arch == VIR_ARCH_PPC64) + return VIR_ARCH_PPC; + if (arch == VIR_ARCH_PARISC64) + return VIR_ARCH_PARISC; + if (arch == VIR_ARCH_SPARC64) + return VIR_ARCH_SPARC; + if (arch == VIR_ARCH_MIPS64) + return VIR_ARCH_MIPS; + if (arch == VIR_ARCH_MIPS64EL) + return VIR_ARCH_MIPSEL; + + return VIR_ARCH_NONE; } diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index c8c70e0..ada72f7 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -63,6 +63,6 @@ int lxcContainerStart(virDomainDefPtr def, int lxcContainerAvailable(int features); -const char *lxcContainerGetAlt32bitArch(const char *arch); +virArch lxcContainerGetAlt32bitArch(virArch arch); #endif /* LXC_CONTAINER_H */ diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 5510b9a..34f38c8 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -29,7 +29,6 @@ #include <sys/socket.h> #include <sys/types.h> #include <sys/un.h> -#include <sys/utsname.h> #include <sys/personality.h> #include <unistd.h> #include <paths.h> @@ -1077,17 +1076,15 @@ static int virLXCControllerDeleteInterfaces(virLXCControllerPtr ctrl) static int lxcSetPersonality(virDomainDefPtr def) { - struct utsname utsname; - const char *altArch; + virArch altArch; - uname(&utsname); - - altArch = lxcContainerGetAlt32bitArch(utsname.machine); + altArch = lxcContainerGetAlt32bitArch(virArchFromHost()); if (altArch && - STREQ(def->os.arch, altArch)) { + (def->os.arch == altArch)) { if (personality(PER_LINUX32) < 0) { virReportSystemError(errno, _("Unable to request personality for %s on %s"), - altArch, utsname.machine); + virArchToString(altArch), + virArchToString(virArchFromHost())); return -1; } } diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 2bd9caf..089f63b 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -40,7 +40,6 @@ #include <limits.h> #include <errno.h> #include <string.h> -#include <sys/utsname.h> #include <sys/wait.h> #include "virterror_internal.h" @@ -170,20 +169,17 @@ error: static int openvzDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ; } virCapsPtr openvzCapsInit(void) { - struct utsname utsname; virCapsPtr caps; virCapsGuestPtr guest; - uname(&utsname); - - if ((caps = virCapabilitiesNew(utsname.machine, + if ((caps = virCapabilitiesNew(virArchFromHost(), 0, 0)) == NULL) goto no_memory; @@ -194,8 +190,7 @@ virCapsPtr openvzCapsInit(void) if ((guest = virCapabilitiesAddGuest(caps, "exe", - utsname.machine, - sizeof(void*) == 4 ? 32 : 64, + caps->host.arch, NULL, NULL, 0, diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index c5f5ca0..fd6ecf8 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -37,7 +37,6 @@ #include <stdlib.h> #include <unistd.h> #include <errno.h> -#include <sys/utsname.h> #include <sys/stat.h> #include <fcntl.h> #include <paths.h> diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 50efd1d..7dc59b9 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -31,7 +31,6 @@ #include <stdlib.h> #include <unistd.h> #include <errno.h> -#include <sys/utsname.h> #include <sys/stat.h> #include <fcntl.h> #include <paths.h> @@ -58,7 +57,6 @@ #define PRLCTL "prlctl" #define PRLSRVCTL "prlsrvctl" -#define PARALLELS_DEFAULT_ARCH "x86_64" #define parallelsDomNotFoundError(domain) \ do { \ @@ -88,7 +86,7 @@ parallelsDriverUnlock(parallelsConnPtr driver) static int parallelsDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } @@ -110,10 +108,9 @@ parallelsBuildCapabilities(void) { virCapsPtr caps; virCapsGuestPtr guest; - struct utsname utsname; - uname(&utsname); - if ((caps = virCapabilitiesNew(utsname.machine, 0, 0)) == NULL) + if ((caps = virCapabilitiesNew(virArchFromHost(), + 0, 0)) == NULL) goto no_memory; if (nodeCapsInitNUMA(caps) < 0) @@ -122,8 +119,9 @@ parallelsBuildCapabilities(void) virCapabilitiesSetMacPrefix(caps, (unsigned char[]) { 0x42, 0x1C, 0x00}); - if ((guest = virCapabilitiesAddGuest(caps, "hvm", PARALLELS_DEFAULT_ARCH, - 64, "parallels", + if ((guest = virCapabilitiesAddGuest(caps, "hvm", + VIR_ARCH_X86_64, + "parallels", NULL, 0, NULL)) == NULL) goto no_memory; @@ -131,8 +129,9 @@ parallelsBuildCapabilities(void) "parallels", NULL, NULL, 0, NULL) == NULL) goto no_memory; - if ((guest = virCapabilitiesAddGuest(caps, "exe", PARALLELS_DEFAULT_ARCH, - 64, "parallels", + if ((guest = virCapabilitiesAddGuest(caps, "exe", + VIR_ARCH_X86_64, + "parallels", NULL, 0, NULL)) == NULL) goto no_memory; @@ -532,8 +531,7 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) goto no_memory; } - if (!(def->os.arch = strdup(PARALLELS_DEFAULT_ARCH))) - goto no_memory; + def->os.arch = VIR_ARCH_X86_64; if (VIR_ALLOC(pdom) < 0) goto no_memory; @@ -1495,7 +1493,7 @@ parallelsApplyChanges(virDomainObjPtr dom, virDomainDefPtr new) * other paramenters are null and boot devices config is default */ if (!STREQ_NULLABLE(old->os.type, new->os.type) || - !STREQ_NULLABLE(old->os.arch, new->os.arch) || + old->os.arch != new->os.arch || new->os.machine != NULL || new->os.bootmenu != 0 || new->os.kernel != NULL || new->os.initrd != NULL || new->os.cmdline != NULL || new->os.root != NULL || diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 63017f4..c82acb6 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -40,7 +40,6 @@ #include <sys/socket.h> #include <netdb.h> #include <fcntl.h> -#include <sys/utsname.h> #include <domain_event.h> #include "internal.h" @@ -289,7 +288,7 @@ phypGetVIOSPartitionID(virConnectPtr conn) static int phypDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } @@ -298,13 +297,11 @@ static int phypDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, static virCapsPtr phypCapsInit(void) { - struct utsname utsname; virCapsPtr caps; virCapsGuestPtr guest; - uname(&utsname); - - if ((caps = virCapabilitiesNew(utsname.machine, 0, 0)) == NULL) + if ((caps = virCapabilitiesNew(virArchFromHost(), + 0, 0)) == NULL) goto no_memory; /* Some machines have problematic NUMA toplogy causing @@ -323,8 +320,7 @@ phypCapsInit(void) if ((guest = virCapabilitiesAddGuest(caps, "linux", - utsname.machine, - sizeof(int) == 4 ? 32 : 8, + caps->host.arch, NULL, NULL, 0, NULL)) == NULL) goto no_memory; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 01a1b98..c9146c4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -576,17 +576,20 @@ cleanup: static char * -qemuCapsFindBinaryForArch(const char *hostarch, - const char *guestarch) +qemuCapsFindBinaryForArch(virArch hostarch, + virArch guestarch) { char *ret; - if (STREQ(guestarch, "i686")) { + /* Some special cases where libvirt's canonical + * guestarch string form can't match qemu's binary + */ + if (guestarch == VIR_ARCH_I686) { ret = virFindFileInPath("qemu-system-i386"); if (ret && !virFileIsExecutable(ret)) VIR_FREE(ret); - if (!ret && STREQ(hostarch, "x86_64")) { + if (!ret && hostarch == VIR_ARCH_X86_64) { ret = virFindFileInPath("qemu-system-x86_64"); if (ret && !virFileIsExecutable(ret)) VIR_FREE(ret); @@ -594,11 +597,12 @@ qemuCapsFindBinaryForArch(const char *hostarch, if (!ret) ret = virFindFileInPath("qemu"); - } else if (STREQ(guestarch, "itanium")) { + } else if (guestarch == VIR_ARCH_ITANIUM) { ret = virFindFileInPath("qemu-system-ia64"); } else { + /* Default case we have matching arch strings */ char *bin; - if (virAsprintf(&bin, "qemu-system-%s", guestarch) < 0) { + if (virAsprintf(&bin, "qemu-system-%s", virArchToString(guestarch)) < 0) { virReportOOMError(); return NULL; } @@ -610,26 +614,15 @@ qemuCapsFindBinaryForArch(const char *hostarch, return ret; } -static int -qemuCapsGetArchWordSize(const char *guestarch) -{ - if (STREQ(guestarch, "i686") || - STREQ(guestarch, "ppc") || - STREQ(guestarch, "sparc") || - STREQ(guestarch, "mips") || - STREQ(guestarch, "mipsel")) - return 32; - return 64; -} static bool -qemuCapsIsValidForKVM(const char *hostarch, - const char *guestarch) +qemuCapsIsValidForKVM(virArch hostarch, + virArch guestarch) { - if (STREQ(hostarch, guestarch)) + if (hostarch == guestarch) return true; - if (STREQ(hostarch, "x86_64") && - STREQ(guestarch, "i686")) + if (hostarch == VIR_ARCH_X86_64 && + guestarch == VIR_ARCH_I686) return true; return false; } @@ -637,8 +630,8 @@ qemuCapsIsValidForKVM(const char *hostarch, static int qemuCapsInitGuest(virCapsPtr caps, qemuCapsCachePtr cache, - const char *hostarch, - const char *guestarch) + virArch hostarch, + virArch guestarch) { virCapsGuestPtr guest; int i; @@ -719,7 +712,6 @@ qemuCapsInitGuest(virCapsPtr caps, if ((guest = virCapabilitiesAddGuest(caps, "hvm", guestarch, - qemuCapsGetArchWordSize(guestarch), binary, NULL, nmachines, @@ -777,13 +769,13 @@ qemuCapsInitGuest(virCapsPtr caps, } - if ((STREQ(guestarch, "i686") || - STREQ(guestarch, "x86_64")) && + if (((guestarch == VIR_ARCH_I686) || + (guestarch == VIR_ARCH_X86_64)) && (virCapabilitiesAddGuestFeature(guest, "acpi", 1, 1) == NULL || virCapabilitiesAddGuestFeature(guest, "apic", 1, 0) == NULL)) goto error; - if (STREQ(guestarch, "i686") && + if ((guestarch == VIR_ARCH_I686) && (virCapabilitiesAddGuestFeature(guest, "pae", 1, 0) == NULL || virCapabilitiesAddGuestFeature(guest, "nonpae", 1, 0) == NULL)) goto error; @@ -807,15 +799,16 @@ error: static int qemuCapsInitCPU(virCapsPtr caps, - const char *arch) + virArch arch) { virCPUDefPtr cpu = NULL; union cpuData *data = NULL; virNodeInfo nodeinfo; int ret = -1; + const char *archstr = virArchToString(arch); if (VIR_ALLOC(cpu) < 0 - || !(cpu->arch = strdup(arch))) { + || !(cpu->arch = strdup(archstr))) { virReportOOMError(); goto error; } @@ -829,14 +822,14 @@ qemuCapsInitCPU(virCapsPtr caps, cpu->threads = nodeinfo.threads; caps->host.cpu = cpu; - if (!(data = cpuNodeData(arch)) + if (!(data = cpuNodeData(archstr)) || cpuDecode(cpu, data, NULL, 0, NULL) < 0) goto cleanup; ret = 0; cleanup: - cpuDataFree(arch, data); + cpuDataFree(archstr, data); return ret; @@ -847,9 +840,10 @@ error: static int qemuDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch) + virArch arch) { - if (STRPREFIX(arch, "s390")) + if (arch == VIR_ARCH_S390 || + arch == VIR_ARCH_S390X) return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; else return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; @@ -858,21 +852,10 @@ static int qemuDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) { - struct utsname utsname; virCapsPtr caps; int i; - const char *const arches[] = { - "i686", "x86_64", "arm", - "microblaze", "microblazeel", - "mips", "mipsel", "sparc", - "ppc", "ppc64", "itanium", - "s390x" - }; - - /* Really, this never fails - look at the man-page. */ - uname(&utsname); - - if ((caps = virCapabilitiesNew(utsname.machine, + + if ((caps = virCapabilitiesNew(virArchFromHost(), 1, 1)) == NULL) goto error; @@ -888,7 +871,7 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) VIR_WARN("Failed to query host NUMA topology, disabling NUMA capabilities"); } - if (qemuCapsInitCPU(caps, utsname.machine) < 0) + if (qemuCapsInitCPU(caps, virArchFromHost()) < 0) VIR_WARN("Failed to get host CPU"); /* Add the power management features of the host */ @@ -899,11 +882,14 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) virCapabilitiesAddHostMigrateTransport(caps, "tcp"); - /* First the pure HVM guests */ - for (i = 0 ; i < ARRAY_CARDINALITY(arches) ; i++) + /* QEMU can support pretty much every arch that exists, + * so just probe for them all - we gracefully fail + * if a qemu-system-$ARCH bvinary can't be found + */ + for (i = 0 ; i < VIR_ARCH_LAST ; i++) if (qemuCapsInitGuest(caps, cache, - utsname.machine, - arches[i]) < 0) + virArchFromHost(), + i) < 0) goto error; /* QEMU Requires an emulator in the XML */ @@ -1610,7 +1596,6 @@ cleanup: return ret; } - static void uname_normalize(struct utsname *ut) { @@ -1625,24 +1610,24 @@ uname_normalize(struct utsname *ut) ut->machine[1] = '6'; } + int qemuCapsGetDefaultVersion(virCapsPtr caps, qemuCapsCachePtr capsCache, unsigned int *version) { const char *binary; - struct utsname ut; qemuCapsPtr qemucaps; if (*version > 0) return 0; - uname_normalize(&ut); if ((binary = virCapabilitiesDefaultGuestEmulator(caps, "hvm", - ut.machine, + virArchFromHost(), "qemu")) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find suitable emulator for %s"), ut.machine); + _("Cannot find suitable emulator for %s"), + virArchToString(virArchFromHost())); return -1; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6968f74..267dce7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -46,7 +46,6 @@ #include "device_conf.h" #include "storage_file.h" -#include <sys/utsname.h> #include <sys/stat.h> #include <fcntl.h> @@ -506,7 +505,7 @@ qemuSetScsiControllerModel(virDomainDefPtr def, return -1; } } else { - if (STREQ(def->os.arch, "ppc64") && + if ((def->os.arch == VIR_ARCH_PPC64) && STREQ(def->os.machine, "pseries")) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; } else if (qemuCapsGet(caps, QEMU_CAPS_SCSI_LSI)) { @@ -796,7 +795,8 @@ qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, } for (i = 0; i < def->nnets ; i++) { - if (STRPREFIX(def->os.arch, "s390") && + if ((def->os.arch == VIR_ARCH_S390 || + def->os.arch == VIR_ARCH_S390X) && def->nets[i]->model == NULL) { def->nets[i]->model = strdup("virtio"); } @@ -913,7 +913,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, for (i = 0 ; i < def->nserials; i++) { if (def->serials[i]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && def->serials[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY && - STREQ(def->os.arch, "ppc64") && + (def->os.arch == VIR_ARCH_PPC64) && STREQ(def->os.machine, "pseries")) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, @@ -2979,7 +2979,7 @@ qemuBuildUSBControllerDevStr(virDomainDefPtr domainDef, model = def->model; if (model == -1) { - if (STREQ(domainDef->os.arch, "ppc64")) + if (domainDef->os.arch == VIR_ARCH_PPC64) model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI; else model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; @@ -4287,7 +4287,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, *hasHwVirt = false; - if (STREQ(def->os.arch, "i686")) + if (def->os.arch == VIR_ARCH_I686) default_model = "qemu32"; else default_model = "qemu64"; @@ -4399,7 +4399,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, * 1. guest OS is i686 * 2. emulator is qemu-system-x86_64 */ - if (STREQ(def->os.arch, "i686") && + if ((def->os.arch == VIR_ARCH_I686) && (((hostarch == VIR_ARCH_X86_64) && strstr(emulator, "kvm")) || strstr(emulator, "x86_64"))) { @@ -6885,13 +6885,13 @@ qemuBuildCommandLine(virConnectPtr conn, */ char * qemuBuildChrDeviceStr(virDomainChrDefPtr serial, - qemuCapsPtr caps, - char *os_arch, - char *machine) + qemuCapsPtr caps, + virArch arch, + char *machine) { virBuffer cmd = VIR_BUFFER_INITIALIZER; - if (STREQ(os_arch, "ppc64") && STREQ(machine, "pseries")) { + if ((arch == VIR_ARCH_PPC64) && STREQ(machine, "pseries")) { if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && serial->source.type == VIR_DOMAIN_CHR_TYPE_PTY && serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) { @@ -8121,7 +8121,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } } while ((p = next)); - if (STREQ(dom->os.arch, "x86_64")) { + if (dom->os.arch == VIR_ARCH_X86_64) { bool is_32bit = false; if (cpu) { union cpuData *cpuData = NULL; @@ -8139,8 +8139,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } if (is_32bit) { - VIR_FREE(dom->os.arch); - dom->os.arch = strdup("i686"); + dom->os.arch = VIR_ARCH_I686; } } VIR_FREE(model); @@ -8338,16 +8337,17 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, else path = strstr(def->emulator, "qemu"); if (def->virtType == VIR_DOMAIN_VIRT_KVM) - def->os.arch = strdup(caps->host.cpu->arch); + def->os.arch = caps->host.arch; else if (path && - STRPREFIX(path, "qemu-system-")) - def->os.arch = strdup(path + strlen("qemu-system-")); - else - def->os.arch = strdup("i686"); + STRPREFIX(path, "qemu-system-")) { + def->os.arch = virArchFromString(path + strlen("qemu-system-")); + } else + def->os.arch = VIR_ARCH_I686; if (!def->os.arch) goto no_memory; - if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64")) + if ((def->os.arch == VIR_ARCH_I686) || + (def->os.arch == VIR_ARCH_X86_64)) def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI) /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/; #define WANT_VALUE() \ diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 6556e6e..59f32a6 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -65,7 +65,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, char * qemuBuildChrDeviceStr (virDomainChrDefPtr serial, qemuCapsPtr caps, - char *os_arch, + virArch arch, char *machine); /* With vlan == -1, use netdev syntax, else old hostnet */ diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8d380a1..04ae69f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -33,7 +33,6 @@ #include <fcntl.h> #include <sys/wait.h> #include <arpa/inet.h> -#include <sys/utsname.h> #include <mntent.h> #include "virterror_internal.h" diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e099c5c..60cf6c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -34,7 +34,6 @@ #include <stdlib.h> #include <unistd.h> #include <errno.h> -#include <sys/utsname.h> #include <sys/stat.h> #include <fcntl.h> #include <signal.h> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e480b30..8f322fc 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -73,7 +73,6 @@ typedef struct { virCapsPtr caps; /* VM capabilities */ char *hvm; /* type of hypervisor (eg hvm, xen) */ char *arch; /* machine architecture */ - int bits; /* bits in the guest */ char *newfile; /* newly added file */ bool append; /* append to .files instead of rewrite */ } vahControl; @@ -640,7 +639,6 @@ verify_xpath_context(xmlXPathContextPtr ctxt) * Parse the xml we received to fill in the following: * ctl->hvm * ctl->arch - * ctl->bits * * These are suitable for setting up a virCapsPtr */ @@ -650,6 +648,7 @@ caps_mockup(vahControl * ctl, const char *xmlStr) int rc = -1; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; + char *arch; if (!(xml = virXMLParseStringCtxt(xmlStr, _("(domain_definition)"), &ctxt))) { @@ -670,24 +669,13 @@ caps_mockup(vahControl * ctl, const char *xmlStr) vah_error(ctl, 0, _("os.type is not 'hvm'")); goto cleanup; } - ctl->arch = virXPathString("string(./os/type[1]/@arch)", ctxt); - if (!ctl->arch) { - /* The XML we are given should have an arch, but in case it doesn't, - * just use the host's arch. - */ - struct utsname utsname; - - /* Really, this never fails - look at the man-page. */ - uname(&utsname); - if ((ctl->arch = strdup(utsname.machine)) == NULL) { - vah_error(ctl, 0, _("could not allocate memory")); - goto cleanup; - } + arch = virXPathString("string(./os/type[1]/@arch)", ctxt); + if (!arch) { + ctl->arch = virArchFromHost(); + } else { + ctl->arch = virArchFromString(arch); + VIR_FREE(arch); } - if (STREQ(ctl->arch, "x86_64")) - ctl->bits = 64; - else - ctl->bits = 32; rc = 0; @@ -699,7 +687,7 @@ caps_mockup(vahControl * ctl, const char *xmlStr) } static int aaDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } @@ -727,7 +715,6 @@ get_definition(vahControl * ctl, const char *xmlStr) if ((guest = virCapabilitiesAddGuest(ctl->caps, ctl->hvm, ctl->arch, - ctl->bits, NULL, NULL, 0, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6ca59e2..256be92 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -150,7 +150,7 @@ static void testDomainObjPrivateFree(void *data) static int testDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } @@ -163,7 +163,7 @@ testBuildCapabilities(virConnectPtr conn) { const char *const guest_types[] = { "hvm", "xen" }; int i; - if ((caps = virCapabilitiesNew(TEST_MODEL, 0, 0)) == NULL) + if ((caps = virCapabilitiesNew(VIR_ARCH_I686, 0, 0)) == NULL) goto no_memory; caps->defaultConsoleTargetType = testDefaultConsoleType; @@ -182,8 +182,7 @@ testBuildCapabilities(virConnectPtr conn) { for (i = 0; i < ARRAY_CARDINALITY(guest_types) ; i++) { if ((guest = virCapabilitiesAddGuest(caps, guest_types[i], - TEST_MODEL, - TEST_MODEL_WORDSIZE, + VIR_ARCH_I686, TEST_EMULATOR, NULL, 0, diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 6ef6de3..7414969 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -33,7 +33,6 @@ #include <fcntl.h> #include <sys/wait.h> #include <arpa/inet.h> -#include <sys/utsname.h> #include "uml_conf.h" #include "uuid.h" @@ -54,21 +53,17 @@ static int umlDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML; } virCapsPtr umlCapsInit(void) { - struct utsname utsname; virCapsPtr caps; virCapsGuestPtr guest; - /* Really, this never fails - look at the man-page. */ - uname(&utsname); - - if ((caps = virCapabilitiesNew(utsname.machine, + if ((caps = virCapabilitiesNew(virArchFromHost(), 0, 0)) == NULL) goto error; @@ -92,8 +87,7 @@ virCapsPtr umlCapsInit(void) { if ((guest = virCapabilitiesAddGuest(caps, "uml", - utsname.machine, - STREQ(utsname.machine, "x86_64") ? 64 : 32, + caps->host.arch, NULL, NULL, 0, @@ -412,11 +406,8 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn, virDomainObjPtr vm) { int i, j; - struct utsname ut; virCommandPtr cmd; - uname(&ut); - cmd = virCommandNew(vm->def->os.kernel); virCommandAddEnvPassCommon(cmd); diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index f9fa442..f3b36f3 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -34,7 +34,6 @@ #include <config.h> -#include <sys/utsname.h> #include <unistd.h> #include <sys/types.h> #include <sys/stat.h> @@ -823,20 +822,18 @@ cleanup: static int vboxDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } -static virCapsPtr vboxCapsInit(void) { - struct utsname utsname; +static virCapsPtr vboxCapsInit(void) +{ virCapsPtr caps; virCapsGuestPtr guest; - uname(&utsname); - - if ((caps = virCapabilitiesNew(utsname.machine, + if ((caps = virCapabilitiesNew(virArchFromHost(), 0, 0)) == NULL) goto no_memory; @@ -847,8 +844,7 @@ static virCapsPtr vboxCapsInit(void) { if ((guest = virCapabilitiesAddGuest(caps, "hvm", - utsname.machine, - sizeof(void *) * CHAR_BIT, + caps->host.arch, NULL, NULL, 0, @@ -2230,7 +2226,6 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { machine->vtbl->GetAccessible(machine, &accessible); if (accessible) { int i = 0; - struct utsname utsname; PRBool PAEEnabled = PR_FALSE; PRBool ACPIEnabled = PR_FALSE; PRBool IOAPICEnabled = PR_FALSE; @@ -2312,8 +2307,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { def->os.type = strdup("hvm"); - uname(&utsname); - def->os.arch = strdup(utsname.machine); + def->os.arch = virArchFromHost(); def->os.nBootDevs = 0; for (i = 0; (i < VIR_DOMAIN_BOOT_LAST) && (i < maxBootPosition); i++) { @@ -3785,7 +3779,7 @@ vboxSetBootDeviceOrder(virDomainDefPtr def, vboxGlobalData *data, int i = 0; VIR_DEBUG("def->os.type %s", def->os.type); - VIR_DEBUG("def->os.arch %s", def->os.arch); + VIR_DEBUG("def->os.arch %s", virArchToString(def->os.arch)); VIR_DEBUG("def->os.machine %s", def->os.machine); VIR_DEBUG("def->os.nBootDevs %zu", def->os.nBootDevs); VIR_DEBUG("def->os.bootDevs[0] %d", def->os.bootDevs[0]); diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 2eed4f8..33100fb 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -22,7 +22,6 @@ #include <config.h> #include <string.h> -#include <sys/utsname.h> #include "command.h" #include "cpu/cpu.h" @@ -51,7 +50,7 @@ vmwareFreeDriver(struct vmware_driver *driver) static int vmwareDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } @@ -60,15 +59,14 @@ static int vmwareDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, virCapsPtr vmwareCapsInit(void) { - struct utsname utsname; virCapsPtr caps = NULL; virCapsGuestPtr guest = NULL; virCPUDefPtr cpu = NULL; union cpuData *data = NULL; + const char *hostarch = NULL; - uname(&utsname); - - if ((caps = virCapabilitiesNew(utsname.machine, 0, 0)) == NULL) + if ((caps = virCapabilitiesNew(virArchFromHost(), + 0, 0)) == NULL) goto error; if (nodeCapsInitNUMA(caps) < 0) @@ -79,8 +77,7 @@ vmwareCapsInit(void) /* i686 guests are always supported */ if ((guest = virCapabilitiesAddGuest(caps, "hvm", - "i686", - 32, + VIR_ARCH_I686, NULL, NULL, 0, NULL)) == NULL) goto error; @@ -89,12 +86,16 @@ vmwareCapsInit(void) NULL, NULL, 0, NULL) == NULL) goto error; - if (VIR_ALLOC(cpu) < 0 - || !(cpu->arch = strdup(utsname.machine))) { + if (VIR_ALLOC(cpu) < 0) { virReportOOMError(); goto error; } + hostarch = virArchToString(caps->host.arch); + if (!(cpu->arch = strdup(hostarch))) { + virReportOOMError(); + goto error; + } cpu->type = VIR_CPU_TYPE_HOST; if (!(data = cpuNodeData(cpu->arch)) @@ -107,15 +108,14 @@ vmwareCapsInit(void) * Or * - Host CPU is x86_64 with virtualization extensions */ - if (STREQ(utsname.machine, "x86_64") || - (cpuHasFeature(utsname.machine, data, "lm") && - (cpuHasFeature(utsname.machine, data, "vmx") || - cpuHasFeature(utsname.machine, data, "svm")))) { + if (caps->host.arch == VIR_ARCH_X86_64 || + (cpuHasFeature(hostarch, data, "lm") && + (cpuHasFeature(hostarch, data, "vmx") || + cpuHasFeature(hostarch, data, "svm")))) { if ((guest = virCapabilitiesAddGuest(caps, "hvm", - "x86_64", - 64, + VIR_ARCH_X86_64, NULL, NULL, 0, NULL)) == NULL) goto error; @@ -129,7 +129,7 @@ vmwareCapsInit(void) cleanup: virCPUDefFree(cpu); - cpuDataFree(utsname.machine, data); + cpuDataFree(hostarch, data); return caps; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index e051de5..a4d1f2e 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1522,14 +1522,9 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) } if (guestOS != NULL && virFileHasSuffix(guestOS, "-64")) { - def->os.arch = strdup("x86_64"); + def->os.arch = VIR_ARCH_X86_64; } else { - def->os.arch = strdup("i686"); - } - - if (def->os.arch == NULL) { - virReportOOMError(); - goto cleanup; + def->os.arch = VIR_ARCH_I686; } /* vmx:smbios.reflecthost -> def:os.smbios_mode */ @@ -3069,14 +3064,15 @@ virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, virDomainDefPtr def, virtualHW_version); /* def:os.arch -> vmx:guestOS */ - if (def->os.arch == NULL || STRCASEEQ(def->os.arch, "i686")) { + if (def->os.arch == VIR_ARCH_I686) { virBufferAddLit(&buffer, "guestOS = \"other\"\n"); - } else if (STRCASEEQ(def->os.arch, "x86_64")) { + } else if (def->os.arch == VIR_ARCH_X86_64) { virBufferAddLit(&buffer, "guestOS = \"other-64\"\n"); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting domain XML attribute 'arch' of entry 'os/type' " - "to be 'i686' or 'x86_64' but found '%s'"), def->os.arch); + "to be 'i686' or 'x86_64' but found '%s'"), + virArchToString(def->os.arch)); goto cleanup; } diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 237a6ab..1a959cd 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -36,7 +36,6 @@ #include <stdint.h> #include <regex.h> #include <errno.h> -#include <sys/utsname.h> #ifdef __sun # include <sys/systeminfo.h> @@ -2292,8 +2291,7 @@ xenHypervisorGetVersion(virConnectPtr conn, unsigned long *hvVer) } struct guest_arch { - const char *model; - int bits; + virArch arch; int hvm; int pae; int nonpae; @@ -2302,7 +2300,7 @@ struct guest_arch { static int xenDefaultConsoleType(const char *ostype, - const char *arch ATTRIBUTE_UNUSED) + virArch arch ATTRIBUTE_UNUSED) { if (STREQ(ostype, "hvm")) return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; @@ -2312,7 +2310,7 @@ static int xenDefaultConsoleType(const char *ostype, static virCapsPtr xenHypervisorBuildCapabilities(virConnectPtr conn, - const char *hostmachine, + virArch hostarch, int host_pae, const char *hvm_type, struct guest_arch *guest_archs, @@ -2322,7 +2320,7 @@ xenHypervisorBuildCapabilities(virConnectPtr conn, int hv_major = hv_versions.hv >> 16; int hv_minor = hv_versions.hv & 0xFFFF; - if ((caps = virCapabilitiesNew(hostmachine, 1, 1)) == NULL) + if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x16, 0x3e }); @@ -2357,9 +2355,8 @@ xenHypervisorBuildCapabilities(virConnectPtr conn, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", - guest_archs[i].model, - guest_archs[i].bits, - (STREQ(hostmachine, "x86_64") ? + guest_archs[i].arch, + (hostarch == VIR_ARCH_X86_64 ? "/usr/lib64/xen/bin/qemu-dm" : "/usr/lib/xen/bin/qemu-dm"), (guest_archs[i].hvm ? @@ -2505,18 +2502,13 @@ xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) struct guest_arch guest_arches[32]; int i = 0; virCapsPtr caps = NULL; - struct utsname utsname; int pae, longmode; const char *hvm; if (!get_cpu_flags(conn, &hvm, &pae, &longmode)) return NULL; - /* Really, this never fails - look at the man-page. */ - uname(&utsname); - - guest_arches[i].model = "i686"; - guest_arches[i].bits = 32; + guest_arches[i].arch = VIR_ARCH_I686; guest_arches[i].hvm = 0; guest_arches[i].pae = pae; guest_arches[i].nonpae = !pae; @@ -2524,8 +2516,7 @@ xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) i++; if (longmode) { - guest_arches[i].model = "x86_64"; - guest_arches[i].bits = 64; + guest_arches[i].arch = VIR_ARCH_X86_64; guest_arches[i].hvm = 0; guest_arches[i].pae = 0; guest_arches[i].nonpae = 0; @@ -2534,8 +2525,7 @@ xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) } if (hvm[0] != '\0') { - guest_arches[i].model = "i686"; - guest_arches[i].bits = 32; + guest_arches[i].arch = VIR_ARCH_I686; guest_arches[i].hvm = 1; guest_arches[i].pae = pae; guest_arches[i].nonpae = 1; @@ -2543,8 +2533,7 @@ xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) i++; if (longmode) { - guest_arches[i].model = "x86_64"; - guest_arches[i].bits = 64; + guest_arches[i].arch = VIR_ARCH_X86_64; guest_arches[i].hvm = 1; guest_arches[i].pae = 0; guest_arches[i].nonpae = 0; @@ -2554,7 +2543,7 @@ xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) } if ((caps = xenHypervisorBuildCapabilities(conn, - utsname.machine, + virArchFromHost(), pae, hvm, guest_arches, i)) == NULL) virReportOOMError(); @@ -2574,7 +2563,7 @@ xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) */ virCapsPtr xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, - const char *hostmachine, + virArch hostarch, FILE *cpuinfo, FILE *capabilities) { char line[1024], *str, *token; @@ -2645,12 +2634,11 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, if (regexec(&xen_cap_rec, token, sizeof(subs) / sizeof(subs[0]), subs, 0) == 0) { int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); - const char *model; - int bits, pae = 0, nonpae = 0, ia64_be = 0; + int pae = 0, nonpae = 0, ia64_be = 0; + virArch arch; if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { - model = "i686"; - bits = 32; + arch = VIR_ARCH_I686; if (subs[3].rm_so != -1 && STRPREFIX(&token[subs[3].rm_so], "p")) pae = 1; @@ -2658,27 +2646,24 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, nonpae = 1; } else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) { - model = "x86_64"; - bits = 64; + arch = VIR_ARCH_X86_64; } else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) { - model = "ia64"; - bits = 64; + arch = VIR_ARCH_ITANIUM; if (subs[3].rm_so != -1 && STRPREFIX(&token[subs[3].rm_so], "be")) ia64_be = 1; } else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) { - model = "ppc64"; - bits = 64; + arch = VIR_ARCH_PPC64; } else { - /* XXX surely no other Xen archs exist */ + /* XXX surely no other Xen archs exist. Arrrrrrrrrm */ continue; } /* Search for existing matching (model,hvm) tuple */ for (i = 0 ; i < nr_guest_archs ; i++) { - if (STREQ(guest_archs[i].model, model) && + if (guest_archs[i].arch == arch && guest_archs[i].hvm == hvm) { break; } @@ -2691,8 +2676,7 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, if (i == nr_guest_archs) nr_guest_archs++; - guest_archs[i].model = model; - guest_archs[i].bits = bits; + guest_archs[i].arch = arch; guest_archs[i].hvm = hvm; /* Careful not to overwrite a previous positive @@ -2710,7 +2694,7 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, } if ((caps = xenHypervisorBuildCapabilities(conn, - hostmachine, + hostarch, host_pae, hvm_type, guest_archs, @@ -2738,10 +2722,6 @@ xenHypervisorMakeCapabilities(virConnectPtr conn) #else virCapsPtr caps = NULL; FILE *cpuinfo, *capabilities; - struct utsname utsname; - - /* Really, this never fails - look at the man-page. */ - uname(&utsname); cpuinfo = fopen("/proc/cpuinfo", "r"); if (cpuinfo == NULL) { @@ -2765,7 +2745,7 @@ xenHypervisorMakeCapabilities(virConnectPtr conn) } caps = xenHypervisorMakeCapabilitiesInternal(conn, - utsname.machine, + virArchFromHost(), cpuinfo, capabilities); if (caps == NULL) diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index fd23f38..786e301 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -62,7 +62,7 @@ int xenHypervisorGetVersion (virConnectPtr conn, unsigned long *hvVer); virCapsPtr xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, - const char *hostmachine, + virArch hostarch, FILE *cpuinfo, FILE *capabilities); char * diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 23b3037..1b04b4c 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -263,7 +263,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, virDomainGraphicsDefPtr graphics = NULL; virDomainHostdevDefPtr hostdev = NULL; int i; - const char *defaultArch, *defaultMachine; + const char *defaultMachine; int vmlocaltime = 0; unsigned long count; char *script = NULL; @@ -290,15 +290,13 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (!(def->os.type = strdup(hvm ? "hvm" : "xen"))) goto no_memory; - defaultArch = virCapabilitiesDefaultGuestArch(caps, def->os.type, virDomainVirtTypeToString(def->virtType)); - if (defaultArch == NULL) { + def->os.arch = virCapabilitiesDefaultGuestArch(caps, def->os.type, virDomainVirtTypeToString(def->virtType)); + if (!def->os.arch) { virReportError(VIR_ERR_INTERNAL_ERROR, _("no supported architecture for os type '%s'"), def->os.type); goto cleanup; } - if (!(def->os.arch = strdup(defaultArch))) - goto no_memory; defaultMachine = virCapabilitiesDefaultGuestMachine(caps, def->os.type, -- 1.7.11.7

On Tue, Dec 11, 2012 at 14:53:39 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Convert the host capabilities and domain config structs to use the virArch datatype. Update the parsers and all drivers to take account of datatype change ... diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index a8ee2cf..97d1b80 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c ... @@ -556,12 +546,12 @@ virCapabilitiesSupportsGuestOSType(virCapsPtr caps, extern int virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps, const char *ostype, - const char *arch) + virArch arch) { int i; for (i = 0 ; i < caps->nguests ; i++) { if (STREQ(caps->guests[i]->ostype, ostype) && - STREQ(caps->guests[i]->arch.name, arch)) + (caps->guests[i]->arch.id == arch))
Unneeded ().
return 1; } return 0; @@ -576,28 +566,35 @@ virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps, * Returns the first architecture able to run the * requested operating system type */ -extern const char * +extern virArch virCapabilitiesDefaultGuestArch(virCapsPtr caps, const char *ostype, const char *domain) { int i, j; - const char *arch = NULL; + + /* First try to find one matching host arch */ for (i = 0 ; i < caps->nguests ; i++) { if (STREQ(caps->guests[i]->ostype, ostype)) { for (j = 0 ; j < caps->guests[i]->arch.ndomains ; j++) { - if (STREQ(caps->guests[i]->arch.domains[j]->type, domain)) { - /* Use the first match... */ - if (!arch) - arch = caps->guests[i]->arch.name; - /* ...unless we can match the host's architecture. */ - if (STREQ(caps->guests[i]->arch.name, caps->host.arch)) - return caps->guests[i]->arch.name; - } + if (STREQ(caps->guests[i]->arch.domains[j]->type, domain) && + (caps->guests[i]->arch.id == caps->host.arch))
You've started to love these extra () recently :-)
+ return caps->guests[i]->arch.id; } } } - return arch; + + /* Otherwise find the first match */ + for (i = 0 ; i < caps->nguests ; i++) { + if (STREQ(caps->guests[i]->ostype, ostype)) { + for (j = 0 ; j < caps->guests[i]->arch.ndomains ; j++) { + if (STREQ(caps->guests[i]->arch.domains[j]->type, domain)) + return caps->guests[i]->arch.id; + } + } + } +
I think both the original and the new version are equally readable, but if you think this new version is better, I'm fine with it.
+ return VIR_ARCH_I686;
However, this should definitely return VIR_ARCH_NONE. The original version would return NULL in case of no match.
}
/** ... @@ -623,11 +620,12 @@ virCapabilitiesDefaultGuestMachine(virCapsPtr caps, virCapsGuestPtr guest = caps->guests[i]; int j;
- if (!STREQ(guest->ostype, ostype) || !STREQ(guest->arch.name, arch)) + if (!STREQ(guest->ostype, ostype) || + (guest->arch.id != arch))
()
continue;
for (j = 0; j < guest->arch.ndomains; j++) { - virCapsGuestDomainPtr dom= guest->arch.domains[j]; + virCapsGuestDomainPtr dom = guest->arch.domains[j];
if (!STREQ(dom->type, domain)) continue; @@ -659,14 +657,14 @@ virCapabilitiesDefaultGuestMachine(virCapsPtr caps, extern const char * virCapabilitiesDefaultGuestEmulator(virCapsPtr caps, const char *ostype, - const char *arch, + virArch arch, const char *domain) { int i, j; for (i = 0 ; i < caps->nguests ; i++) { char *emulator; if (STREQ(caps->guests[i]->ostype, ostype) && - STREQ(caps->guests[i]->arch.name, arch)) { + (caps->guests[i]->arch.id == arch)) {
()
emulator = caps->guests[i]->arch.defaultInfo.emulator; for (j = 0 ; j < caps->guests[i]->arch.ndomains ; j++) { if (STREQ(caps->guests[i]->arch.domains[j]->type, domain)) {
...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6aa5f79..1f978db 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c ... @@ -9412,12 +9411,20 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; }
- def->os.arch = virXPathString("string(./os/type[1]/@arch)", ctxt); - if (def->os.arch) { + tmp = virXPathString("string(./os/type[1]/@arch)", ctxt); + if (tmp) { + virArch arch = virArchFromString(tmp); + if (!arch) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown architecture %s"), + tmp); + goto error; + } +
Memory leak, you need to call VIR_FREE(tmp) here.
if (!virCapabilitiesSupportsGuestArch(caps, def->os.arch)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No guest options available for arch '%s'"), - def->os.arch); + virArchToString(def->os.arch)); goto error; }
@@ -9426,20 +9433,17 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->os.arch)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No os type '%s' available for arch '%s'"), - def->os.type, def->os.arch); + def->os.type, virArchToString(def->os.arch)); goto error; } } else { - const char *defaultArch = virCapabilitiesDefaultGuestArch(caps, def->os.type, virDomainVirtTypeToString(def->virtType)); - if (defaultArch == NULL) { + def->os.arch = virCapabilitiesDefaultGuestArch(caps, def->os.type, virDomainVirtTypeToString(def->virtType));
Make this line shorter while changing it.
+ if (!def->os.arch) { virReportError(VIR_ERR_INTERNAL_ERROR, _("no supported architecture for os type '%s'"), def->os.type); goto error; } - if (!(def->os.arch = strdup(defaultArch))) { - goto no_memory; - } }
def->os.machine = virXPathString("string(./os/type[1]/@machine)", ctxt); @@ -10050,7 +10054,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
def->memballoon = memballoon; VIR_FREE(nodes); - } else if (!STREQ(def->os.arch,"s390x")) { + } else if ((def->os.arch != VIR_ARCH_S390) && + (def->os.arch != VIR_ARCH_S390X)) {
Too many extra (), I won't report them anymore :-)
/* TODO: currently no balloon support on s390 -> no default balloon */ if (def->virtType == VIR_DOMAIN_VIRT_XEN || def->virtType == VIR_DOMAIN_VIRT_QEMU ||
...
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 302f81c..c02cb19 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c ... @@ -334,14 +333,10 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) return -1; }
- uname(&utsname); - if (virStrncpy(info->model, - utsname.machine, - strlen(utsname.machine), - sizeof(info->model)) == NULL) { + if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) == NULL) {
This wouldn't even compile; s/nodeinfo/info/
virReportError(VIR_ERR_INTERNAL_ERROR, _("machine type %s too big for destination"), - utsname.machine); + virArchToString(hostarch)); return -1; }
...
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 01a1b98..c9146c4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c ... @@ -899,11 +882,14 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) virCapabilitiesAddHostMigrateTransport(caps, "tcp");
- /* First the pure HVM guests */ - for (i = 0 ; i < ARRAY_CARDINALITY(arches) ; i++) + /* QEMU can support pretty much every arch that exists, + * so just probe for them all - we gracefully fail + * if a qemu-system-$ARCH bvinary can't be found
s/bvinary/binary/
+ */ + for (i = 0 ; i < VIR_ARCH_LAST ; i++) if (qemuCapsInitGuest(caps, cache, - utsname.machine, - arches[i]) < 0) + virArchFromHost(), + i) < 0) goto error;
/* QEMU Requires an emulator in the XML */ ... diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6968f74..267dce7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c ... @@ -8338,16 +8337,17 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, else path = strstr(def->emulator, "qemu"); if (def->virtType == VIR_DOMAIN_VIRT_KVM) - def->os.arch = strdup(caps->host.cpu->arch); + def->os.arch = caps->host.arch; else if (path && - STRPREFIX(path, "qemu-system-")) - def->os.arch = strdup(path + strlen("qemu-system-")); - else - def->os.arch = strdup("i686"); + STRPREFIX(path, "qemu-system-")) { + def->os.arch = virArchFromString(path + strlen("qemu-system-")); + } else + def->os.arch = VIR_ARCH_I686; if (!def->os.arch) goto no_memory;
No need for the extra {}, however, if you want to add them, add them to all branches of this if statement.
- if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64")) + if ((def->os.arch == VIR_ARCH_I686) || + (def->os.arch == VIR_ARCH_X86_64)) def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI) /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/; #define WANT_VALUE() \
...
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 23b3037..1b04b4c 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c ... @@ -290,15 +290,13 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (!(def->os.type = strdup(hvm ? "hvm" : "xen"))) goto no_memory;
- defaultArch = virCapabilitiesDefaultGuestArch(caps, def->os.type, virDomainVirtTypeToString(def->virtType)); - if (defaultArch == NULL) { + def->os.arch = virCapabilitiesDefaultGuestArch(caps, def->os.type, virDomainVirtTypeToString(def->virtType));
Line too long.
+ if (!def->os.arch) { virReportError(VIR_ERR_INTERNAL_ERROR, _("no supported architecture for os type '%s'"), def->os.type); goto cleanup; } - if (!(def->os.arch = strdup(defaultArch))) - goto no_memory;
defaultMachine = virCapabilitiesDefaultGuestMachine(caps, def->os.type,
ACK with the small issues fixed. Jirka

From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/conf/cpu_conf.c | 24 +++++++++++++++--------- src/conf/cpu_conf.h | 3 ++- src/cpu/cpu.c | 40 ++++++++++++++++++++-------------------- src/cpu/cpu.h | 11 ++++++----- src/cpu/cpu_arm.c | 2 +- src/cpu/cpu_generic.c | 10 ++++++---- src/cpu/cpu_powerpc.c | 14 +++++++------- src/cpu/cpu_s390.c | 2 +- src/cpu/cpu_x86.c | 18 +++++++++--------- src/qemu/qemu_capabilities.c | 10 +++++----- src/qemu/qemu_command.c | 8 ++++---- src/vmware/vmware_conf.c | 12 +++++------- 12 files changed, 81 insertions(+), 73 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 8cb54a3..2b3b44d 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -79,7 +79,6 @@ virCPUDefFree(virCPUDefPtr def) if (!def) return; - VIR_FREE(def->arch); virCPUDefFreeModel(def); for (i = 0 ; i < def->ncells ; i++) { @@ -148,9 +147,7 @@ virCPUDefCopy(const virCPUDefPtr cpu) copy->sockets = cpu->sockets; copy->cores = cpu->cores; copy->threads = cpu->threads; - - if (cpu->arch && !(copy->arch = strdup(cpu->arch))) - goto no_memory; + copy->arch = cpu->arch; if (virCPUDefCopyModel(copy, cpu, false) < 0) goto error; @@ -269,12 +266,19 @@ virCPUDefParseXML(const xmlNodePtr node, } if (def->type == VIR_CPU_TYPE_HOST) { - def->arch = virXPathString("string(./arch[1])", ctxt); - if (!def->arch) { + char *arch = virXPathString("string(./arch[1])", ctxt); + if (!arch) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing CPU architecture")); goto error; } + if ((def->arch = virArchFromString(arch)) == VIR_ARCH_NONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown architecture %s"), arch); + VIR_FREE(arch); + goto error; + } + VIR_FREE(arch); } if (!(def->model = virXPathString("string(./model[1])", ctxt)) && @@ -560,7 +564,8 @@ virCPUDefFormatBufFull(virBufferPtr buf, virBufferAddLit(buf, ">\n"); if (def->arch) - virBufferAsprintf(buf, " <arch>%s</arch>\n", def->arch); + virBufferAsprintf(buf, " <arch>%s</arch>\n", + virArchToString(def->arch)); virBufferAdjustIndent(buf, 2); if (virCPUDefFormatBuf(buf, def, flags) < 0) @@ -740,10 +745,11 @@ virCPUDefIsEqual(virCPUDefPtr src, goto cleanup; } - if (STRNEQ_NULLABLE(src->arch, dst->arch)) { + if (src->arch != dst->arch) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target CPU arch %s does not match source %s"), - NULLSTR(dst->arch), NULLSTR(src->arch)); + virArchToString(dst->arch), + virArchToString(src->arch)); goto cleanup; } diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 879f8eb..5bff70e 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -28,6 +28,7 @@ # include "buf.h" # include "xml.h" # include "bitmap.h" +# include "virarch.h" # define VIR_CPU_VENDOR_ID_LENGTH 12 @@ -104,7 +105,7 @@ struct _virCPUDef { int type; /* enum virCPUType */ int mode; /* enum virCPUMode */ int match; /* enum virCPUMatch */ - char *arch; + virArch arch; char *model; char *vendor_id; /* vendor id returned by CPUID in the guest */ int fallback; /* enum virCPUFallback */ diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 4263b88..53c4cc3 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -48,12 +48,12 @@ static struct cpuArchDriver *drivers[] = { static struct cpuArchDriver * -cpuGetSubDriver(const char *arch) +cpuGetSubDriver(virArch arch) { unsigned int i; unsigned int j; - if (arch == NULL) { + if (arch == VIR_ARCH_NONE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("undefined hardware architecture")); return NULL; @@ -61,7 +61,7 @@ cpuGetSubDriver(const char *arch) for (i = 0; i < NR_DRIVERS - 1; i++) { for (j = 0; j < drivers[i]->narch; j++) { - if (STREQ(arch, drivers[i]->arch[j])) + if (arch == drivers[i]->arch[j]) return drivers[i]; } } @@ -120,7 +120,7 @@ cpuCompare(virCPUDefPtr host, if (driver->compare == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot compare CPUs of %s architecture"), - host->arch); + virArchToString(host->arch)); return VIR_CPU_COMPARE_ERROR; } @@ -163,7 +163,7 @@ cpuDecode(virCPUDefPtr cpu, if (driver->decode == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot decode CPU data for %s architecture"), - cpu->arch); + virArchToString(cpu->arch)); return -1; } @@ -172,7 +172,7 @@ cpuDecode(virCPUDefPtr cpu, int -cpuEncode(const char *arch, +cpuEncode(virArch arch, const virCPUDefPtr cpu, union cpuData **forced, union cpuData **required, @@ -185,7 +185,7 @@ cpuEncode(const char *arch, VIR_DEBUG("arch=%s, cpu=%p, forced=%p, required=%p, " "optional=%p, disabled=%p, forbidden=%p, vendor=%p", - NULLSTR(arch), cpu, forced, required, + virArchToString(arch), cpu, forced, required, optional, disabled, forbidden, vendor); if ((driver = cpuGetSubDriver(arch)) == NULL) @@ -194,7 +194,7 @@ cpuEncode(const char *arch, if (driver->encode == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot encode CPU data for %s architecture"), - arch); + virArchToString(arch)); return -1; } @@ -204,12 +204,12 @@ cpuEncode(const char *arch, void -cpuDataFree(const char *arch, +cpuDataFree(virArch arch, union cpuData *data) { struct cpuArchDriver *driver; - VIR_DEBUG("arch=%s, data=%p", NULLSTR(arch), data); + VIR_DEBUG("arch=%s, data=%p", virArchToString(arch), data); if (data == NULL) return; @@ -220,7 +220,7 @@ cpuDataFree(const char *arch, if (driver->free == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot free CPU data for %s architecture"), - arch); + virArchToString(arch)); return; } @@ -229,11 +229,11 @@ cpuDataFree(const char *arch, union cpuData * -cpuNodeData(const char *arch) +cpuNodeData(virArch arch) { struct cpuArchDriver *driver; - VIR_DEBUG("arch=%s", NULLSTR(arch)); + VIR_DEBUG("arch=%s", virArchToString(arch)); if ((driver = cpuGetSubDriver(arch)) == NULL) return NULL; @@ -241,7 +241,7 @@ cpuNodeData(const char *arch) if (driver->nodeData == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot get node CPU data for %s architecture"), - arch); + virArchToString(arch)); return NULL; } @@ -265,7 +265,7 @@ cpuGuestData(virCPUDefPtr host, if (driver->guestData == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot compute guest CPU data for %s architecture"), - host->arch); + virArchToString(host->arch)); return VIR_CPU_COMPARE_ERROR; } @@ -391,7 +391,7 @@ cpuBaseline(virCPUDefPtr *cpus, if (driver->baseline == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot compute baseline CPU of %s architecture"), - cpus[0]->arch); + virArchToString(cpus[0]->arch)); return NULL; } @@ -413,7 +413,7 @@ cpuUpdate(virCPUDefPtr guest, if (driver->update == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot update guest CPU data for %s architecture"), - host->arch); + virArchToString(host->arch)); return -1; } @@ -421,14 +421,14 @@ cpuUpdate(virCPUDefPtr guest, } int -cpuHasFeature(const char *arch, +cpuHasFeature(virArch arch, const union cpuData *data, const char *feature) { struct cpuArchDriver *driver; VIR_DEBUG("arch=%s, data=%p, feature=%s", - arch, data, feature); + virArchToString(arch), data, feature); if ((driver = cpuGetSubDriver(arch)) == NULL) return -1; @@ -436,7 +436,7 @@ cpuHasFeature(const char *arch, if (driver->hasFeature == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot check guest CPU data for %s architecture"), - arch); + virArchToString(arch)); return -1; } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 01c732c..5153b62 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -26,6 +26,7 @@ # include "virterror_internal.h" # include "datatypes.h" +# include "virarch.h" # include "conf/cpu_conf.h" # include "cpu_x86_data.h" # include "cpu_ppc_data.h" @@ -88,7 +89,7 @@ typedef int struct cpuArchDriver { const char *name; - const char **arch; + const virArch *arch; unsigned int narch; cpuArchCompare compare; cpuArchDecode decode; @@ -118,7 +119,7 @@ cpuDecode (virCPUDefPtr cpu, const char *preferred); extern int -cpuEncode (const char *arch, +cpuEncode (virArch arch, const virCPUDefPtr cpu, union cpuData **forced, union cpuData **required, @@ -128,11 +129,11 @@ cpuEncode (const char *arch, union cpuData **vendor); extern void -cpuDataFree (const char *arch, +cpuDataFree (virArch arch, union cpuData *data); extern union cpuData * -cpuNodeData (const char *arch); +cpuNodeData (virArch arch); extern virCPUCompareResult cpuGuestData(virCPUDefPtr host, @@ -157,7 +158,7 @@ cpuUpdate (virCPUDefPtr guest, const virCPUDefPtr host); extern int -cpuHasFeature(const char *arch, +cpuHasFeature(virArch arch, const union cpuData *data, const char *feature); diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 2d005df..36c9db0 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -28,7 +28,7 @@ #define VIR_FROM_THIS VIR_FROM_CPU -static const char *archs[] = { "armv7l" }; +static const virArch archs[] = { VIR_ARCH_ARMV7L }; static union cpuData * ArmNodeData(void) diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index 3a6ed6c..b10ff47 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -64,7 +64,8 @@ genericCompare(virCPUDefPtr host, unsigned int i; unsigned int reqfeatures; - if ((cpu->arch && STRNEQ(host->arch, cpu->arch)) || + if (((cpu->arch != VIR_ARCH_NONE) && + (host->arch != cpu->arch)) || STRNEQ(host->model, cpu->model)) return VIR_CPU_COMPARE_INCOMPATIBLE; @@ -139,11 +140,11 @@ genericBaseline(virCPUDefPtr *cpus, } if (VIR_ALLOC(cpu) < 0 || - !(cpu->arch = strdup(cpus[0]->arch)) || !(cpu->model = strdup(cpus[0]->model)) || VIR_ALLOC_N(features, cpus[0]->nfeatures) < 0) goto no_memory; + cpu->arch = cpus[0]->arch; cpu->type = VIR_CPU_TYPE_HOST; count = nfeatures = cpus[0]->nfeatures; @@ -153,10 +154,11 @@ genericBaseline(virCPUDefPtr *cpus, for (i = 1; i < ncpus; i++) { virHashTablePtr hash; - if (STRNEQ(cpu->arch, cpus[i]->arch)) { + if (cpu->arch != cpus[i]->arch) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPUs have incompatible architectures: '%s' != '%s'"), - cpu->arch, cpus[i]->arch); + virArchToString(cpu->arch), + virArchToString(cpus[i]->arch)); goto error; } diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index e420ffb..ac10789 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -36,7 +36,7 @@ #define VIR_FROM_THIS VIR_FROM_CPU -static const char *archs[] = { "ppc64" }; +static const virArch archs[] = { VIR_ARCH_PPC64 }; struct cpuPowerPC { const char *name; @@ -417,7 +417,8 @@ static virCPUCompareResult PowerPCCompare(virCPUDefPtr host, virCPUDefPtr cpu) { - if ((cpu->arch && STRNEQ(host->arch, cpu->arch)) || + if ((cpu->arch != VIR_ARCH_NONE && + (host->arch != cpu->arch)) || STRNEQ(host->model, cpu->model)) return VIR_CPU_COMPARE_INCOMPATIBLE; @@ -589,9 +590,10 @@ PowerPCBaseline(virCPUDefPtr *cpus, goto error; } - if (VIR_ALLOC(cpu) < 0 || - !(cpu->arch = strdup(cpus[0]->arch))) - goto no_memory; + if (VIR_ALLOC(cpu) < 0) + goto no_memory; + + cpu->arch = cpus[0]->arch; cpu->type = VIR_CPU_TYPE_GUEST; cpu->match = VIR_CPU_MATCH_EXACT; @@ -610,8 +612,6 @@ PowerPCBaseline(virCPUDefPtr *cpus, if (!outputModel) VIR_FREE(cpu->model); - VIR_FREE(cpu->arch); - cleanup: ppcModelFree(base_model); ppcMapFree(map); diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index 137c15f..ffbad03 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -29,7 +29,7 @@ #define VIR_FROM_THIS VIR_FROM_CPU -static const char *archs[] = { "s390", "s390x" }; +static const virArch archs[] = { VIR_ARCH_S390, VIR_ARCH_S390X }; static union cpuData * s390NodeData(void) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index ca8cd92..2001b99 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -40,7 +40,7 @@ static const struct cpuX86cpuid cpuidNull = { 0, 0, 0, 0, 0 }; -static const char *archs[] = { "i686", "x86_64" }; +static const virArch archs[] = { VIR_ARCH_I686, VIR_ARCH_X86_64 }; struct x86_vendor { char *name; @@ -1165,22 +1165,23 @@ x86Compute(virCPUDefPtr host, enum compare_result result; unsigned int i; - if (cpu->arch != NULL) { + if (cpu->arch != VIR_ARCH_NONE) { bool found = false; for (i = 0; i < ARRAY_CARDINALITY(archs); i++) { - if (STREQ(archs[i], cpu->arch)) { + if (archs[i] == cpu->arch) { found = true; break; } } if (!found) { - VIR_DEBUG("CPU arch %s does not match host arch", cpu->arch); + VIR_DEBUG("CPU arch %s does not match host arch", + virArchToString(cpu->arch)); if (message && virAsprintf(message, _("CPU arch %s does not match host arch"), - cpu->arch) < 0) + virArchToString(cpu->arch)) < 0) goto no_memory; return VIR_CPU_COMPARE_INCOMPATIBLE; } @@ -1643,9 +1644,10 @@ x86Baseline(virCPUDefPtr *cpus, if (!(base_model = x86ModelFromCPU(cpus[0], map, VIR_CPU_FEATURE_REQUIRE))) goto error; - if (VIR_ALLOC(cpu) < 0 || - !(cpu->arch = strdup(cpus[0]->arch))) + if (VIR_ALLOC(cpu) < 0) goto no_memory; + + cpu->arch = cpus[0]->arch; cpu->type = VIR_CPU_TYPE_GUEST; cpu->match = VIR_CPU_MATCH_EXACT; @@ -1713,8 +1715,6 @@ x86Baseline(virCPUDefPtr *cpus, if (!outputVendor) VIR_FREE(cpu->vendor); - VIR_FREE(cpu->arch); - cleanup: x86ModelFree(base_model); x86MapFree(map); diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c9146c4..f6b53ca 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -805,14 +805,14 @@ qemuCapsInitCPU(virCapsPtr caps, union cpuData *data = NULL; virNodeInfo nodeinfo; int ret = -1; - const char *archstr = virArchToString(arch); - if (VIR_ALLOC(cpu) < 0 - || !(cpu->arch = strdup(archstr))) { + if (VIR_ALLOC(cpu) < 0) { virReportOOMError(); goto error; } + cpu->arch = arch; + if (nodeGetInfo(NULL, &nodeinfo)) goto error; @@ -822,14 +822,14 @@ qemuCapsInitCPU(virCapsPtr caps, cpu->threads = nodeinfo.threads; caps->host.cpu = cpu; - if (!(data = cpuNodeData(archstr)) + if (!(data = cpuNodeData(arch)) || cpuDecode(cpu, data, NULL, 0, NULL) < 0) goto cleanup; ret = 0; cleanup: - cpuDataFree(archstr, data); + cpuDataFree(arch, data); return ret; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 267dce7..2628f7d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4358,10 +4358,10 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, virBufferAddLit(&buf, "host"); } else { if (VIR_ALLOC(guest) < 0 || - !(guest->arch = strdup(host->arch)) || (cpu->vendor_id && !(guest->vendor_id = strdup(cpu->vendor_id)))) goto no_memory; + guest->arch = host->arch; if (cpu->match == VIR_CPU_MATCH_MINIMUM) preferred = host->model; else @@ -8127,13 +8127,13 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, union cpuData *cpuData = NULL; int ret; - ret = cpuEncode("x86_64", cpu, NULL, &cpuData, + ret = cpuEncode(VIR_ARCH_X86_64, cpu, NULL, &cpuData, NULL, NULL, NULL, NULL); if (ret < 0) goto error; - is_32bit = (cpuHasFeature("x86_64", cpuData, "lm") != 1); - cpuDataFree("x86_64", cpuData); + is_32bit = (cpuHasFeature(VIR_ARCH_X86_64, cpuData, "lm") != 1); + cpuDataFree(VIR_ARCH_X86_64, cpuData); } else if (model) { is_32bit = STREQ(model, "qemu32"); } diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 33100fb..bada4a5 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -63,7 +63,6 @@ vmwareCapsInit(void) virCapsGuestPtr guest = NULL; virCPUDefPtr cpu = NULL; union cpuData *data = NULL; - const char *hostarch = NULL; if ((caps = virCapabilitiesNew(virArchFromHost(), 0, 0)) == NULL) @@ -91,8 +90,7 @@ vmwareCapsInit(void) goto error; } - hostarch = virArchToString(caps->host.arch); - if (!(cpu->arch = strdup(hostarch))) { + if (!(cpu->arch = caps->host.arch)) { virReportOOMError(); goto error; } @@ -109,9 +107,9 @@ vmwareCapsInit(void) * - Host CPU is x86_64 with virtualization extensions */ if (caps->host.arch == VIR_ARCH_X86_64 || - (cpuHasFeature(hostarch, data, "lm") && - (cpuHasFeature(hostarch, data, "vmx") || - cpuHasFeature(hostarch, data, "svm")))) { + (cpuHasFeature(caps->host.arch, data, "lm") && + (cpuHasFeature(caps->host.arch, data, "vmx") || + cpuHasFeature(caps->host.arch, data, "svm")))) { if ((guest = virCapabilitiesAddGuest(caps, "hvm", @@ -129,7 +127,7 @@ vmwareCapsInit(void) cleanup: virCPUDefFree(cpu); - cpuDataFree(hostarch, data); + cpuDataFree(caps->host.arch, data); return caps; -- 1.7.11.7

On Tue, Dec 11, 2012 at 14:53:40 +0000, Daniel P. Berrange wrote: ...
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 8cb54a3..2b3b44d 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c ... @@ -740,10 +745,11 @@ virCPUDefIsEqual(virCPUDefPtr src, goto cleanup; }
- if (STRNEQ_NULLABLE(src->arch, dst->arch)) { + if (src->arch != dst->arch) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target CPU arch %s does not match source %s"), - NULLSTR(dst->arch), NULLSTR(src->arch)); + virArchToString(dst->arch), + virArchToString(src->arch));
Hmm, I wonder if VIR_ARCH_NONE should have its name set to "none" rather than "" in patch 1 to provide better error or debugging messages. It should not have any ill side effects I believe.
goto cleanup; }
ACK Jirka

From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 94 ++++++++++++++++++-------------------------- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 40 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f6b53ca..a0fe1c5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -42,7 +42,6 @@ #include <sys/stat.h> #include <unistd.h> #include <sys/wait.h> -#include <sys/utsname.h> #include <stdarg.h> #define VIR_FROM_THIS VIR_FROM_QEMU @@ -209,7 +208,7 @@ struct _qemuCaps { unsigned int version; unsigned int kvmVersion; - char *arch; + virArch arch; size_t ncpuDefinitions; char **cpuDefinitions; @@ -544,14 +543,14 @@ qemuCapsProbeCPUModels(qemuCapsPtr caps, qemuCapsHookDataPtr hookData) qemuCapsParseCPUModels parse; virCommandPtr cmd; - if (STREQ(caps->arch, "i686") || - STREQ(caps->arch, "x86_64")) + if (caps->arch == VIR_ARCH_I686 || + caps->arch == VIR_ARCH_X86_64) parse = qemuCapsParseX86Models; - else if (STREQ(caps->arch, "ppc64")) + else if (caps->arch == VIR_ARCH_PPC64) parse = qemuCapsParsePPCModels; else { VIR_DEBUG("don't know how to parse %s CPU models", - caps->arch); + virArchToString(caps->arch)); return 0; } @@ -599,6 +598,8 @@ qemuCapsFindBinaryForArch(virArch hostarch, ret = virFindFileInPath("qemu"); } else if (guestarch == VIR_ARCH_ITANIUM) { ret = virFindFileInPath("qemu-system-ia64"); + } else if (guestarch == VIR_ARCH_ARMV7L) { + ret = virFindFileInPath("qemu-system-arm"); } else { /* Default case we have matching arch strings */ char *bin; @@ -1596,20 +1597,6 @@ cleanup: return ret; } -static void -uname_normalize(struct utsname *ut) -{ - uname(ut); - - /* Map i386, i486, i586 to i686. */ - if (ut->machine[0] == 'i' && - ut->machine[1] != '\0' && - ut->machine[2] == '8' && - ut->machine[3] == '6' && - ut->machine[4] == '\0') - ut->machine[1] = '6'; -} - int qemuCapsGetDefaultVersion(virCapsPtr caps, qemuCapsCachePtr capsCache, @@ -1677,10 +1664,7 @@ qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps) ret->version = caps->version; ret->kvmVersion = caps->kvmVersion; - - if (caps->arch && - !(ret->arch = strdup(caps->arch))) - goto no_memory; + ret->arch = caps->arch; if (VIR_ALLOC_N(ret->cpuDefinitions, caps->ncpuDefinitions) < 0) goto no_memory; @@ -1717,8 +1701,6 @@ void qemuCapsDispose(void *obj) qemuCapsPtr caps = obj; size_t i; - VIR_FREE(caps->arch); - for (i = 0 ; i < caps->nmachineTypes ; i++) { VIR_FREE(caps->machineTypes[i]); VIR_FREE(caps->machineAliases[i]); @@ -1789,7 +1771,7 @@ const char *qemuCapsGetBinary(qemuCapsPtr caps) return caps->binary; } -const char *qemuCapsGetArch(qemuCapsPtr caps) +virArch qemuCapsGetArch(qemuCapsPtr caps) { return caps->arch; } @@ -2120,6 +2102,18 @@ int qemuCapsProbeQMP(qemuCapsPtr caps, } +static virArch qemuCapsArchFromString(const char *arch) +{ + if (STREQ(arch, "ia64")) + return VIR_ARCH_ITANIUM; + if (STREQ(arch, "i386")) + return VIR_ARCH_I686; + if (STREQ(arch, "arm")) + return VIR_ARCH_ARMV7L; + + return virArchFromString(arch); +} + #define QEMU_SYSTEM_PREFIX "qemu-system-" static int @@ -2130,7 +2124,6 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid) char *help = NULL; int ret = -1; const char *tmp; - struct utsname ut; qemuCapsHookData hookData; VIR_DEBUG("caps=%p", caps); @@ -2139,18 +2132,9 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid) if (tmp) { tmp += strlen(QEMU_SYSTEM_PREFIX); - /* For historical compat we use 'itanium' as arch name */ - if (STREQ(tmp, "ia64")) - tmp = "itanium"; - else if (STREQ(tmp, "i386")) - tmp = "i686"; + caps->arch = qemuCapsArchFromString(tmp); } else { - uname_normalize(&ut); - tmp = ut.machine; - } - if (!(caps->arch = strdup(tmp))) { - virReportOOMError(); - goto cleanup; + caps->arch = virArchFromHost(); } hookData.runUid = runUid; @@ -2171,14 +2155,15 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid) goto cleanup; /* Currently only x86_64 and i686 support PCI-multibus. */ - if (STREQLEN(caps->arch, "x86_64", 6) || - STREQLEN(caps->arch, "i686", 4)) { + if (caps->arch == VIR_ARCH_X86_64 || + caps->arch == VIR_ARCH_I686) { qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); } /* S390 and probably other archs do not support no-acpi - maybe the qemu option parsing should be re-thought. */ - if (STRPREFIX(caps->arch, "s390")) + if (caps->arch == VIR_ARCH_S390 || + caps->arch == VIR_ARCH_S390X) qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); /* qemuCapsExtractDeviceStr will only set additional caps if qemu @@ -2289,6 +2274,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, char *monpath = NULL; char *pidfile = NULL; qemuCapsHookData hookData; + char *archstr; /* the ".sock" sufix is important to avoid a possible clash with a qemu * domain called "capabilities" @@ -2381,26 +2367,24 @@ qemuCapsInitQMP(qemuCapsPtr caps, qemuCapsInitQMPBasic(caps); - if (!(caps->arch = qemuMonitorGetTargetArch(mon))) + archstr = qemuMonitorGetTargetArch(mon); + if ((caps->arch = qemuCapsArchFromString(archstr)) == VIR_ARCH_NONE) { + VIR_DEBUG("Unknown QEMU arch %s", archstr); + ret = 0; + VIR_FREE(archstr); goto cleanup; - - /* Map i386, i486, i586 to i686. */ - if (caps->arch[0] == 'i' && - caps->arch[1] != '\0' && - caps->arch[2] == '8' && - caps->arch[3] == '6' && - caps->arch[4] == '\0') - caps->arch[1] = '6'; + } + VIR_FREE(archstr); /* Currently only x86_64 and i686 support PCI-multibus. */ - if (STREQLEN(caps->arch, "x86_64", 6) || - STREQLEN(caps->arch, "i686", 4)) { + if (caps->arch == VIR_ARCH_X86_64 || + caps->arch == VIR_ARCH_I686) qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); - } /* S390 and probably other archs do not support no-acpi - maybe the qemu option parsing should be re-thought. */ - if (STRPREFIX(caps->arch, "s390")) + if (caps->arch == VIR_ARCH_S390 || + caps->arch == VIR_ARCH_S390X) qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); if (qemuCapsProbeQMPCommands(caps, mon) < 0) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3da8672..c27b48c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -191,7 +191,7 @@ bool qemuCapsGet(qemuCapsPtr caps, char *qemuCapsFlagsString(qemuCapsPtr caps); const char *qemuCapsGetBinary(qemuCapsPtr caps); -const char *qemuCapsGetArch(qemuCapsPtr caps); +virArch qemuCapsGetArch(qemuCapsPtr caps); unsigned int qemuCapsGetVersion(qemuCapsPtr caps); unsigned int qemuCapsGetKVMVersion(qemuCapsPtr caps); int qemuCapsAddCPUDefinition(qemuCapsPtr caps, -- 1.7.11.7

On Tue, Dec 11, 2012 at 14:53:41 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 94 ++++++++++++++++++-------------------------- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 40 insertions(+), 56 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f6b53ca..a0fe1c5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c ... @@ -599,6 +598,8 @@ qemuCapsFindBinaryForArch(virArch hostarch, ret = virFindFileInPath("qemu"); } else if (guestarch == VIR_ARCH_ITANIUM) { ret = virFindFileInPath("qemu-system-ia64"); + } else if (guestarch == VIR_ARCH_ARMV7L) { + ret = virFindFileInPath("qemu-system-arm"); } else { /* Default case we have matching arch strings */ char *bin;
I think this hunk should go to 4/6. Actually, it would be even better to create a reverse mapping function to qemuCapsArchFromString and call it here to keep the qemu special naming at one place. ...
@@ -2120,6 +2102,18 @@ int qemuCapsProbeQMP(qemuCapsPtr caps, }
+static virArch qemuCapsArchFromString(const char *arch) +{ + if (STREQ(arch, "ia64")) + return VIR_ARCH_ITANIUM; + if (STREQ(arch, "i386")) + return VIR_ARCH_I686; + if (STREQ(arch, "arm")) + return VIR_ARCH_ARMV7L; + + return virArchFromString(arch); +} + #define QEMU_SYSTEM_PREFIX "qemu-system-"
static int ... @@ -2381,26 +2367,24 @@ qemuCapsInitQMP(qemuCapsPtr caps,
qemuCapsInitQMPBasic(caps);
- if (!(caps->arch = qemuMonitorGetTargetArch(mon))) + archstr = qemuMonitorGetTargetArch(mon);
This ignores possible failure in qemuMonitorGetTargetArch(); neither qemuCapsArchFromString nor virArchFromString work with NULL arch.
+ if ((caps->arch = qemuCapsArchFromString(archstr)) == VIR_ARCH_NONE) { + VIR_DEBUG("Unknown QEMU arch %s", archstr); + ret = 0;
Why is this non-fatal?
+ VIR_FREE(archstr); goto cleanup; - - /* Map i386, i486, i586 to i686. */ - if (caps->arch[0] == 'i' && - caps->arch[1] != '\0' && - caps->arch[2] == '8' && - caps->arch[3] == '6' && - caps->arch[4] == '\0') - caps->arch[1] = '6'; + } + VIR_FREE(archstr);
/* Currently only x86_64 and i686 support PCI-multibus. */ - if (STREQLEN(caps->arch, "x86_64", 6) || - STREQLEN(caps->arch, "i686", 4)) { + if (caps->arch == VIR_ARCH_X86_64 || + caps->arch == VIR_ARCH_I686) qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); - }
/* S390 and probably other archs do not support no-acpi - maybe the qemu option parsing should be re-thought. */ - if (STRPREFIX(caps->arch, "s390")) + if (caps->arch == VIR_ARCH_S390 || + caps->arch == VIR_ARCH_S390X) qemuCapsClear(caps, QEMU_CAPS_NO_ACPI);
if (qemuCapsProbeQMPCommands(caps, mon) < 0) ...
Jirka

From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 155 +++++++++++++++++++++---------------------- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 76 insertions(+), 81 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6a02161..ae75bc6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -42,7 +42,6 @@ #include <sys/stat.h> #include <unistd.h> #include <sys/wait.h> -#include <sys/utsname.h> #include <stdarg.h> #define VIR_FROM_THIS VIR_FROM_QEMU @@ -215,7 +214,7 @@ struct _qemuCaps { unsigned int version; unsigned int kvmVersion; - char *arch; + virArch arch; size_t ncpuDefinitions; char **cpuDefinitions; @@ -250,6 +249,32 @@ static int qemuCapsOnceInit(void) VIR_ONCE_GLOBAL_INIT(qemuCaps) +static virArch qemuCapsArchFromString(const char *arch) +{ + if (STREQ(arch, "ia64")) + return VIR_ARCH_ITANIUM; + if (STREQ(arch, "i386")) + return VIR_ARCH_I686; + if (STREQ(arch, "arm")) + return VIR_ARCH_ARMV7L; + + return virArchFromString(arch); +} + + +static const char *qemuCapsArchToString(virArch arch) +{ + if (arch == VIR_ARCH_I686) + return "i386"; + else if (arch == VIR_ARCH_ITANIUM) + return "ia64"; + else if (arch == VIR_ARCH_ARMV7L) + return "arm"; + + return virArchToString(arch); +} + + struct _qemuCapsHookData { uid_t runUid; gid_t runGid; @@ -550,14 +575,14 @@ qemuCapsProbeCPUModels(qemuCapsPtr caps, qemuCapsHookDataPtr hookData) qemuCapsParseCPUModels parse; virCommandPtr cmd; - if (STREQ(caps->arch, "i686") || - STREQ(caps->arch, "x86_64")) + if (caps->arch == VIR_ARCH_I686 || + caps->arch == VIR_ARCH_X86_64) parse = qemuCapsParseX86Models; - else if (STREQ(caps->arch, "ppc64")) + else if (caps->arch == VIR_ARCH_PPC64) parse = qemuCapsParsePPCModels; else { VIR_DEBUG("don't know how to parse %s CPU models", - caps->arch); + virArchToString(caps->arch)); return 0; } @@ -586,37 +611,34 @@ qemuCapsFindBinaryForArch(virArch hostarch, virArch guestarch) { char *ret; + const char *archstr = qemuCapsArchToString(guestarch); + char *binary; - /* Some special cases where libvirt's canonical - * guestarch string form can't match qemu's binary - */ - if (guestarch == VIR_ARCH_I686) { - ret = virFindFileInPath("qemu-system-i386"); + if (virAsprintf(&binary, "qemu-system-%s", archstr) < 0) { + virReportOOMError(); + return NULL; + } + + ret = virFindFileInPath(binary); + VIR_FREE(binary); + if (ret && !virFileIsExecutable(ret)) + VIR_FREE(ret); + + if (guestarch == VIR_ARCH_I686 && + !ret && + hostarch == VIR_ARCH_X86_64) { + ret = virFindFileInPath("qemu-system-x86_64"); if (ret && !virFileIsExecutable(ret)) VIR_FREE(ret); + } - if (!ret && hostarch == VIR_ARCH_X86_64) { - ret = virFindFileInPath("qemu-system-x86_64"); - if (ret && !virFileIsExecutable(ret)) - VIR_FREE(ret); - } - - if (!ret) - ret = virFindFileInPath("qemu"); - } else if (guestarch == VIR_ARCH_ITANIUM) { - ret = virFindFileInPath("qemu-system-ia64"); - } else { - /* Default case we have matching arch strings */ - char *bin; - if (virAsprintf(&bin, "qemu-system-%s", virArchToString(guestarch)) < 0) { - virReportOOMError(); - return NULL; - } - ret = virFindFileInPath(bin); - VIR_FREE(bin); + if (guestarch == VIR_ARCH_I686 && + !ret) { + ret = virFindFileInPath("qemu"); + if (ret && !virFileIsExecutable(ret)) + VIR_FREE(ret); } - if (ret && !virFileIsExecutable(ret)) - VIR_FREE(ret); + return ret; } @@ -1608,20 +1630,6 @@ cleanup: return ret; } -static void -uname_normalize(struct utsname *ut) -{ - uname(ut); - - /* Map i386, i486, i586 to i686. */ - if (ut->machine[0] == 'i' && - ut->machine[1] != '\0' && - ut->machine[2] == '8' && - ut->machine[3] == '6' && - ut->machine[4] == '\0') - ut->machine[1] = '6'; -} - int qemuCapsGetDefaultVersion(virCapsPtr caps, qemuCapsCachePtr capsCache, @@ -1689,10 +1697,7 @@ qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps) ret->version = caps->version; ret->kvmVersion = caps->kvmVersion; - - if (caps->arch && - !(ret->arch = strdup(caps->arch))) - goto no_memory; + ret->arch = caps->arch; if (VIR_ALLOC_N(ret->cpuDefinitions, caps->ncpuDefinitions) < 0) goto no_memory; @@ -1729,8 +1734,6 @@ void qemuCapsDispose(void *obj) qemuCapsPtr caps = obj; size_t i; - VIR_FREE(caps->arch); - for (i = 0 ; i < caps->nmachineTypes ; i++) { VIR_FREE(caps->machineTypes[i]); VIR_FREE(caps->machineAliases[i]); @@ -1801,7 +1804,7 @@ const char *qemuCapsGetBinary(qemuCapsPtr caps) return caps->binary; } -const char *qemuCapsGetArch(qemuCapsPtr caps) +virArch qemuCapsGetArch(qemuCapsPtr caps) { return caps->arch; } @@ -2142,7 +2145,6 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid) char *help = NULL; int ret = -1; const char *tmp; - struct utsname ut; qemuCapsHookData hookData; VIR_DEBUG("caps=%p", caps); @@ -2151,18 +2153,9 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid) if (tmp) { tmp += strlen(QEMU_SYSTEM_PREFIX); - /* For historical compat we use 'itanium' as arch name */ - if (STREQ(tmp, "ia64")) - tmp = "itanium"; - else if (STREQ(tmp, "i386")) - tmp = "i686"; + caps->arch = qemuCapsArchFromString(tmp); } else { - uname_normalize(&ut); - tmp = ut.machine; - } - if (!(caps->arch = strdup(tmp))) { - virReportOOMError(); - goto cleanup; + caps->arch = virArchFromHost(); } hookData.runUid = runUid; @@ -2183,14 +2176,15 @@ qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid) goto cleanup; /* Currently only x86_64 and i686 support PCI-multibus. */ - if (STREQLEN(caps->arch, "x86_64", 6) || - STREQLEN(caps->arch, "i686", 4)) { + if (caps->arch == VIR_ARCH_X86_64 || + caps->arch == VIR_ARCH_I686) { qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); } /* S390 and probably other archs do not support no-acpi - maybe the qemu option parsing should be re-thought. */ - if (STRPREFIX(caps->arch, "s390")) + if (caps->arch == VIR_ARCH_S390 || + caps->arch == VIR_ARCH_S390X) qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); /* qemuCapsExtractDeviceStr will only set additional caps if qemu @@ -2302,6 +2296,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, char *monpath = NULL; char *pidfile = NULL; qemuCapsHookData hookData; + char *archstr; /* the ".sock" sufix is important to avoid a possible clash with a qemu * domain called "capabilities" @@ -2394,26 +2389,26 @@ qemuCapsInitQMP(qemuCapsPtr caps, qemuCapsInitQMPBasic(caps); - if (!(caps->arch = qemuMonitorGetTargetArch(mon))) + if (!(archstr = qemuMonitorGetTargetArch(mon))) goto cleanup; - /* Map i386, i486, i586 to i686. */ - if (caps->arch[0] == 'i' && - caps->arch[1] != '\0' && - caps->arch[2] == '8' && - caps->arch[3] == '6' && - caps->arch[4] == '\0') - caps->arch[1] = '6'; + if ((caps->arch = qemuCapsArchFromString(archstr)) == VIR_ARCH_NONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown QEMU arch %s"), archstr); + VIR_FREE(archstr); + goto cleanup; + } + VIR_FREE(archstr); /* Currently only x86_64 and i686 support PCI-multibus. */ - if (STREQLEN(caps->arch, "x86_64", 6) || - STREQLEN(caps->arch, "i686", 4)) { + if (caps->arch == VIR_ARCH_X86_64 || + caps->arch == VIR_ARCH_I686) qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); - } /* S390 and probably other archs do not support no-acpi - maybe the qemu option parsing should be re-thought. */ - if (STRPREFIX(caps->arch, "s390")) + if (caps->arch == VIR_ARCH_S390 || + caps->arch == VIR_ARCH_S390X) qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); if (qemuCapsProbeQMPCommands(caps, mon) < 0) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index bf4eef8..700b0ac 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -197,7 +197,7 @@ bool qemuCapsGet(qemuCapsPtr caps, char *qemuCapsFlagsString(qemuCapsPtr caps); const char *qemuCapsGetBinary(qemuCapsPtr caps); -const char *qemuCapsGetArch(qemuCapsPtr caps); +virArch qemuCapsGetArch(qemuCapsPtr caps); unsigned int qemuCapsGetVersion(qemuCapsPtr caps); unsigned int qemuCapsGetKVMVersion(qemuCapsPtr caps); int qemuCapsAddCPUDefinition(qemuCapsPtr caps, -- 1.7.11.7

On Tue, Dec 18, 2012 at 14:09:16 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 155 +++++++++++++++++++++---------------------- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 76 insertions(+), 81 deletions(-)
ACK Jirka

On Tue, Dec 11, 2012 at 02:53:35PM +0000, Daniel P. Berrange wrote:
Different operating systems have different names for architectures. eg x86_64 x64, amd64. This can cause inconsistency in libvirt code dealing with architectures. To deal with this define an enum for valid architectures and a canonical string mapping. Update all code internally to use the enum instead of 'char *' for arch data.
Sounds a good idea but a bit late for 1.0.1 Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

On Tue, Dec 11, 2012 at 11:36:35PM +0800, Daniel Veillard wrote:
On Tue, Dec 11, 2012 at 02:53:35PM +0000, Daniel P. Berrange wrote:
Different operating systems have different names for architectures. eg x86_64 x64, amd64. This can cause inconsistency in libvirt code dealing with architectures. To deal with this define an enum for valid architectures and a canonical string mapping. Update all code internally to use the enum instead of 'char *' for arch data.
Sounds a good idea but a bit late for 1.0.1
Yep, it isn't required for this release Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Dec 11, 2012 at 11:36:35PM +0800, Daniel Veillard wrote:
On Tue, Dec 11, 2012 at 02:53:35PM +0000, Daniel P. Berrange wrote:
Different operating systems have different names for architectures. eg x86_64 x64, amd64. This can cause inconsistency in libvirt code dealing with architectures. To deal with this define an enum for valid architectures and a canonical string mapping. Update all code internally to use the enum instead of 'char *' for arch data.
Sounds a good idea but a bit late for 1.0.1
Be nice to have this reviewed now that 1.0.1 is out 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 :|
participants (5)
-
Daniel P. Berrange
-
Daniel Veillard
-
Eric Blake
-
Jiri Denemark
-
Richard W.M. Jones