[PATCH] test_driver: Implement virDomainGetSecurityLabelList
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.
participants (2)
-
Luke Yue -
Martin Kletzander