Re: [PULL v2 00/27] Block patches

2021-02-05 Thread Daniel P . Berrangé
On Fri, Feb 05, 2021 at 05:52:59PM +0100, Thomas Huth wrote:
> On 05/02/2021 17.23, Peter Maydell wrote:
> > On Fri, 5 Feb 2021 at 16:21, Stefan Hajnoczi  wrote:
> > > Thanks, I update the patch in question.
> > > 
> > > It looks like the GitLab CI doesn't include a clang version that
> > > produces this error because the pipeline passed for me:
> > > https://gitlab.com/stefanha/qemu/-/pipelines/251524779
> > > 
> > > Is there something clang-specific you want to check in the CI? Maybe
> > > clang 3.4, the oldest version supported according to ./configure?
> > 
> > Would probably be nice I guess. My ad-hoc builds use clang 6,
> > which is what tripped up here.
> 
> We should maybe discuss first whether we can bump the minimum version of
> Clang that we would like to support. I once picked Clang 3.4 since that was
> available in EPEL for RHEL7, but I think there were newer versions of Clang
> available in RHEL7 via other repos later, so 3.4 is likely really just way
> too old now...
> 
> According to https://developers.redhat.com/HW/ClangLLVM-RHEL-7 there was at
> least Clang 7.0 available on RHEL7. Debian stable seems to have at least
> 7.0, too, according to repology.org. Ubuntu 18.04 seems to have version 6,
> but later ones are available via updates? Anyway, I think we could at least
> bump the minimum version to 6.0 nowadays...

Per our support matrix, this is the last dev cycle where we need to
care about RHEL-7, as RHEL-7 will be past the 2 year cutoff in
the QEMU 6.1 cycle.

Furthermore given that CLang was only ever an EPEL package, not a
core part of the distro, I think we are justified in just ignoring
RHEL-7 already for purpose of choosing CLang min version.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PULL v2 00/27] Block patches

2021-02-05 Thread Thomas Huth

On 05/02/2021 17.23, Peter Maydell wrote:

On Fri, 5 Feb 2021 at 16:21, Stefan Hajnoczi  wrote:

Thanks, I update the patch in question.

It looks like the GitLab CI doesn't include a clang version that
produces this error because the pipeline passed for me:
https://gitlab.com/stefanha/qemu/-/pipelines/251524779

Is there something clang-specific you want to check in the CI? Maybe
clang 3.4, the oldest version supported according to ./configure?


Would probably be nice I guess. My ad-hoc builds use clang 6,
which is what tripped up here.


We should maybe discuss first whether we can bump the minimum version of 
Clang that we would like to support. I once picked Clang 3.4 since that was 
available in EPEL for RHEL7, but I think there were newer versions of Clang 
available in RHEL7 via other repos later, so 3.4 is likely really just way 
too old now...


According to https://developers.redhat.com/HW/ClangLLVM-RHEL-7 there was at 
least Clang 7.0 available on RHEL7. Debian stable seems to have at least 
7.0, too, according to repology.org. Ubuntu 18.04 seems to have version 6, 
but later ones are available via updates? Anyway, I think we could at least 
bump the minimum version to 6.0 nowadays...


 Thomas




Re: [PULL v2 00/27] Block patches

2021-02-05 Thread Peter Maydell
On Fri, 5 Feb 2021 at 16:21, Stefan Hajnoczi  wrote:
> Thanks, I update the patch in question.
>
> It looks like the GitLab CI doesn't include a clang version that
> produces this error because the pipeline passed for me:
> https://gitlab.com/stefanha/qemu/-/pipelines/251524779
>
> Is there something clang-specific you want to check in the CI? Maybe
> clang 3.4, the oldest version supported according to ./configure?

Would probably be nice I guess. My ad-hoc builds use clang 6,
which is what tripped up here.

thanks
-- PMM



Re: [PULL v2 00/27] Block patches

2021-02-05 Thread Stefan Hajnoczi
On Thu, Feb 04, 2021 at 05:35:31PM +, Peter Maydell wrote:
> On Thu, 4 Feb 2021 at 15:43, Stefan Hajnoczi  wrote:
> >
> > The following changes since commit db754f8ccaf2f073c9aed46a4389e9c0c2080399:
> >
> >   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210202' 
> > into staging (2021-02-03 19:35:57 +)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> >
> > for you to fetch changes up to abe42229db7b87caa11b3c02835ebf9d384e0bd4:
> >
> >   docs: fix Parallels Image "dirty bitmap" section (2021-02-04 15:17:10 
> > +)
> >
> > 
> > Pull request
> >
> > v2:
> >  * Rebase to resolve memory_region_init_ram_from_file() conflict due to the 
> > new
> >offset argument that was added in qemu.git/master in the meantime [Peter]
> >
> > 
> 
> Fails to compile, clang:
> 
> ../../hw/remote/mpqemu-link.c:40:29: error: suggest braces around
> initialization of subobject [-Werror,-Wmissing-braces]
> struct iovec send[2] = {0};
> ^
> {}
> 
> 
> Don't use {0}, use {} -- the former may be the C standard thing,
> but the latter is the one all our supported compilers accept
> without complaint. (cf eg commit 039d4e3df0).

