[libvirt] [PATCH] test_driver: provide virDomainGetTime implementation
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
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
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
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
Στις Τρί, 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
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
participants (4)
-
Cole Robinson -
Ilias Stamatis -
Ján Tomko -
Pavel Hrdina