Re: [PATCH] test_driver: Implement virDomainGetSecurityLabelList

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux