From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 47BE2C433EF for ; Thu, 11 Nov 2021 09:33:21 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E8118610F8 for ; Thu, 11 Nov 2021 09:33:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org E8118610F8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:36470 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ml6SO-0002m8-2V for qemu-devel@archiver.kernel.org; Thu, 11 Nov 2021 04:33:20 -0500 Received: from eggs.gnu.org ([209.51.188.92]:57372) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ml6Qo-0000dk-63 for qemu-devel@nongnu.org; Thu, 11 Nov 2021 04:31:42 -0500 Received: from szxga08-in.huawei.com ([45.249.212.255]:2896) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ml6Ql-0005NI-13 for qemu-devel@nongnu.org; Thu, 11 Nov 2021 04:31:41 -0500 Received: from dggemv711-chm.china.huawei.com (unknown [172.30.72.56]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4HqbyR56Qqz1DJPH; Thu, 11 Nov 2021 17:29:19 +0800 (CST) Received: from dggpemm500023.china.huawei.com (7.185.36.83) by dggemv711-chm.china.huawei.com (10.1.198.66) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.15; Thu, 11 Nov 2021 17:31:34 +0800 Received: from [10.174.187.128] (10.174.187.128) by dggpemm500023.china.huawei.com (7.185.36.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2308.15; Thu, 11 Nov 2021 17:31:34 +0800 Subject: Re: [PATCH v4 2/2] tests/unit: Add an unit test for smp parsing To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , References: <20211028150913.1975305-1-philmd@redhat.com> <20211028150913.1975305-3-philmd@redhat.com> From: "wangyanan (Y)" Message-ID: <14250bbd-c3fb-9afc-f08d-587326f0382c@huawei.com> Date: Thu, 11 Nov 2021 17:31:33 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [10.174.187.128] X-ClientProxiedBy: dggeme704-chm.china.huawei.com (10.1.199.100) To dggpemm500023.china.huawei.com (7.185.36.83) X-CFilter-Loop: Reflected Received-SPF: pass client-ip=45.249.212.255; envelope-from=wangyanan55@huawei.com; helo=szxga08-in.huawei.com X-Spam_score_int: -81 X-Spam_score: -8.2 X-Spam_bar: -------- X-Spam_report: (-8.2 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-3.999, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Jones , Eduardo Habkost Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 2021/11/11 17:14, Philippe Mathieu-Daudé wrote: > On 10/28/21 17:09, Philippe Mathieu-Daudé wrote: >> From: Yanan Wang >> >> Now that we have a generic parser smp_parse(), let's add an unit >> test for the code. All possible valid/invalid SMP configurations >> that the user can specify are covered. >> >> Signed-off-by: Yanan Wang >> Reviewed-by: Andrew Jones >> Tested-by: Philippe Mathieu-Daudé >> Message-Id: <20211026034659.22040-3-wangyanan55@huawei.com> >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> tests/unit/test-smp-parse.c | 594 ++++++++++++++++++++++++++++++++++++ >> MAINTAINERS | 1 + >> tests/unit/meson.build | 1 + >> 3 files changed, 596 insertions(+) >> create mode 100644 tests/unit/test-smp-parse.c >> +static struct SMPTestData data_generic_valid[] = { >> + { >> + /* config: no configuration provided >> + * expect: cpus=1,sockets=1,cores=1,threads=1,maxcpus=1 */ > [1] > >> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, F, 0, F, 0), >> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1), >> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1), >> + }, { >> +static void test_generic(void) >> +{ >> + Object *obj = object_new(TYPE_MACHINE); >> + MachineState *ms = MACHINE(obj); >> + MachineClass *mc = MACHINE_GET_CLASS(obj); > Watch out, while you create a machine instance in each > test, there is only one machine class registered (see > type_register_static(&smp_machine_info) below in [2]), > ... Yes, I noticed this. So on the top of each sub-test function, the properties of the single machine class is re-initialized by smp_machine_class_init(mc). See [*] below. >> + SMPTestData *data = &(SMPTestData){0}; >> + int i; >> + >> + smp_machine_class_init(mc); [*] >> + >> + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) { >> + *data = data_generic_valid[i]; >> + unsupported_params_init(mc, data); >> + >> + smp_parse_test(ms, data, true); >> + >> + /* Unsupported parameters can be provided with their values as 1 */ >> + data->config.has_dies = true; >> + data->config.dies = 1; >> + smp_parse_test(ms, data, true); >> + } >> + >> + /* Reset the supported min CPUs and max CPUs */ >> + mc->min_cpus = 2; >> + mc->max_cpus = 511; > ... and here you are modifying the single machine class state, ... > >> + >> + for (i = 0; i < ARRAY_SIZE(data_generic_invalid); i++) { >> + *data = data_generic_invalid[i]; >> + unsupported_params_init(mc, data); >> + >> + smp_parse_test(ms, data, false); >> + } >> + >> + object_unref(obj); >> +} >> + >> +static void test_with_dies(void) >> +{ >> + Object *obj = object_new(TYPE_MACHINE); >> + MachineState *ms = MACHINE(obj); >> + MachineClass *mc = MACHINE_GET_CLASS(obj); > ... so here the machine class state is inconsistent, ... > >> + SMPTestData *data = &(SMPTestData){0}; >> + unsigned int num_dies = 2; >> + int i; >> + >> + smp_machine_class_init(mc); And here [*]. >> + mc->smp_props.dies_supported = true; >> + >> + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) { >> + *data = data_generic_valid[i]; >> + unsupported_params_init(mc, data); >> + >> + /* when dies parameter is omitted, it will be set as 1 */ >> + data->expect_prefer_sockets.dies = 1; >> + data->expect_prefer_cores.dies = 1; >> + >> + smp_parse_test(ms, data, true); > ... in particular the first test [1] is tested with mc->min_cpus = 2. > > I wonder why you are not getting: > > Output error report: Invalid SMP CPUs 1. The min CPUs supported by > machine '(null)' is 2 > > for [1]. So as I have explained above, we won't get an output error report like this here. :) Thanks, Yanan >> + >> + /* when dies parameter is specified */ >> + data->config.has_dies = true; >> + data->config.dies = num_dies; >> + if (data->config.has_cpus) { >> + data->config.cpus *= num_dies; >> + } >> + if (data->config.has_maxcpus) { >> + data->config.maxcpus *= num_dies; >> + } >> + >> + data->expect_prefer_sockets.dies = num_dies; >> + data->expect_prefer_sockets.cpus *= num_dies; >> + data->expect_prefer_sockets.max_cpus *= num_dies; >> + data->expect_prefer_cores.dies = num_dies; >> + data->expect_prefer_cores.cpus *= num_dies; >> + data->expect_prefer_cores.max_cpus *= num_dies; >> + >> + smp_parse_test(ms, data, true); >> + } >> + >> + for (i = 0; i < ARRAY_SIZE(data_with_dies_invalid); i++) { >> + *data = data_with_dies_invalid[i]; >> + unsupported_params_init(mc, data); >> + >> + smp_parse_test(ms, data, false); >> + } >> + >> + object_unref(obj); >> +} >> + >> +int main(int argc, char *argv[]) >> +{ >> + g_test_init(&argc, &argv, NULL); >> + >> + module_call_init(MODULE_INIT_QOM); >> + type_register_static(&smp_machine_info); > [2] > > .