1. Patchew
  2. Libvirt
  3. [PATCH v2 0/3] Implement virDomainGetMessages for test driver
  4. View patch

[PATCH v2 1/3] test_driver: Implement virDomainGetMessages

Luke Yue posted 3 patches 4 years, 7 months ago
There is a newer version of this series
[PATCH v2 1/3] test_driver: Implement virDomainGetMessages
Posted by Luke Yue 4 years, 7 months ago 
Signed-off-by: Luke Yue <lukedyue@gmail.com>
---
 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;
+}
+
 /*
  * Test driver
  */
@@ -9489,6 +9541,7 @@ static virHypervisorDriver testHypervisorDriver = {
     .domainCheckpointLookupByName = testDomainCheckpointLookupByName, /* 5.6.0 */
     .domainCheckpointGetParent = testDomainCheckpointGetParent, /* 5.6.0 */
     .domainCheckpointDelete = testDomainCheckpointDelete, /* 5.6.0 */
+    .domainGetMessages = testDomainGetMessages, /* 7.5.0 */
 };
 
 static virNetworkDriver testNetworkDriver = {
-- 
2.32.0
Re: [PATCH v2 1/3] test_driver: Implement virDomainGetMessages
Posted by Martin Kletzander 4 years, 7 months ago 
On Thu, Jun 24, 2021 at 06:59:59PM +0800, Luke Yue wrote:
>Signed-off-by: Luke Yue <lukedyue@gmail.com>
>---
> 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.

> /*
>  * Test driver
>  */
>@@ -9489,6 +9541,7 @@ static virHypervisorDriver testHypervisorDriver = {
>     .domainCheckpointLookupByName = testDomainCheckpointLookupByName, /* 5.6.0 */
>     .domainCheckpointGetParent = testDomainCheckpointGetParent, /* 5.6.0 */
>     .domainCheckpointDelete = testDomainCheckpointDelete, /* 5.6.0 */
>+    .domainGetMessages = testDomainGetMessages, /* 7.5.0 */
> };
>
> static virNetworkDriver testNetworkDriver = {
>-- 
>2.32.0
>
Re: [PATCH v2 1/3] test_driver: Implement virDomainGetMessages
Posted by Luke Yue 4 years, 7 months ago 
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@gmail.com>
> > ---
> > 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.