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