[libvirt] [PATCH] test_driver: provide virDomainGetTime implementation

Ilias Stamatis posted 1 patch 5 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190407234317.28458-1-stamatis.iliass@gmail.com
There is a newer version of this series
src/test/test_driver.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
[libvirt] [PATCH] test_driver: provide virDomainGetTime implementation
Posted by Ilias Stamatis 5 years ago
Implement testDomainGetTime by returning the current time.

Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
---
 I initially implemented this using clock_gettime, but Pavel suggested
 that this might not be a good idea since it isn't a cross-platform
 function. So I used virTimeMillisNow instead and set the nanoseconds
 part to 0 which can be ok for the test driver.

 src/test/test_driver.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d5eecf4b7f..b6f12e784f 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -64,6 +64,7 @@
 #include "virinterfaceobj.h"
 #include "virhostcpu.h"
 #include "virdomainsnapshotobjlist.h"
+#include "virtime.h"

 #define VIR_FROM_THIS VIR_FROM_TEST

@@ -1943,6 +1944,23 @@ testDomainGetState(virDomainPtr domain,
     return 0;
 }

+static int
+testDomainGetTime(virDomainPtr dom ATTRIBUTE_UNUSED,
+                  long long *seconds,
+                  unsigned int *nseconds,
+                  unsigned int flags ATTRIBUTE_UNUSED)
+{
+    unsigned long long now;
+
+    if (virTimeMillisNow(&now) < 0)
+        return -1;
+
+    *seconds = now / 1000;
+    *nseconds = 0;
+
+    return 0;
+}
+
 #define TEST_SAVE_MAGIC "TestGuestMagic"

 static int