Thanks, I update the patch in question.

It looks like the GitLab CI doesn't include a clang version that
produces this error because the pipeline passed for me:
https://gitlab.com/stefanha/qemu/-/pipelines/251524779

Is there something clang-specific you want to check in the CI? Maybe
clang 3.4, the oldest version supported according to ./configure?

Stefan


signature.asc
Description: PGP signature


Re: [PULL v2 00/27] Block patches

2021-02-05 Thread Stefan Hajnoczi
On Thu, Feb 04, 2021 at 10:49:26AM -0800, elena wrote:
> On Thu, Feb 04, 2021 at 05:35:31PM +, Peter Maydell wrote:
> > On Thu, 4 Feb 2021 at 15:43, Stefan Hajnoczi  wrote:
> > >
> > > The following changes since commit 
> > > db754f8ccaf2f073c9aed46a4389e9c0c2080399:
> > >
> > >   Merge remote-tracking branch 
> > > 'remotes/rth-gitlab/tags/pull-tcg-20210202' into staging (2021-02-03 
> > > 19:35:57 +)
> > >
> > > are available in the Git repository at:
> > >
> > >   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> > >
> > > for you to fetch changes up to abe42229db7b87caa11b3c02835ebf9d384e0bd4:
> > >
> > >   docs: fix Parallels Image "dirty bitmap" section (2021-02-04 15:17:10 
> > > +)
> > >
> > > 
> > > Pull request
> > >
> > > v2:
> > >  * Rebase to resolve memory_region_init_ram_from_file() conflict due to 
> > > the new
> > >offset argument that was added in qemu.git/master in the meantime 
> > > [Peter]
> > >
> > > 
> > 
> > Fails to compile, clang:
> > 
> > ../../hw/remote/mpqemu-link.c:40:29: error: suggest braces around
> > initialization of subobject [-Werror,-Wmissing-braces]
> > struct iovec send[2] = {0};
> > ^
> > {}
> 
> Stefan, should we make changes for the patch?

I'll send another revision of this pull request since it's a trivial
fix.

Thanks,
Stefan


signature.asc
Description: PGP signature


Re: [PULL v2 00/27] Block patches

2021-02-04 Thread elena
On Thu, Feb 04, 2021 at 05:35:31PM +, Peter Maydell wrote:
> On Thu, 4 Feb 2021 at 15:43, Stefan Hajnoczi  wrote:
> >
> > The following changes since commit db754f8ccaf2f073c9aed46a4389e9c0c2080399:
> >
> >   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210202' 
> > into staging (2021-02-03 19:35:57 +)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> >
> > for you to fetch changes up to abe42229db7b87caa11b3c02835ebf9d384e0bd4:
> >
> >   docs: fix Parallels Image "dirty bitmap" section (2021-02-04 15:17:10 
> > +)
> >
> > 
> > Pull request
> >
> > v2:
> >  * Rebase to resolve memory_region_init_ram_from_file() conflict due to the 
> > new
> >offset argument that was added in qemu.git/master in the meantime [Peter]
> >
> > 
> 
> Fails to compile, clang:
> 
> ../../hw/remote/mpqemu-link.c:40:29: error: suggest braces around
> initialization of subobject [-Werror,-Wmissing-braces]
> struct iovec send[2] = {0};
> ^
> {}

Stefan, should we make changes for the patch?
Please let us know.

Thanks,

Elena
> 
> 
> Don't use {0}, use {} -- the former may be the C standard thing,
> but the latter is the one all our supported compilers accept
> without complaint. (cf eg commit 039d4e3df0).
> 
> thanks
> -- PMM



Re: [PULL v2 00/27] Block patches

2021-02-04 Thread Peter Maydell
On Thu, 4 Feb 2021 at 15:43, Stefan Hajnoczi  wrote:
>
> The following changes since commit db754f8ccaf2f073c9aed46a4389e9c0c2080399:
>
>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210202' 
> into staging (2021-02-03 19:35:57 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to abe42229db7b87caa11b3c02835ebf9d384e0bd4:
>
>   docs: fix Parallels Image "dirty bitmap" section (2021-02-04 15:17:10 +)
>
> 
> Pull request
>
> v2:
>  * Rebase to resolve memory_region_init_ram_from_file() conflict due to the 
> new
>offset argument that was added in qemu.git/master in the meantime [Peter]
>
> 

Fails to compile, clang:

../../hw/remote/mpqemu-link.c:40:29: error: suggest braces around
initialization of subobject [-Werror,-Wmissing-braces]
struct iovec send[2] = {0};
^
{}


Don't use {0}, use {} -- the former may be the C standard thing,
but the latter is the one all our supported compilers accept
without complaint. (cf eg commit 039d4e3df0).

thanks
-- PMM