[Qemu-devel] [PATCH v2 0/0] .travis and minor compile fixes

[Qemu-devel] [PATCH v2 0/0] .travis and minor compile fixes

[alex.bennee]

Hi,

I've updated the .travis.yml following feedback from Stefan to greatly
increase the number of compiles it does. In the process:

* Disabled ust backend (horribly broken)
* Found gov/gprof builds don't enforce --disable-pie
* Fixed a minor compiler warning when stderr/ftrace back-end enabled

I'm sending this before confirming a clean run (which takes around 2
hours on the Travis infrastructure). However you can watch it's
progress at:

https://travis-ci.org/stsquad/qemu/builds/11694820

Given I found a couple of issues while doing this I think this is
useful alongside the existing buildbot efforts. It allows developers
to run simple smoke-tests without access to the buildbot
infrastructure (but of course needing github/travis access, free for
FLOSS projects). I can see further development possibilities in the
future, one example would be a tree that auto-generates branches with
testing results for each patch series as they come into the mailing
list.

Alex Bennée (3):
      .travis.yml: basic compile and check recipes
      .travis.yml: greatly expand the coverage + more builds
      block/stream.c: ensure copy always set

 .travis.yml    | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block/stream.c |  2 ++
 2 files changed, 57 insertions(+)

[Qemu-devel] [PATCH 1/3] .travis.yml: basic compile and check recipes

[alex.bennee]

From: Alex Bennée <alex@bennee.com>

While QEMU already has various continuous integration set-ups in
buildbot and commercial Jenkins setups there is some value in supporting
travis-ci as well. It is well integrated into GitHub work flow and will
trigger on all branch pushes and pull requests. This makes it easier for
an individual to kick off smoke tests on a work-in-progress branch
before eventual submission of patches/pull requests upstream.

The build matrix is currently split by target architecture because a
full build of QEMU can take some time. This way you get quick feedback
for any obvious errors.
---
 .travis.yml | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index 0000000..69f60c1
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,19 @@
+language: c
+env:
+  - TARGETS=alpha-softmmu,alpha-linux-user
+  - TARGETS=arm-softmmu,arm-linux-user
+  - TARGETS=cris-softmmu
+  - TARGETS=i386-softmmu,x86_64-softmmu
+  - TARGETS=lm32-softmmu
+  - TARGETS=m68k-softmmu 
+  - TARGETS=microblaze-softmmu,microblazeel-softmmu
+  - TARGETS=mips-softmmu,mips64-softmmu,mips64el-softmmu,mipsel-softmmu
+  - TARGETS=moxie-softmmu
+  - TARGETS=or32-softmmu,
+  - TARGETS=ppc-softmmu,ppc64-softmmu,ppcemb-softmmu
+  - TARGETS=s390x-softmmu
+  - TARGETS=sh4-softmmu,sh4eb-softmmu
+  - TARGETS=sparc-softmmu,sparc64-softmmu
+  - TARGETS=unicore32-softmmu
+  - TARGETS=xtensa-softmmu,xtensaeb-softmmu
+script: "./configure --target-list=${TARGETS} && make && make check"
-- 
1.8.4

[Qemu-devel] [PATCH 2/3] .travis.yml: greatly expand the coverage + more builds

[alex.bennee]

From: Alex Bennée <alex@bennee.com>

I've made the definitions more modular to allow for greater build
control in each part of the matrix. I've confined the additional stuff
to x86/x86_64 for convenience.
---
 .travis.yml | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 69f60c1..f311f84 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,5 +1,14 @@
 language: c
 env:
+  global:
+    - TEST_CMD="make check"
+    - EXTRA_CONFIG=""
+    # Development packages, EXTRA_PKGS saved for additional builds
+    - CORE_PKGS="libusb-1.0-0-dev libiscsi-dev librados-dev libncurses5-dev"
+    - NET_PKGS="libseccomp-dev libgnutls-dev libssh2-1-dev  libspice-server-dev libspice-protocol-dev libnss3-dev"
+    - GUI_PKGS="libgtk-3-dev libvte-2.90-dev libsdl1.2-dev libpng12-dev libpixman-1-dev"
+    - EXTRA_PKGS=""
+  matrix:
   - TARGETS=alpha-softmmu,alpha-linux-user
   - TARGETS=arm-softmmu,arm-linux-user
   - TARGETS=cris-softmmu
