Re: [Qemu-devel] [PATCH v2] tests: numa-test: use -numa memdev option instead of legacy 'mem' option

[Igor Mammedov]

On Wed, 3 Jul 2019 16:37:02 -0300
Eduardo Habkost <address@hidden> wrote:

> On Wed, Jul 03, 2019 at 11:47:23AM -0400, Igor Mammedov wrote:
> > it drops testing of deprecated 'mem' option and will test preferred memdev
> > option instead.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> > v2:
> >   - make backend size depend on initial RAM size and a number of backends
> >     fixes test faulure with spapr machine (Eduardo Habkost <address@hidden>)
> > 
> >  tests/numa-test.c | 85 +++++++++++++++++++++++++++++------------------
> >  1 file changed, 52 insertions(+), 33 deletions(-)
> > 
> > diff --git a/tests/numa-test.c b/tests/numa-test.c
> > index 8de8581231..e102b3b70f 100644
> > --- a/tests/numa-test.c
> > +++ b/tests/numa-test.c
> > @@ -14,9 +14,28 @@
> >  #include "qapi/qmp/qdict.h"
> >  #include "qapi/qmp/qlist.h"
> >  
> > -static char *make_cli(const char *generic_cli, const char *test_cli)
> > +typedef struct {
> > +    const char *args;
> > +    int ram_size; /* size in MB */
> > +} TestData;
> > +
> > +static char *make_cli(const TestData *data, int memdev_count,
> > +                      const char *test_cli)
> >  {
> > -    return g_strdup_printf("%s %s", generic_cli ? generic_cli : "", 
> > test_cli);
> > +    int i;
> > +    char *tmp = NULL;
> > +    char *cli = g_strdup_printf("%s -m %d", data->args ? : "", 
> > data->ram_size);
> > +
> > +    for (i = 0; memdev_count && (i < memdev_count); i++) {
> > +        tmp = g_strdup_printf("%s -object 
> > memory-backend-ram,id=r%d,size=%dM",
> > +                              cli, i, data->ram_size / memdev_count);
> > +        g_free(cli);
> > +        cli = tmp;
> > +    }
> > +    tmp = g_strdup_printf("%s %s", cli, test_cli);
> > +    g_free(cli);
> > +    cli = tmp;
> > +    return cli;
> >  }  
> [...]
> > +    TestData data = { .ram_size = 128 };
> >  
> > -    if (strcmp(arch, "aarch64") == 0) {
> > -        args = "-machine virt";
> > +    if (!strcmp(arch, "aarch64")) {
> > +        data.args = "-machine virt";
> > +    } else if (!strcmp(arch, "ppc64")) {
> > +        data.ram_size = 512;  
> 
> Wouldn't it be simpler to always use "-m 512M" and the same node
> sizes?
it would, but it might make some CI instances start failing more
often where it used to work before. So I picked side of being
conservative and not to hog memory unless it's necessary.