[libvirt] [PATCH 0/3] Drop some useless CPU code
Jiri Denemark (3): cpu: Consolidate ARM drivers cpu: Drop generic driver cpu: Drop NR_DRIVERS macro po/POTFILES.in | 1 - src/Makefile.am | 2 - src/cpu/cpu.c | 16 ++-- src/cpu/cpu_aarch64.c | 133 ----------------------------- src/cpu/cpu_aarch64.h | 31 ------- src/cpu/cpu_arm.c | 37 +++++---- src/cpu/cpu_generic.c | 226 -------------------------------------------------- src/cpu/cpu_generic.h | 32 ------- 8 files changed, 26 insertions(+), 452 deletions(-) delete mode 100644 src/cpu/cpu_aarch64.c delete mode 100644 src/cpu/cpu_aarch64.h delete mode 100644 src/cpu/cpu_generic.c delete mode 100644 src/cpu/cpu_generic.h -- 2.9.0
Both ARM and AArch64 drivers are exactly the same (modulo function names). Let's use just one driver for all ARM architectures. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/Makefile.am | 1 - src/cpu/cpu.c | 2 - src/cpu/cpu_aarch64.c | 133 -------------------------------------------------- src/cpu/cpu_aarch64.h | 31 ------------ src/cpu/cpu_arm.c | 37 +++++++------- 5 files changed, 20 insertions(+), 184 deletions(-) delete mode 100644 src/cpu/cpu_aarch64.c delete mode 100644 src/cpu/cpu_aarch64.h diff --git a/src/Makefile.am b/src/Makefile.am index 9f8b638..b91ff74 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1080,7 +1080,6 @@ CPU_SOURCES = \ cpu/cpu_x86.h cpu/cpu_x86.c cpu/cpu_x86_data.h \ cpu/cpu_s390.h cpu/cpu_s390.c \ cpu/cpu_arm.h cpu/cpu_arm.c \ - cpu/cpu_aarch64.h cpu/cpu_aarch64.c \ cpu/cpu_ppc64.h cpu/cpu_ppc64.c \ cpu/cpu_ppc64_data.h \ cpu/cpu_map.h cpu/cpu_map.c diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 7decc74..62a420f 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -32,7 +32,6 @@ #include "cpu_ppc64.h" #include "cpu_s390.h" #include "cpu_arm.h" -#include "cpu_aarch64.h" #include "cpu_generic.h" #include "util/virstring.h" @@ -47,7 +46,6 @@ static struct cpuArchDriver *drivers[] = { &cpuDriverPPC64, &cpuDriverS390, &cpuDriverArm, - &cpuDriverAARCH64, /* generic driver must always be the last one */ &cpuDriverGeneric }; diff --git a/src/cpu/cpu_aarch64.c b/src/cpu/cpu_aarch64.c deleted file mode 100644 index e6d5f53..0000000 --- a/src/cpu/cpu_aarch64.c +++ /dev/null @@ -1,133 +0,0 @@ -/* - * cpu_aarch64.c: CPU driver for AArch64 CPUs - * - * - * 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/>. - * - * Authors: - * Anup Patel <anup.patel@linaro.org> - * Pranavkumar Sawargaonkar <pranavkumar@linaro.org> - */ - -#include <config.h> - -#include "viralloc.h" -#include "cpu.h" -#include "virstring.h" - -#define VIR_FROM_THIS VIR_FROM_CPU - -static const virArch archs[] = { VIR_ARCH_AARCH64 }; - -static virCPUDataPtr -AArch64NodeData(virArch arch) -{ - virCPUDataPtr data; - - if (VIR_ALLOC(data) < 0) - return NULL; - - data->arch = arch; - - return data; -} - -static int -AArch64Decode(virCPUDefPtr cpu, - const virCPUData *data ATTRIBUTE_UNUSED, - const char **models ATTRIBUTE_UNUSED, - unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED, - unsigned int flags) -{ - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); - - if (cpu->model == NULL && - VIR_STRDUP(cpu->model, "host") < 0) - return -1; - - return 0; -} - -static void -AArch64DataFree(virCPUDataPtr data) -{ - VIR_FREE(data); -} - -static int -AArch64Update(virCPUDefPtr guest, - const virCPUDef *host) -{ - guest->match = VIR_CPU_MATCH_EXACT; - virCPUDefFreeModel(guest); - return virCPUDefCopyModel(guest, host, true); -} - -static virCPUCompareResult -AArch64GuestData(virCPUDefPtr host ATTRIBUTE_UNUSED, - virCPUDefPtr guest ATTRIBUTE_UNUSED, - virCPUDataPtr *data ATTRIBUTE_UNUSED, - char **message ATTRIBUTE_UNUSED) -{ - return VIR_CPU_COMPARE_IDENTICAL; -} - -static virCPUDefPtr -AArch64Baseline(virCPUDefPtr *cpus, - unsigned int ncpus ATTRIBUTE_UNUSED, - const char **models ATTRIBUTE_UNUSED, - unsigned int nmodels ATTRIBUTE_UNUSED, - unsigned int flags) -{ - virCPUDefPtr cpu = NULL; - - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | - VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); - - if (VIR_ALLOC(cpu) < 0 || - VIR_STRDUP(cpu->model, cpus[0]->model) < 0) { - virCPUDefFree(cpu); - return NULL; - } - - cpu->type = VIR_CPU_TYPE_GUEST; - cpu->match = VIR_CPU_MATCH_EXACT; - - return cpu; -} - -static virCPUCompareResult -AArch64Compare(virCPUDefPtr host ATTRIBUTE_UNUSED, - virCPUDefPtr cpu ATTRIBUTE_UNUSED, - bool failIncompatible ATTRIBUTE_UNUSED) -{ - return VIR_CPU_COMPARE_IDENTICAL; -} - -struct cpuArchDriver cpuDriverAARCH64 = { - .name = "aarch64", - .arch = archs, - .narch = ARRAY_CARDINALITY(archs), - .compare = AArch64Compare, - .decode = AArch64Decode, - .encode = NULL, - .free = AArch64DataFree, - .nodeData = AArch64NodeData, - .guestData = AArch64GuestData, - .baseline = AArch64Baseline, - .update = AArch64Update, - .hasFeature = NULL, -}; diff --git a/src/cpu/cpu_aarch64.h b/src/cpu/cpu_aarch64.h deleted file mode 100644 index 8e48368..0000000 --- a/src/cpu/cpu_aarch64.h +++ /dev/null @@ -1,31 +0,0 @@ -/* - * cpu_aarch64.h: CPU driver for AArch64 CPUs - * - * - * 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/>. - * - * Authors: - * Anup Patel <anup.patel@linaro.org> - * Pravakumar Sawargaonkar <pranavkumar@linaro.org> - */ - -#ifndef __VIR_CPU_AARCH64_H__ -# define __VIR_CPU_AARCH64_H__ - -# include "cpu.h" - -extern struct cpuArchDriver cpuDriverAARCH64; - -#endif /* __VIR_CPU_AARCH64_H__ */ diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 0403a8b..6090253 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -30,12 +30,15 @@ #define VIR_FROM_THIS VIR_FROM_CPU -static const virArch archs[] = {VIR_ARCH_ARMV6L, - VIR_ARCH_ARMV7B, - VIR_ARCH_ARMV7L}; +static const virArch archs[] = { + VIR_ARCH_ARMV6L, + VIR_ARCH_ARMV7B, + VIR_ARCH_ARMV7L, + VIR_ARCH_AARCH64, +}; static virCPUDataPtr -ArmNodeData(virArch arch) +armNodeData(virArch arch) { virCPUDataPtr data; @@ -48,7 +51,7 @@ ArmNodeData(virArch arch) } static int -ArmDecode(virCPUDefPtr cpu, +armDecode(virCPUDefPtr cpu, const virCPUData *data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, @@ -65,13 +68,13 @@ ArmDecode(virCPUDefPtr cpu, } static void -ArmDataFree(virCPUDataPtr data) +armDataFree(virCPUDataPtr data) { VIR_FREE(data); } static int -ArmUpdate(virCPUDefPtr guest, +armUpdate(virCPUDefPtr guest, const virCPUDef *host) { guest->match = VIR_CPU_MATCH_EXACT; @@ -80,7 +83,7 @@ ArmUpdate(virCPUDefPtr guest, } static virCPUCompareResult -ArmGuestData(virCPUDefPtr host ATTRIBUTE_UNUSED, +armGuestData(virCPUDefPtr host ATTRIBUTE_UNUSED, virCPUDefPtr guest ATTRIBUTE_UNUSED, virCPUDataPtr *data ATTRIBUTE_UNUSED, char **message ATTRIBUTE_UNUSED) @@ -89,7 +92,7 @@ ArmGuestData(virCPUDefPtr host ATTRIBUTE_UNUSED, } static virCPUDefPtr -ArmBaseline(virCPUDefPtr *cpus, +armBaseline(virCPUDefPtr *cpus, unsigned int ncpus ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, @@ -113,7 +116,7 @@ ArmBaseline(virCPUDefPtr *cpus, } static virCPUCompareResult -ArmCompare(virCPUDefPtr host ATTRIBUTE_UNUSED, +armCompare(virCPUDefPtr host ATTRIBUTE_UNUSED, virCPUDefPtr cpu ATTRIBUTE_UNUSED, bool failMessages ATTRIBUTE_UNUSED) { @@ -124,13 +127,13 @@ struct cpuArchDriver cpuDriverArm = { .name = "arm", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = ArmCompare, - .decode = ArmDecode, + .compare = armCompare, + .decode = armDecode, .encode = NULL, - .free = ArmDataFree, - .nodeData = ArmNodeData, - .guestData = ArmGuestData, - .baseline = ArmBaseline, - .update = ArmUpdate, + .free = armDataFree, + .nodeData = armNodeData, + .guestData = armGuestData, + .baseline = armBaseline, + .update = armUpdate, .hasFeature = NULL, }; -- 2.9.0
On Fri, Jun 24, 2016 at 07:36:00PM +0200, Jiri Denemark wrote:
Both ARM and AArch64 drivers are exactly the same (modulo function names). Let's use just one driver for all ARM architectures.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
ACK
On Fri, Jun 24, 2016 at 23:51:35 +0200, Pavel Hrdina wrote:
On Fri, Jun 24, 2016 at 07:36:00PM +0200, Jiri Denemark wrote:
Both ARM and AArch64 drivers are exactly the same (modulo function names). Let's use just one driver for all ARM architectures.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
ACK
Pushed, thanks. Jirka
Pretending (partial) support for something we don't understand is risky. Reporting a failure is much better. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- po/POTFILES.in | 1 - src/Makefile.am | 1 - src/cpu/cpu.c | 13 ++- src/cpu/cpu_generic.c | 226 -------------------------------------------------- src/cpu/cpu_generic.h | 32 ------- 5 files changed, 6 insertions(+), 267 deletions(-) delete mode 100644 src/cpu/cpu_generic.c delete mode 100644 src/cpu/cpu_generic.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 67838f5..c18bf70 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -41,7 +41,6 @@ src/conf/virchrdev.c src/conf/virdomainobjlist.c src/conf/virsecretobj.c src/cpu/cpu.c -src/cpu/cpu_generic.c src/cpu/cpu_map.c src/cpu/cpu_ppc64.c src/cpu/cpu_x86.c diff --git a/src/Makefile.am b/src/Makefile.am index b91ff74..3474acc 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1076,7 +1076,6 @@ NODE_DEVICE_DRIVER_UDEV_SOURCES = \ CPU_SOURCES = \ cpu/cpu.h cpu/cpu.c \ - cpu/cpu_generic.h cpu/cpu_generic.c \ cpu/cpu_x86.h cpu/cpu_x86.c cpu/cpu_x86_data.h \ cpu/cpu_s390.h cpu/cpu_s390.c \ cpu/cpu_arm.h cpu/cpu_arm.c \ diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 62a420f..c8d50e7 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -32,7 +32,6 @@ #include "cpu_ppc64.h" #include "cpu_s390.h" #include "cpu_arm.h" -#include "cpu_generic.h" #include "util/virstring.h" @@ -46,8 +45,6 @@ static struct cpuArchDriver *drivers[] = { &cpuDriverPPC64, &cpuDriverS390, &cpuDriverArm, - /* generic driver must always be the last one */ - &cpuDriverGeneric }; @@ -63,15 +60,17 @@ cpuGetSubDriver(virArch arch) return NULL; } - for (i = 0; i < NR_DRIVERS - 1; i++) { + for (i = 0; i < NR_DRIVERS; i++) { for (j = 0; j < drivers[i]->narch; j++) { if (arch == drivers[i]->arch[j]) return drivers[i]; } } - /* use generic driver by default */ - return drivers[NR_DRIVERS - 1]; + virReportError(VIR_ERR_NO_SUPPORT, + _("'%s' architecture is not supported by CPU driver"), + virArchToString(arch)); + return NULL; } @@ -80,7 +79,7 @@ cpuGetSubDriverByName(const char *name) { size_t i; - for (i = 0; i < NR_DRIVERS - 1; i++) { + for (i = 0; i < NR_DRIVERS; i++) { if (STREQ_NULLABLE(name, drivers[i]->name)) return drivers[i]; } diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c deleted file mode 100644 index f26a62d..0000000 --- a/src/cpu/cpu_generic.c +++ /dev/null @@ -1,226 +0,0 @@ -/* - * cpu_generic.c: CPU manipulation driver for architectures which are not - * handled by their own driver - * - * Copyright (C) 2009-2011 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/>. - * - * Authors: - * Jiri Denemark <jdenemar@redhat.com> - */ - -#include <config.h> - -#include "viralloc.h" -#include "virhash.h" -#include "cpu.h" -#include "cpu_generic.h" -#include "virstring.h" - -#define VIR_FROM_THIS VIR_FROM_CPU - - -static virHashTablePtr -genericHashFeatures(virCPUDefPtr cpu) -{ - virHashTablePtr hash; - size_t i; - - if ((hash = virHashCreate(cpu->nfeatures, NULL)) == NULL) - return NULL; - - for (i = 0; i < cpu->nfeatures; i++) { - if (virHashAddEntry(hash, - cpu->features[i].name, - cpu->features + i)) { - virHashFree(hash); - return NULL; - } - } - - return hash; -} - - -static virCPUCompareResult -genericCompare(virCPUDefPtr host, - virCPUDefPtr cpu, - bool failIncompatible) -{ - virHashTablePtr hash = NULL; - virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; - size_t i; - unsigned int reqfeatures; - - if (!cpu->model) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("no guest CPU model specified")); - goto cleanup; - } - - if ((cpu->arch != VIR_ARCH_NONE && - host->arch != cpu->arch) || - STRNEQ(host->model, cpu->model)) { - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - goto cleanup; - } - - if ((hash = genericHashFeatures(host)) == NULL) - goto cleanup; - - reqfeatures = 0; - for (i = 0; i < cpu->nfeatures; i++) { - void *hval = virHashLookup(hash, cpu->features[i].name); - - if (hval) { - if (cpu->type == VIR_CPU_TYPE_GUEST && - cpu->features[i].policy == VIR_CPU_FEATURE_FORBID) { - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - goto cleanup; - } - reqfeatures++; - } else if (cpu->type == VIR_CPU_TYPE_HOST || - cpu->features[i].policy == VIR_CPU_FEATURE_REQUIRE) { - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - goto cleanup; - } - } - - if (host->nfeatures > reqfeatures) { - if (cpu->type == VIR_CPU_TYPE_GUEST && - cpu->match == VIR_CPU_MATCH_STRICT) - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - else - ret = VIR_CPU_COMPARE_SUPERSET; - } else { - ret = VIR_CPU_COMPARE_IDENTICAL; - } - - cleanup: - virHashFree(hash); - if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { - ret = VIR_CPU_COMPARE_ERROR; - virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); - } - return ret; -} - - -static virCPUDefPtr -genericBaseline(virCPUDefPtr *cpus, - unsigned int ncpus, - const char **models, - unsigned int nmodels, - unsigned int flags) -{ - virCPUDefPtr cpu = NULL; - virCPUFeatureDefPtr features = NULL; - unsigned int nfeatures; - unsigned int count; - size_t i, j; - - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | - VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); - - if (!cpuModelIsAllowed(cpus[0]->model, models, nmodels)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("CPU model %s is not supported by hypervisor"), - cpus[0]->model); - goto error; - } - - if (VIR_ALLOC(cpu) < 0 || - VIR_STRDUP(cpu->model, cpus[0]->model) < 0 || - VIR_ALLOC_N(features, cpus[0]->nfeatures) < 0) - goto error; - - cpu->arch = cpus[0]->arch; - cpu->type = VIR_CPU_TYPE_HOST; - - count = nfeatures = cpus[0]->nfeatures; - for (i = 0; i < nfeatures; i++) - features[i].name = cpus[0]->features[i].name; - - for (i = 1; i < ncpus; i++) { - virHashTablePtr hash; - - if (cpu->arch != cpus[i]->arch) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("CPUs have incompatible architectures: '%s' != '%s'"), - virArchToString(cpu->arch), - virArchToString(cpus[i]->arch)); - goto error; - } - - if (STRNEQ(cpu->model, cpus[i]->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("CPU models don't match: '%s' != '%s'"), - cpu->model, cpus[i]->model); - goto error; - } - - if (!(hash = genericHashFeatures(cpus[i]))) - goto error; - - for (j = 0; j < nfeatures; j++) { - if (features[j].name && - !virHashLookup(hash, features[j].name)) { - features[j].name = NULL; - count--; - } - } - - virHashFree(hash); - } - - if (VIR_ALLOC_N(cpu->features, count) < 0) - goto error; - cpu->nfeatures = count; - - j = 0; - for (i = 0; i < nfeatures; i++) { - if (!features[i].name) - continue; - - if (VIR_STRDUP(cpu->features[j++].name, features[i].name) < 0) - goto error; - } - - cleanup: - VIR_FREE(features); - - return cpu; - - error: - virCPUDefFree(cpu); - cpu = NULL; - goto cleanup; -} - - -struct cpuArchDriver cpuDriverGeneric = { - .name = "generic", - .arch = NULL, - .narch = 0, - .compare = genericCompare, - .decode = NULL, - .encode = NULL, - .free = NULL, - .nodeData = NULL, - .guestData = NULL, - .baseline = genericBaseline, - .update = NULL, -}; diff --git a/src/cpu/cpu_generic.h b/src/cpu/cpu_generic.h deleted file mode 100644 index c7c66cc..0000000 --- a/src/cpu/cpu_generic.h +++ /dev/null @@ -1,32 +0,0 @@ -/* - * cpu_generic.h: CPU manipulation driver for architectures which are not - * handled by their own driver - * - * Copyright (C) 2009 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/>. - * - * Authors: - * Jiri Denemark <jdenemar@redhat.com> - */ - -#ifndef __VIR_CPU_GENERIC_H__ -# define __VIR_CPU_GENERIC_H__ - -# include "cpu.h" - -extern struct cpuArchDriver cpuDriverGeneric; - -#endif /* __VIR_CPU_GENERIC_H__ */ -- 2.9.0
On Fri, Jun 24, 2016 at 07:36:01PM +0200, Jiri Denemark wrote:
Pretending (partial) support for something we don't understand is risky. Reporting a failure is much better.
I agree that pretending support is risky, but I'm not sure about this. For example it seems that someone uses libvirt on VIR_ARCH_OR32 (commit c88bf5726) which should be probably included in cpu_arm driver. There are also some other archs (VIR_ARCH_ALPHA, VIR_ARCH_PPCEMB, VIR_ARCH_SH4, VIR_ARCH_SH4EB, VIR_ARCH_SPARC and VIR_ARCH_SPARC64 qemu_domain.c) that wouldn't be supported. I'm not an expert in this area, I'm just asking to make sure that we don't break anything by removing the cpu_generic driver. Pavel
On Fri, Jun 24, 2016 at 23:50:43 +0200, Pavel Hrdina wrote:
On Fri, Jun 24, 2016 at 07:36:01PM +0200, Jiri Denemark wrote:
Pretending (partial) support for something we don't understand is risky. Reporting a failure is much better.
I agree that pretending support is risky, but I'm not sure about this. For example it seems that someone uses libvirt on VIR_ARCH_OR32 (commit c88bf5726) which should be probably included in cpu_arm driver. There are also some other archs (VIR_ARCH_ALPHA, VIR_ARCH_PPCEMB, VIR_ARCH_SH4, VIR_ARCH_SH4EB, VIR_ARCH_SPARC and VIR_ARCH_SPARC64 qemu_domain.c) that wouldn't be supported.
I'm not an expert in this area, I'm just asking to make sure that we don't break anything by removing the cpu_generic driver.
One would think so, but in reality the generic driver was pretty useless anyway. The only CPU driver APIs it implemented were baseline and compare. Both of them expect that libvirt can report host CPU definition. It's not a strict requirement for baseline, since one can create the input XMLs even by hand, but compare has nothing to compare the supplied XML with if libvirt cannot detect a host CPU. And since the host CPU detection APIs were not implemented in generic driver, both baseline and compare could not really be used. For the more common case of just starting a domain of one of those architectures with a CPU model specified the following combinations are possible: - KVM (host is the same arch as guest) -- did not work due to the missing host CPU detection APIs - TCG when host arch == guest arch -- did not work for the same reason and because the qemu driver only allows a guest CPU to be selected when libvirt know what CPU model the host has (yes, this is wrong and I have a patch for it in my WIP series) - TCG when host arch is one of those supported by our CPU driver -- worked and will work because libvirt knows what CPU the host has (yes, this is even worse since the host CPU is completely irrelevant and I have a patch for this too) and the guest-arch's CPU driver is not used at all in this case (wrong too, patch in progress) In other words, the CPU driver and its interactions with QEMU driver is hopelessly broken and this patch does not make it any worse :-) Libvirt 2.1 should be much better in this respect I believe. Jirka
On Fri, Jun 24, 2016 at 07:36:01PM +0200, Jiri Denemark wrote:
Pretending (partial) support for something we don't understand is risky. Reporting a failure is much better.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- po/POTFILES.in | 1 - src/Makefile.am | 1 - src/cpu/cpu.c | 13 ++- src/cpu/cpu_generic.c | 226 -------------------------------------------------- src/cpu/cpu_generic.h | 32 ------- 5 files changed, 6 insertions(+), 267 deletions(-) delete mode 100644 src/cpu/cpu_generic.c delete mode 100644 src/cpu/cpu_generic.h
ACK Jan
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index c8d50e7..11a68f0 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -35,7 +35,6 @@ #include "util/virstring.h" -#define NR_DRIVERS ARRAY_CARDINALITY(drivers) #define VIR_FROM_THIS VIR_FROM_CPU VIR_LOG_INIT("cpu.cpu"); @@ -60,7 +59,7 @@ cpuGetSubDriver(virArch arch) return NULL; } - for (i = 0; i < NR_DRIVERS; i++) { + for (i = 0; i < ARRAY_CARDINALITY(drivers); i++) { for (j = 0; j < drivers[i]->narch; j++) { if (arch == drivers[i]->arch[j]) return drivers[i]; @@ -79,7 +78,7 @@ cpuGetSubDriverByName(const char *name) { size_t i; - for (i = 0; i < NR_DRIVERS; i++) { + for (i = 0; i < ARRAY_CARDINALITY(drivers); i++) { if (STREQ_NULLABLE(name, drivers[i]->name)) return drivers[i]; } -- 2.9.0
participants (3)
-
Jiri Denemark -
Ján Tomko -
Pavel Hrdina