test_driver: implement virDomainMemoryPeek

[libvirt] [PATCH] test_driver: implement virDomainMemoryPeek

Posted by Ilias Stamatis 4 years, 11 months ago

Begins by writing a @start byte in the first position of @buffer and
then for every next byte it stores the value of its previous one
incremented by one.

Behaves the same for both supported flags.

Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
---

Initially I thought about checking whether start+size exceeds the
available physical or virtual memory. However I noticed that even when
I pass -1 as @start the qemu driver doesn't complain and it fills the
buffer with data. So I left it as is, without performing any additional
checks.

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

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a4c17ef0df..04fdf7a4b8 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6000,6 +6000,42 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
     return 0;
 }

+static int
+testDomainMemoryPeek(virDomainPtr dom,
+                     unsigned long long start,
+                     size_t size,
+                     void *buffer,
+                     unsigned int flags)
+{
+    int ret = -1;
+    size_t i;
+    unsigned char b = start;
+    virDomainObjPtr vm = NULL;
+
+    virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1);
+
+    if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       "%s", _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL"));
+        goto cleanup;
+    }
+
+    if (!(vm = testDomObjFromDomain(dom)))
+        goto cleanup;
+
+    if (virDomainObjCheckActive(vm) < 0)
+        goto cleanup;
+
+    for (i = 0; i < size; i++)
+        ((unsigned char *) buffer)[i] = b++;
+
+    ret = 0;
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
+}
+

 /*
  * Snapshot APIs
@@ -6897,6 +6933,7 @@ static virHypervisorDriver testHypervisorDriver = {
     .domainManagedSave = testDomainManagedSave, /* 1.1.4 */
     .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.4 */
     .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.4 */
+    .domainMemoryPeek = testDomainMemoryPeek, /* 5.4.0 */

     .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
     .domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */
--
2.21.0

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

Re: [libvirt] [PATCH] test_driver: implement virDomainMemoryPeek

Posted by Ján Tomko 4 years, 11 months ago

On Thu, May 23, 2019 at 01:37:01PM +0200, Ilias Stamatis wrote:
>Begins by writing a @start byte in the first position of @buffer and
>then for every next byte it stores the value of its previous one
>incremented by one.
>
>Behaves the same for both supported flags.
>
>Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
>---
>
>Initially I thought about checking whether start+size exceeds the
>available physical or virtual memory. However I noticed that even when
>I pass -1 as @start the qemu driver doesn't complain and it fills the
>buffer with data. So I left it as is, without performing any additional
>checks.
>
> src/test/test_driver.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
>diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>index a4c17ef0df..04fdf7a4b8 100644
>--- a/src/test/test_driver.c
>+++ b/src/test/test_driver.c
>@@ -6000,6 +6000,42 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
>     return 0;
> }
>
>+static int
>+testDomainMemoryPeek(virDomainPtr dom,
>+                     unsigned long long start,
>+                     size_t size,
>+                     void *buffer,
>+                     unsigned int flags)
>+{
>+    int ret = -1;
>+    size_t i;
>+    unsigned char b = start;
>+    virDomainObjPtr vm = NULL;
>+
>+    virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1);
>+

>+    if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) {
>+        virReportError(VIR_ERR_INVALID_ARG,
>+                       "%s", _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL"));
>+        goto cleanup;
>+    }

You can use VIR_EXCLUSIVE_FLAGS_RET instead of open-coding it.

Jano

>+
>+    if (!(vm = testDomObjFromDomain(dom)))
>+        goto cleanup;
>+
>+    if (virDomainObjCheckActive(vm) < 0)
>+        goto cleanup;
>+
>+    for (i = 0; i < size; i++)
>+        ((unsigned char *) buffer)[i] = b++;
>+
>+    ret = 0;
>+
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] test_driver: implement virDomainMemoryPeek

Posted by Ilias Stamatis 4 years, 11 months ago

On Fri, May 24, 2019 at 2:39 PM Ján Tomko <jtomko@redhat.com> wrote:
>
> On Thu, May 23, 2019 at 01:37:01PM +0200, Ilias Stamatis wrote:
> >Begins by writing a @start byte in the first position of @buffer and
> >then for every next byte it stores the value of its previous one
> >incremented by one.
> >
> >Behaves the same for both supported flags.
> >
> >Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> >---
> >
> >Initially I thought about checking whether start+size exceeds the
> >available physical or virtual memory. However I noticed that even when
> >I pass -1 as @start the qemu driver doesn't complain and it fills the
> >buffer with data. So I left it as is, without performing any additional
> >checks.
> >
> > src/test/test_driver.c | 37 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> >
> >diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> >index a4c17ef0df..04fdf7a4b8 100644
> >--- a/src/test/test_driver.c
> >+++ b/src/test/test_driver.c
> >@@ -6000,6 +6000,42 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
> >     return 0;
> > }
> >
> >+static int
> >+testDomainMemoryPeek(virDomainPtr dom,
> >+                     unsigned long long start,
> >+                     size_t size,
> >+                     void *buffer,
> >+                     unsigned int flags)
> >+{
> >+    int ret = -1;
> >+    size_t i;
> >+    unsigned char b = start;
> >+    virDomainObjPtr vm = NULL;
> >+
> >+    virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1);
> >+
>
> >+    if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) {
> >+        virReportError(VIR_ERR_INVALID_ARG,
> >+                       "%s", _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL"));
> >+        goto cleanup;
> >+    }
>
> You can use VIR_EXCLUSIVE_FLAGS_RET instead of open-coding it.
>
> Jano

We're not checking mutual exclusivity here. We just want to make sure
that the flags passed matches one of these 2 values (and it is not
some other random value).

I was a bit hesitant about adding this, but this is exactly how it is
written in the qemu driver implementation of this function as well.

Is there some other macro for this case?

>
> >+
> >+    if (!(vm = testDomObjFromDomain(dom)))
> >+        goto cleanup;
> >+
> >+    if (virDomainObjCheckActive(vm) < 0)
> >+        goto cleanup;
> >+
> >+    for (i = 0; i < size; i++)
> >+        ((unsigned char *) buffer)[i] = b++;
> >+
> >+    ret = 0;
> >+

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

Re: [libvirt] [PATCH] test_driver: implement virDomainMemoryPeek

Posted by Ján Tomko 4 years, 11 months ago

On Fri, May 24, 2019 at 02:50:27PM +0200, Ilias Stamatis wrote:
>On Fri, May 24, 2019 at 2:39 PM Ján Tomko <jtomko@redhat.com> wrote:
>>
>> On Thu, May 23, 2019 at 01:37:01PM +0200, Ilias Stamatis wrote:
>> >Begins by writing a @start byte in the first position of @buffer and
>> >then for every next byte it stores the value of its previous one
>> >incremented by one.
>> >
>> >Behaves the same for both supported flags.
>> >
>> >Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
>> >---
>> >
>> >Initially I thought about checking whether start+size exceeds the
>> >available physical or virtual memory. However I noticed that even when
>> >I pass -1 as @start the qemu driver doesn't complain and it fills the
>> >buffer with data. So I left it as is, without performing any additional
>> >checks.
>> >
>> > src/test/test_driver.c | 37 +++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 37 insertions(+)
>> >
>> >diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>> >index a4c17ef0df..04fdf7a4b8 100644
>> >--- a/src/test/test_driver.c
>> >+++ b/src/test/test_driver.c
>> >@@ -6000,6 +6000,42 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
>> >     return 0;
>> > }
>> >
>> >+static int
>> >+testDomainMemoryPeek(virDomainPtr dom,
>> >+                     unsigned long long start,
>> >+                     size_t size,
>> >+                     void *buffer,
>> >+                     unsigned int flags)
>> >+{
>> >+    int ret = -1;
>> >+    size_t i;
>> >+    unsigned char b = start;
>> >+    virDomainObjPtr vm = NULL;
>> >+
>> >+    virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1);
>> >+
>>
>> >+    if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) {
>> >+        virReportError(VIR_ERR_INVALID_ARG,
>> >+                       "%s", _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL"));
>> >+        goto cleanup;
>> >+    }
>>
>> You can use VIR_EXCLUSIVE_FLAGS_RET instead of open-coding it.
>>
>> Jano
>
>We're not checking mutual exclusivity here. We just want to make sure
>that the flags passed matches one of these 2 values (and it is not
>some other random value).
>

Oh right, the macro would miss the case where none of the flags are
present.

>I was a bit hesitant about adding this, but this is exactly how it is
>written in the qemu driver implementation of this function as well.
>
>Is there some other macro for this case?
>

I don't think we need one.

Reviewed-by: Ján Tomko <jtomko@redhat.com>

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