@@ -16,4 +25,31 @@ env:
   - TARGETS=sparc-softmmu,sparc64-softmmu
   - TARGETS=unicore32-softmmu
   - TARGETS=xtensa-softmmu,xtensaeb-softmmu
-script: "./configure --target-list=${TARGETS} && make && make check"
+before_install:
+  - git submodule update --init --recursive
+  - sudo apt-get update -qq
+  - sudo apt-get install -qq ${CORE_PKGS} ${NET_PKGS} ${GUI_PKGS} ${EXTRA_PKGS}
+script: "./configure --target-list=${TARGETS} ${EXTRA_CONFIG} && make && ${TEST_CMD}"
+matrix:
+  # We manually include a number of additional build for non-standard bits
+  include:
+    # Debug related options
+    - env: TARGETS=i386-softmmu,x86_64-softmmu
+           EXTRA_CONFIG="--enable-debug--enable-debug-tcg --enable-tcg-interpreter"
+    # Currently configure doesn't force --disable-pie
+    - env: TARGETS=i386-softmmu,x86_64-softmmu
+           EXTRA_CONFIG="--enable-gprof --enable-gcov --disable-pie"
+    - env: TARGETS=i386-softmmu,x86_64-softmmu
+           EXTRA_PKGS="sparse"
+           EXTRA_CONFIG="--enable-sparse"
+    # All the trace backends (apart from dtrace)
+    - env: TARGETS=i386-softmmu,x86_64-softmmu
+           EXTRA_CONFIG="--enable-trace-backend=stderr"
+    - env: TARGETS=i386-softmmu,x86_64-softmmu
+           EXTRA_CONFIG="--enable-trace-backend=simple"
+    - env: TARGETS=i386-softmmu,x86_64-softmmu
+           EXTRA_CONFIG="--enable-trace-backend=ftrace"
+    # Currently broken on 12.04 due to mis-packaged liburcu and changed API, will be pulled.
+    #- env: TARGETS=i386-softmmu,x86_64-softmmu
+    #       EXTRA_PKGS="liblttng-ust-dev liburcu-dev"
+    #       EXTRA_CONFIG="--enable-trace-backend=ust"
-- 
1.8.4

[Qemu-devel] [PATCH 3/3] block/stream.c: ensure copy always set

[alex.bennee]

From: Alex Bennée <alex@bennee.com>

This only showed up when compiling with
--enable-trace-backend=stderr|ftrace at which point the compiler
complains with the following:

block/stream.c: In function ‘stream_run’:
block/stream.c:141:22: error: ‘copy’ may be used uninitialized in this function [-Werror=uninitialized]

Not sure exactly why it needs these options but it does seem clear the
negative return case should be handled.
---
 block/stream.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/stream.c b/block/stream.c
index 078ce4a..3b9c198 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -136,6 +136,8 @@ wait:
             }
 
             copy = (ret == 1);
