[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 83/86] tests:numa-test: make top level args dynamic and g_
From: |
Thomas Huth |
Subject: |
Re: [PATCH v2 83/86] tests:numa-test: make top level args dynamic and g_autofree(cli) cleanups |
Date: |
Fri, 17 Jan 2020 12:14:11 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 |
On 16/01/2020 18.06, Igor Mammedov wrote:
> On Thu, 16 Jan 2020 17:35:32 +0100
> Thomas Huth <address@hidden> wrote:
>
>> On 15/01/2020 16.07, Igor Mammedov wrote:
>>> Use GString to pass argument to make_cli() so that it would be easy
>>> to dynamically change test case arguments from main(). The follow up
>>> patch will use it to change RAM size options depending on target.
>>>
>>> While at it cleanup 'cli' freeing, using g_autofree annotation.
>>
>> Hmm, I'd use g_autofree for new code or do it in a separate cleanup
>> patch, but doing this here distracts quite a bit from the real changes
>> that you are doing...
> I'll split it into separate patch
>
>>
>>> Signed-off-by: Igor Mammedov <address@hidden>
>>> ---
>>> PS:
>>> made as a separate patch so it won't clutter followup testcase changes.
>>>
>>> CC: address@hidden
>>> CC: address@hidden
>>> ---
>>> tests/qtest/numa-test.c | 38 ++++++++++++++------------------------
>>> 1 file changed, 14 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
>>> index 17dd807..a696dfd 100644
>>> --- a/tests/qtest/numa-test.c
>>> +++ b/tests/qtest/numa-test.c
>>> @@ -14,16 +14,16 @@
>>> #include "qapi/qmp/qdict.h"
>>> #include "qapi/qmp/qlist.h"
>>>
>>> -static char *make_cli(const char *generic_cli, const char *test_cli)
>>> +static char *make_cli(const GString *generic_cli, const char *test_cli)
>>> {
>>> - return g_strdup_printf("%s %s", generic_cli ? generic_cli : "",
>>> test_cli);
>>> + return g_strdup_printf("%s %s", generic_cli->str, test_cli);
>>> }
>> [...]
>>> @@ -539,11 +529,11 @@ static void pc_hmat_erange_cfg(const void *data)
>>>
>>> int main(int argc, char **argv)
>>> {
>>> - const char *args = NULL;
>>> + g_autoptr(GString) args = g_string_new("");
>>
>> I think g_string_new(NULL) would be better?
>>
>>> const char *arch = qtest_get_arch();
>>>
>>> if (strcmp(arch, "aarch64") == 0) {
>>> - args = "-machine virt";
>>> + g_string_append(args, " -machine virt")> }
>>
>> Is this really required? Looking at your next patch, you could also
>> simply do
>>
>> args = " -object memory-backend-ram,id=ram,size=xxxM"
> xxx is variable so options are
> 1 build this part of CLI dynamically
> 2 mostly duplicate testcase function and include per target size there
> 3 make up a test data structure and pass that to test cases
>
> Given simplicity of current testcases, I'd prefer continue with
> passing CLI as testcase data (option #1).
Sorry, I think I missed something here... currently I see in the next patch:
+ if (!strcmp(arch, "ppc64")) {
+ g_string_append(args, " -object
memory-backend-ram,id=ram,size=512M");
+ } else {
+ g_string_append(args, " -object
memory-backend-ram,id=ram,size=128M");
+ }
... so these are static strings which could also be handled fine without
GString? Or do you plan to update this in later patches?
Thomas
- Re: [PATCH v2 85/86] numa: make exit() usage consistent, (continued)
[PATCH v2 86/86] numa: remove deprecated implicit RAM distribution between nodes, Igor Mammedov, 2020/01/15
[PATCH v2 83/86] tests:numa-test: make top level args dynamic and g_autofree(cli) cleanups, Igor Mammedov, 2020/01/15
Re: [PATCH v2 00/86] refactor main RAM allocation to use hostmem backend, no-reply, 2020/01/15
Re: [PATCH v2 00/86] refactor main RAM allocation to use hostmem backend, no-reply, 2020/01/15
Re: [PATCH v2 00/86] refactor main RAM allocation to use hostmem backend, no-reply, 2020/01/15
Re: [PATCH v2 00/86] refactor main RAM allocation to use hostmem backend, no-reply, 2020/01/15