@@ -6786,6 +6804,7 @@ static virHypervisorDriver testHypervisorDriver = {
     .domainSetMemory = testDomainSetMemory, /* 0.1.4 */
     .domainGetInfo = testDomainGetInfo, /* 0.1.1 */
     .domainGetState = testDomainGetState, /* 0.9.2 */
+    .domainGetTime = testDomainGetTime, /* 5.3.0 */
     .domainSave = testDomainSave, /* 0.3.2 */
     .domainSaveFlags = testDomainSaveFlags, /* 0.9.4 */
     .domainRestore = testDomainRestore, /* 0.3.2 */
--
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: provide virDomainGetTime implementation
Posted by Ján Tomko 5 years ago
On Mon, Apr 08, 2019 at 01:43:17AM +0200, Ilias Stamatis wrote:
>Implement testDomainGetTime by returning the current time.
>
>Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
>---
> I initially implemented this using clock_gettime, but Pavel suggested
> that this might not be a good idea since it isn't a cross-platform
> function. So I used virTimeMillisNow instead and set the nanoseconds
> part to 0 which can be ok for the test driver.
>

Do you have a consumer for this?

IIUC these APIs are used for testing by higher layers like virt-manager
or libvirt-dbus and having it return a different value every time does
not seem that useful. For example for nodeCPUstats we return hardcoded
values.

> src/test/test_driver.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>

Code-wise the patch looks good.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: provide virDomainGetTime implementation
Posted by Cole Robinson 5 years ago
On 4/9/19 6:16 AM, Ján Tomko wrote:
> On Mon, Apr 08, 2019 at 01:43:17AM +0200, Ilias Stamatis wrote:
>> Implement testDomainGetTime by returning the current time.
>>
>> Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
>> ---
>> I initially implemented this using clock_gettime, but Pavel suggested
>> that this might not be a good idea since it isn't a cross-platform
>> function. So I used virTimeMillisNow instead and set the nanoseconds
>> part to 0 which can be ok for the test driver.
>>
> 
> Do you have a consumer for this?
> 
> IIUC these APIs are used for testing by higher layers like virt-manager
> or libvirt-dbus and having it return a different value every time does
> not seem that useful. For example for nodeCPUstats we return hardcoded
> values.

I agree, hardcoded is slightly preferred. Consider if we wanted to have
virsh unit tests or python binding unit tests, a constant value by
default would help ensure some intermediate piece isn't screwing up the
value.

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: provide virDomainGetTime implementation
Posted by Ilias Stamatis 5 years ago
Στις Τρί, 16 Απρ 2019 στις 2:00 π.μ., ο/η Cole Robinson
<crobinso@redhat.com> έγραψε:
>
> On 4/9/19 6:16 AM, Ján Tomko wrote:
> > On Mon, Apr 08, 2019 at 01:43:17AM +0200, Ilias Stamatis wrote:
> >> Implement testDomainGetTime by returning the current time.
> >>
> >> Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> >> ---
> >> I initially implemented this using clock_gettime, but Pavel suggested
> >> that this might not be a good idea since it isn't a cross-platform
> >> function. So I used virTimeMillisNow instead and set the nanoseconds
> >> part to 0 which can be ok for the test driver.
> >>
> >
> > Do you have a consumer for this?
> >
> > IIUC these APIs are used for testing by higher layers like virt-manager
> > or libvirt-dbus and having it return a different value every time does
> > not seem that useful. For example for nodeCPUstats we return hardcoded
> > values.
>
> I agree, hardcoded is slightly preferred. Consider if we wanted to have
> virsh unit tests or python binding unit tests, a constant value by
> default would help ensure some intermediate piece isn't screwing up the
> value.
>
> - Cole

So I can change this to a fixed value. Is there any specific value
that you would prefer or should I use my current time?

Thanks,
Ilias

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: provide virDomainGetTime implementation
Posted by Ján Tomko 5 years ago
On Tue, Apr 16, 2019 at 11:27:44AM +0200, Ilias Stamatis wrote:
>Στις Τρί, 16 Απρ 2019 στις 2:00 π.μ., ο/η Cole Robinson
><crobinso@redhat.com> έγραψε:
>>
>> On 4/9/19 6:16 AM, Ján Tomko wrote:
>> > On Mon, Apr 08, 2019 at 01:43:17AM +0200, Ilias Stamatis wrote:
>> >> Implement testDomainGetTime by returning the current time.
>> >>
>> >> Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
>> >> ---
>> >> I initially implemented this using clock_gettime, but Pavel suggested
>> >> that this might not be a good idea since it isn't a cross-platform
>> >> function. So I used virTimeMillisNow instead and set the nanoseconds
>> >> part to 0 which can be ok for the test driver.
>> >>
>> >
>> > Do you have a consumer for this?
>> >
>> > IIUC these APIs are used for testing by higher layers like virt-manager
>> > or libvirt-dbus and having it return a different value every time does
>> > not seem that useful. For example for nodeCPUstats we return hardcoded
>> > values.
>>
>> I agree, hardcoded is slightly preferred. Consider if we wanted to have
>> virsh unit tests or python binding unit tests, a constant value by
>> default would help ensure some intermediate piece isn't screwing up the
>> value.
>>
>> - Cole
>
>So I can change this to a fixed value. Is there any specific value
>that you would prefer or should I use my current time?
>

Personally, I'd prefer 627319920

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: provide virDomainGetTime implementation
Posted by Pavel Hrdina 5 years ago
On Tue, Apr 09, 2019 at 12:16:22PM +0200, Ján Tomko wrote:
> On Mon, Apr 08, 2019 at 01:43:17AM +0200, Ilias Stamatis wrote:
> > Implement testDomainGetTime by returning the current time.
> > 
> > Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> > ---
> > I initially implemented this using clock_gettime, but Pavel suggested
> > that this might not be a good idea since it isn't a cross-platform
> > function. So I used virTimeMillisNow instead and set the nanoseconds
> > part to 0 which can be ok for the test driver.
> > 
> 
> Do you have a consumer for this?

There is probably no consumer for now.  However the ultimate goal is to
implement all APIs in the test driver in order to make the test driver
implementation mandatory for every new API.

> IIUC these APIs are used for testing by higher layers like virt-manager
> or libvirt-dbus and having it return a different value every time does
> not seem that useful. For example for nodeCPUstats we return hardcoded
> values.

I'm OK with hard-coding some specific value, my guess is that no
management application will check the specific value but we can
give them that possibility.

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