Re: [Qemu-devel] [PATCH v3 3/5] tests: use qtest_pc_boot()/qtest_shutdown() in virtio tests

[Greg Kurz <groug@kaod.org>]

On Fri, 7 Oct 2016 19:54:54 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 07/10/2016 18:38, Greg Kurz wrote:
> > On Sat,  1 Oct 2016 20:56:02 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> This patch replaces calls to qtest_start() and qtest_end() by
> >> calls to qtest_pc_boot() and qtest_shutdown().
> >>
> >> This allows to initialize memory allocator and PCI interface
> >> functions. This will ease to enable virtio tests on other
> >> architectures by only adding a specific qtest_XXX_boot() (like
> >> qtest_spapr_boot()).
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> Reviewed-by: Greg Kurz <groug@kaod.org>
> >> ---  
> > 
> > Oops I had missed it during review but I have a single remark for the 9p
> > test.
> > 
> > My R-b stands anyway.
> >   
> >>  tests/virtio-9p-test.c   |  51 +++++++---------
> >>  tests/virtio-blk-test.c  | 150 ++++++++++++++++++++---------------------------
> >>  tests/virtio-net-test.c  |  39 +++++-------
> >>  tests/virtio-scsi-test.c |  67 +++++++++------------
> >>  4 files changed, 129 insertions(+), 178 deletions(-)
> >>
> >> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> >> index e8b2196..7698014 100644
> >> --- a/tests/virtio-9p-test.c
> >> +++ b/tests/virtio-9p-test.c
> >> @@ -10,62 +10,56 @@
> >>  #include "qemu/osdep.h"
> >>  #include "libqtest.h"
> >>  #include "qemu-common.h"
> >> -#include "libqos/pci-pc.h"
> >> +#include "libqos/libqos-pc.h"
> >>  #include "libqos/virtio.h"
> >>  #include "libqos/virtio-pci.h"
> >> -#include "libqos/malloc.h"
> >> -#include "libqos/malloc-pc.h"
> >>  #include "standard-headers/linux/virtio_ids.h"
> >>  #include "standard-headers/linux/virtio_pci.h"
> >>  
> >>  static const char mount_tag[] = "qtest";
> >>  static char *test_share;
> >>  
> >> -static void qvirtio_9p_start(void)
> >> -{
> >> -    char *args;
> >>  
> >> +static QOSState *qvirtio_9p_start(void)
> >> +{
> >>      test_share = g_strdup("/tmp/qtest.XXXXXX");
> >>      g_assert_nonnull(mkdtemp(test_share));
> >> +    const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s "
> >> +                      "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s";
> >>    
> > 
> > Even if C99 supports declarations within the code, I prefer the old school way
> > of declaring variables followed by an empty line, at the top.  
> 
> Yes, you're right, I prefer too...
> 
> I will resend the series next week.
> 
> Thanks,
> Laurent
> 
> 

Hi Laurent,

Do you plan to repost soon ? I ask because I have some patches to add
functional testing to virtio-9p-test, that will likely conflict with
this series.

Cheers.

--
Greg