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