Signed-off-by: Luke Yue <lukedyue@gmail.com>
---
 src/test/test_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 611ec6d7ec..ae1b8ebc23 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -136,6 +136,7 @@ VIR_ONCE_GLOBAL_INIT(testDriver);
 
 #define TEST_MODEL "i686"
 #define TEST_EMULATOR "/usr/bin/test-hv"
+#define TEST_SECURITY_LABEL_LIST_LENGTH 2
 
 static const virNodeInfo defaultNodeInfo = {
     TEST_MODEL,
@@ -5037,6 +5038,45 @@ testDomainGetSecurityLabel(virDomainPtr dom,
     return ret;
 }
 
+static int
+testDomainGetSecurityLabelList(virDomainPtr dom,
+                               virSecurityLabelPtr* seclabels)
+{
+    virDomainObj *vm;
+    size_t i;
+    int ret = -1;
+
+    if (!(vm = testDomObjFromDomain(dom)))
+        return -1;
+
+    if (!virDomainObjIsActive(vm)) {
+        /* No seclabels */
+        *seclabels = NULL;
+        ret = 0;
+    } else {
+        int len = TEST_SECURITY_LABEL_LIST_LENGTH;
+
+        (*seclabels) = g_new0(virSecurityLabel, len);
+        memset(*seclabels, 0, sizeof(**seclabels) * len);
+
+        /* Fill the array */
+        for (i = 0; i < len; i++) {
+            if (virStrcpyStatic(seclabels[i]->label, "libvirt-test") < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                            _("security label exceeds maximum: %zu"),
+                            sizeof(seclabels[i]->label) - 1);
+                VIR_FREE(*seclabels);
+                goto cleanup;
+            }
+        }
+        ret = len;
+    }
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
+}
+
 static int
 testNodeGetSecurityModel(virConnectPtr conn,
                          virSecurityModelPtr secmodel)
@@ -9357,6 +9397,7 @@ static virHypervisorDriver testHypervisorDriver = {
     .domainGetVcpuPinInfo = testDomainGetVcpuPinInfo, /* 1.2.18 */
     .domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
     .domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
+    .domainGetSecurityLabelList = testDomainGetSecurityLabelList, /* 7.5.0 */
     .nodeGetSecurityModel = testNodeGetSecurityModel, /* 7.5.0 */
     .domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
     .domainSetMemoryParameters = testDomainSetMemoryParameters, /* 5.6.0 */
-- 
2.32.0

On Mon, Jun 14, 2021 at 09:12:57PM +0800, Luke Yue wrote:
>Signed-off-by: Luke Yue <lukedyue@gmail.com>
>---
> src/test/test_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>

This patch looks fine, but it would be good to have tests for it also.
The good thing is that thanks to the fact that this is a test driver API
the check can be done using just virsh, no need to write tests and mock
syscalls.  The previous patches added at least some checks, because it
was already in some other test codepath, but this (and later ones as
well) are going to need to have some new ones added.

Thanks,
Martin

On Tue, 2021-06-15 at 10:08 +0200, Martin Kletzander wrote:
> On Mon, Jun 14, 2021 at 09:12:57PM +0800, Luke Yue wrote:
> > Signed-off-by: Luke Yue <lukedyue@gmail.com>
> > ---
> > src/test/test_driver.c | 41
> > +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> > 
> 
> This patch looks fine, but it would be good to have tests for it
> also.
> The good thing is that thanks to the fact that this is a test driver
> API
> the check can be done using just virsh, no need to write tests and
> mock
> syscalls.  The previous patches added at least some checks, because
> it
> was already in some other test codepath, but this (and later ones as
> well) are going to need to have some new ones added.
> 
Thanks for the review!

It seems that there is no command in virsh uses
virDomainGetSecurityLabelList, should we add one? Or is there any other
testing methods?

Thanks,
Luke

[Just found out I got couple of mails lost, so resending even though it was sent
  a week ago]

On Wed, Jun 16, 2021 at 05:21:17PM +0800, Luke Yue wrote:
>On Tue, 2021-06-15 at 10:08 +0200, Martin Kletzander wrote:
>> On Mon, Jun 14, 2021 at 09:12:57PM +0800, Luke Yue wrote:
>> > Signed-off-by: Luke Yue <lukedyue@gmail.com>
>> > ---
>> > src/test/test_driver.c | 41
>> > +++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 41 insertions(+)
>> >
>>
>> This patch looks fine, but it would be good to have tests for it
>> also.
>> The good thing is that thanks to the fact that this is a test driver
>> API
>> the check can be done using just virsh, no need to write tests and
>> mock
>> syscalls.  The previous patches added at least some checks, because
>> it
>> was already in some other test codepath, but this (and later ones as
>> well) are going to need to have some new ones added.
>>
>Thanks for the review!
>
>It seems that there is no command in virsh uses
>virDomainGetSecurityLabelList, should we add one? Or is there any other
>testing methods?
>

You can add a command, or you can just write a small program that checks it.
The former approach would require a round of design so that it is sensible for
virsh to have such command, however the latter approach would add a whole extra
binary to the build just to call one API.  LLet's see what others think.  I
think we should definitely test it, especially when it can share most of its
codepath with qemu and others.

>Thanks,
>Luke
>
>

On Wed, 2021-06-23 at 00:07 +0200, Martin Kletzander wrote:
> [Just found out I got couple of mails lost, so resending even though
> it was sent
>   a week ago]
> 
> On Wed, Jun 16, 2021 at 05:21:17PM +0800, Luke Yue wrote:
> > On Tue, 2021-06-15 at 10:08 +0200, Martin Kletzander wrote:
> > > On Mon, Jun 14, 2021 at 09:12:57PM +0800, Luke Yue wrote:
> > > > Signed-off-by: Luke Yue <lukedyue@gmail.com>
> > > > ---
> > > > src/test/test_driver.c | 41
> > > > +++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 41 insertions(+)
> > > > 
> > > 
> > > This patch looks fine, but it would be good to have tests for it
> > > also.
> > > The good thing is that thanks to the fact that this is a test
> > > driver
> > > API
> > > the check can be done using just virsh, no need to write tests
> > > and
> > > mock
> > > syscalls.  The previous patches added at least some checks,
> > > because
> > > it
> > > was already in some other test codepath, but this (and later ones
> > > as
> > > well) are going to need to have some new ones added.
> > > 
> > Thanks for the review!
> > 
> > It seems that there is no command in virsh uses
> > virDomainGetSecurityLabelList, should we add one? Or is there any
> > other
> > testing methods?
> > 
> 
> You can add a command, or you can just write a small program that
> checks it.
> The former approach would require a round of design so that it is
> sensible for
> virsh to have such command, however the latter approach would add a
> whole extra
> binary to the build just to call one API.  LLet's see what others
> think.  I
> think we should definitely test it, especially when it can share most
> of its
> codepath with qemu and others.
> > 
I think adding a command may be a better choice? Cause the command can
not only be used for testing purpose but also be used by normal users.

I will learn and try to add the command if we finally choose this way.
>