Re: [PATCH RFC] test_driver: check that the domain is running in testDomainGetTime

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/20/19 1:41 PM, Ilias Stamatis wrote:
Signed-off-by: Ilias Stamatis <stamatis.iliass@xxxxxxxxx>
---

Currently in the test driver in APIs that would normally require guest
agents and similar we are just checking if the domain is active (using
virDomainObjCheckActive).

But a domain will be active even if stopped, so I would say that most of
the time this is not sufficient since we really want to now that the
domain is actually running, not that it's just active.

So something like what I include in this patch is needed instead.

I wonder if it would make sense to add a new function such as
testDomainAgentAvailable inspired by the QEMU driver.

So the check whether the domain is actually running or not could be done
there.

Also, in that case I'm not sure if calling both virDomainObjCheckActive
and testDomainAgentAvailable is needed. I feel like calling the second
one only is sufficient. However, in the QEMU driver always both are
called.

Should I go ahead with implementing the testDomainAgentAvailable
function and search for all the places in the test driver that it should
be used instead of virDomainObjCheckActive?

  src/test/test_driver.c | 20 ++++++++++++++++++--
  1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2a0ffbc6c5..8e5bd4e670 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1984,17 +1984,33 @@ testDomainGetState(virDomainPtr domain,
  }

  static int
-testDomainGetTime(virDomainPtr dom ATTRIBUTE_UNUSED,
+testDomainGetTime(virDomainPtr dom,
                    long long *seconds,
                    unsigned int *nseconds,
                    unsigned int flags)
  {
+    int ret = -1;
+    virDomainObjPtr vm = NULL;

Personal preference, I like @ret to be the last variable.

+
      virCheckFlags(0, -1);

+    if (!(vm = testDomObjFromDomain(dom)))
+        goto cleanup;

Again, nit pick s/goto cleanup/return -1/ ;-)

+
+    if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) {
+        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                       _("domain is not running"));
+        goto cleanup;
+    }
+
      *seconds = 627319920;
      *nseconds = 0;

-    return 0;
+    ret = 0;
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
  }


ACKed and pushed.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux