Re: [PATCH] test_driver: implement virDomainMemoryPeek

[Ján Tomko <jtomko@xxxxxxxxxx>]

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@xxxxxxxxxx> 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@xxxxxxxxx>
> > >---
> > >
> > >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@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature

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