+        } else {
+            copy = false;
         }
         trace_stream_one_iteration(s, sector_num, n, ret);
         if (ret >= 0 && copy) {
-- 
1.8.4

Re: [Qemu-devel] [PATCH v2 0/0] .travis and minor compile fixes

[Stefan Hajnoczi]

On Mon, Sep 23, 2013 at 05:07:27PM +0100, alex.bennee@linaro.org wrote:
> Given I found a couple of issues while doing this I think this is
> useful alongside the existing buildbot efforts. It allows developers
> to run simple smoke-tests without access to the buildbot
> infrastructure (but of course needing github/travis access, free for
> FLOSS projects). I can see further development possibilities in the
> future, one example would be a tree that auto-generates branches with
> testing results for each patch series as they come into the mailing
> list.

This is looking very interesting.  Buildbot is centralized and can be
hard to debug.  As a result not many people triage and fix configuration
issues.

Travis CI allows individual developers to modify the build
configuration.  This is much more friendly.

Can you explain the limitations of Travis CI?  For example, does it
start costing money at some point?

Stefan

Re: [Qemu-devel] [PATCH 3/3] block/stream.c: ensure copy always set

[Stefan Hajnoczi]

On Mon, Sep 23, 2013 at 05:07:30PM +0100, alex.bennee@linaro.org wrote:
> From: Alex Bennée <alex@bennee.com>
> 
> This only showed up when compiling with
> --enable-trace-backend=stderr|ftrace at which point the compiler
> complains with the following:
> 
> block/stream.c: In function ‘stream_run’:
> block/stream.c:141:22: error: ‘copy’ may be used uninitialized in this function [-Werror=uninitialized]
> 
> Not sure exactly why it needs these options but it does seem clear the
> negative return case should be handled.
> ---
>  block/stream.c | 2 ++
>  1 file changed, 2 insertions(+)

Stefan Weil already posted a similar patch:
http://patchwork.ozlabs.org/patch/276936/

Re: [Qemu-devel] [PATCH v2 0/0] .travis and minor compile fixes

[Stefan Hajnoczi]

On Mon, Sep 23, 2013 at 05:07:27PM +0100, alex.bennee@linaro.org wrote:
> I've updated the .travis.yml following feedback from Stefan to greatly
> increase the number of compiles it does. In the process:
> 
> * Disabled ust backend (horribly broken)
> * Found gov/gprof builds don't enforce --disable-pie
> * Fixed a minor compiler warning when stderr/ftrace back-end enabled
> 
> I'm sending this before confirming a clean run (which takes around 2
> hours on the Travis infrastructure). However you can watch it's
> progress at:
> 
> https://travis-ci.org/stsquad/qemu/builds/11694820
> 
> Given I found a couple of issues while doing this I think this is
> useful alongside the existing buildbot efforts. It allows developers
> to run simple smoke-tests without access to the buildbot
> infrastructure (but of course needing github/travis access, free for
> FLOSS projects). I can see further development possibilities in the
> future, one example would be a tree that auto-generates branches with
> testing results for each patch series as they come into the mailing
> list.
> 
> Alex Bennée (3):
>       .travis.yml: basic compile and check recipes
>       .travis.yml: greatly expand the coverage + more builds
>       block/stream.c: ensure copy always set
> 
>  .travis.yml    | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block/stream.c |  2 ++
>  2 files changed, 57 insertions(+)

The last patch can be dropped since Stefan Weil's fix has already been
reviewed on the list.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/0] .travis and minor compile fixes

[Alex Bennée]

stefanha@redhat.com writes:

> On Mon, Sep 23, 2013 at 05:07:27PM +0100, alex.bennee@linaro.org wrote:
>> Given I found a couple of issues while doing this I think this is
>> useful alongside the existing buildbot efforts. It allows developers
>> to run simple smoke-tests without access to the buildbot
>> infrastructure (but of course needing github/travis access, free for
>> FLOSS projects).
<snip>
> Travis CI allows individual developers to modify the build
> configuration.  This is much more friendly.
>
> Can you explain the limitations of Travis CI?  For example, does it
> start costing money at some point?

It is free to all public repositories. There is a "Pro" service which
allows you to set-up testing on private repos (much like GitHubs model).
The actual code for Travis is open source
(https://github.com/travis-ci/travis-ci) although I'm not aware of
anyone setting up another instance of it. I'm not aware of any plans to
charge open source projects for it's use.

The main limitation is the host build VM is Ubuntu 12.04 LTS. I'm not
sure how much of a limitation this is for the general developer -
Debian/RPM differences aside I would expect most compile failures to be
caught regardless of the host.

Also with a 38-way build matrix the current QEMU is a fairly heavy user
of the shared resource. I suspect running gcc and clang over all builds
is possibly a little over the top ;-)

I don't think this should attempt to replace other CI efforts. I know
that Linaro runs some tests through it's LAVA infrastructure and that's
an area I'm likely to look at expanding. The LAVA infrastructure is more
suited to testing on non-x86 hardware and running system image/kvm
integration tests. Having said that I think it can certainly act as a
good gatekeeper into "master" - if your not passing the Travis tests
then the branch should probably not be merged.

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH 3/3] block/stream.c: ensure copy always set

[Alex Bennée]

stefanha@redhat.com writes:

> On Mon, Sep 23, 2013 at 05:07:30PM +0100, alex.bennee@linaro.org wrote:
>> From: Alex Bennée <alex@bennee.com>
>> 
>> This only showed up when compiling with
>> --enable-trace-backend=stderr|ftrace at which point the compiler
>> complains with the following:
<nsip>
>
> Stefan Weil already posted a similar patch:
> http://patchwork.ozlabs.org/patch/276936/

