Re: [PATCH v2 1/3] test_driver: Implement virDomainGetMessages

On Thu, 2021-06-24 at 17:19 +0200, Martin Kletzander wrote: > On Thu, Jun 24, 2021 at 06:59:59PM +0800, Luke Yue wrote: > > Signed-off-by: Luke Yue <lukedyue(a)gmail.com&gt; > > --- > > src/test/test_driver.c | 53 > > ++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 53 insertions(+) > > > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c > > index 65710b78ef..dff96bceb6 100644 > > --- a/src/test/test_driver.c > > +++ b/src/test/test_driver.c > > @@ -9331,6 +9331,58 @@ > > testDomainCheckpointDelete(virDomainCheckpointPtr checkpoint, > >     return ret; > > } > > > > +static int > > +testDomainGetMessages(virDomainPtr dom, > > +                      char ***msgs, > > +                      unsigned int flags) > > +{ > > +    virDomainObj *vm = NULL; > > +    int rv = -1; > > +    size_t i, n; > > +    int nmsgs; > > + > > +    virCheckFlags(VIR_DOMAIN_MESSAGE_DEPRECATION | > > +                  VIR_DOMAIN_MESSAGE_TAINTING, -1); > > + > > +    if (!(vm = testDomObjFromDomain(dom))) > > +        return -1; > > + > > +    *msgs = NULL; > > +    nmsgs = 0; > > +    n = 0; > > + > > +    if (!flags || (flags & VIR_DOMAIN_MESSAGE_TAINTING)) { > > +        nmsgs += __builtin_popcount(vm->taint); > > +        *msgs = g_renew(char *, *msgs, nmsgs+1); > > + > > +        for (i = 0; i < VIR_DOMAIN_TAINT_LAST; i++) { > > +            if (vm->taint & (1 << i)) { > > +                (*msgs)[n++] = g_strdup_printf( > > +                    _("tainted: %s"), > > +                    _(virDomainTaintMessageTypeToString(i))); > > +            } > > +        } > > +    } > > + > > +    if (!flags || (flags & VIR_DOMAIN_MESSAGE_DEPRECATION)) { > > +        nmsgs += vm->ndeprecations; > > +        *msgs = g_renew(char *, *msgs, nmsgs+1); > > + > > +        for (i = 0; i < vm->ndeprecations; i++) { > > +            (*msgs)[n++] = g_strdup_printf( > > +                _("deprecated configuration: %s"), > > +                vm->deprecations[i]); > > +        } > > +    } > > + > > +    (*msgs)[nmsgs] = NULL; > > + > > +    rv = nmsgs; > > + > > +    virDomainObjEndAPI(&vm); > > +    return rv; > > +} > > + > > As I mentioned in the review for v1, it is nice that someone can > check how this > API behaves without spinning up a VM, just using the test driver.  > However most > of this code is copy-paste from the qemu driver and can hence be > extracted to a > separate function which would be called from both drivers (and maybe > more than > the ones mentioned here are using the same logic).  That would make > the codebase > smaller, the library smaller, it would keep the test driver updated > and it would > actually test the codepath used in the qemu driver. > > I'm sorry, I forgot that. So if I want to extract a function like below ``` int virDomainGetMessagesFromVM(virDomainObj *vm, char **msgs, unsigned int flags) { size_t i, n; int nmsgs; int rv = -1; *msgs = NULL; nmsgs = 0; n = 0; ... rv = nmsgs; return rv; } ``` Where should I place this function? In `domain_conf.c` or somewhere else?

Thanks, Luke