OK I'll drop the patch in v3 which I guess is getting ready to be a
proper PULL request ;-)

What's the typical lag for trivial patches getting in? I see there are
periodic trivial pull requests. I assume there are maintainers that
collect these up into trees?

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH v2 0/0] .travis and minor compile fixes

[Stefan Hajnoczi]

On Tue, Sep 24, 2013 at 03:19:29PM +0100, Alex Bennée wrote:
> 
> stefanha@redhat.com writes:
> 
> > On Mon, Sep 23, 2013 at 05:07:27PM +0100, alex.bennee@linaro.org wrote:
> >> Given I found a couple of issues while doing this I think this is
> >> useful alongside the existing buildbot efforts. It allows developers
> >> to run simple smoke-tests without access to the buildbot
> >> infrastructure (but of course needing github/travis access, free for
> >> FLOSS projects).
> <snip>
> > Travis CI allows individual developers to modify the build
> > configuration.  This is much more friendly.
> >
> > Can you explain the limitations of Travis CI?  For example, does it
> > start costing money at some point?
> 
> It is free to all public repositories. There is a "Pro" service which
> allows you to set-up testing on private repos (much like GitHubs model).
> The actual code for Travis is open source
> (https://github.com/travis-ci/travis-ci) although I'm not aware of
> anyone setting up another instance of it. I'm not aware of any plans to
> charge open source projects for it's use.
> 
> The main limitation is the host build VM is Ubuntu 12.04 LTS. I'm not
> sure how much of a limitation this is for the general developer -
> Debian/RPM differences aside I would expect most compile failures to be
> caught regardless of the host.

Hmm...that misses extra bases we cover:

 * Solaris, FreeBSD, Mac OS, etc hosts
 * Stable distros with older library versions (e.g. Red Hat Enterprise
   Linux 5.x)
 * Different gcc versions report different warnings

> I don't think this should attempt to replace other CI efforts. I know
> that Linaro runs some tests through it's LAVA infrastructure and that's
> an area I'm likely to look at expanding. The LAVA infrastructure is more
> suited to testing on non-x86 hardware and running system image/kvm
> integration tests. Having said that I think it can certainly act as a
> good gatekeeper into "master" - if your not passing the Travis tests
> then the branch should probably not be merged.

I would like to replace buildbot because the utility vs maintenance
ratio is too low (but still better than no continuous integration at
all).

The ideal system allows untrusted users to build their own source trees
and make modifications to their build configuration.  That way everyone
is able to take advantage of continuous integration/build farm.

Stefan

Re: [Qemu-devel] [PATCH 3/3] block/stream.c: ensure copy always set

[Stefan Hajnoczi]

On Tue, Sep 24, 2013 at 03:27:49PM +0100, Alex Bennée wrote:
> 
> stefanha@redhat.com writes:
> 
> > On Mon, Sep 23, 2013 at 05:07:30PM +0100, alex.bennee@linaro.org wrote:
> >> From: Alex Bennée <alex@bennee.com>
> >> 
> >> This only showed up when compiling with
> >> --enable-trace-backend=stderr|ftrace at which point the compiler
> >> complains with the following:
> <nsip>
> >
> > Stefan Weil already posted a similar patch:
> > http://patchwork.ozlabs.org/patch/276936/
> 
> OK I'll drop the patch in v3 which I guess is getting ready to be a
> proper PULL request ;-)

You don't need to send a pull request yourself.  Once your patch is
reviewed it should be merged by a maintainer.

> What's the typical lag for trivial patches getting in? I see there are
> periodic trivial pull requests. I assume there are maintainers that
> collect these up into trees?

Contributors send patches to qemu-devel.  After the patches are reviewed
they are merged either directly into qemu.git or into a subsystem tree
by a maintainer.

Subsystem maintainers send pull requests to flush their patch queue when
they feel it is appropriate.  For example, Kevin and I send a weekly
block layer pull request that usually contains 5-15 patches.

Expect 1.5 weeks to get the average patch reviewed and merged into
qemu.git.  If you're lucky they can be merged in a day.  If you're
unlucky you may have to ping the list to move the process along.

Stefan