Bug 208026 - qemu.spec violates PackagingGuidelines / ReviewGuidelines
qemu.spec violates PackagingGuidelines / ReviewGuidelines
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: qemu (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: David Woodhouse
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-25 17:24 EDT by Till Maas
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-25 17:38:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Till Maas 2006-09-25 17:24:16 EDT
Description of problem:
- You do not use the RPM_OPT_FLAGS:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-8b14098227aebff1cf6188939e9d0877295ac448

and Review/Guidelines
- The Licenses are included in the upstream tarball but not in the rpm
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

As well -fomit-frame-pointer destroys debuginfo:
https://www.redhat.com/archives/fedora-extras-list/2006-July/msg00712.html
Comment 1 David Woodhouse 2006-09-25 17:38:11 EDT
Feel free to provide a tested patch which doesn't affect performance.
Comment 2 Hans de Goede 2007-01-31 03:04:14 EST
Hi David,

As you may or may not know this bug has lead to some discussion on the
mailinglist. So I decided to take a look.

I understand your pov, but has there been done test how much performance penalty
not using fomit-framepointer (and maybe enabling something like stack smashing
protection) yields?

Would it otherwise be an idea to create several versions of qemu with different
compile flags. So one with upstream's compile flags and another one in a
subpackage with RPM_OPT_FLAGS?

I must say that I don't like either the not being able to debug, nor the not
having the security features enabled "features" of the current package, esp. not
having the security features enabled may be an issue as qemu is used by people
to run code from unknown origine in a sandbox way. I must admit that the
possibilities for attacks through this are small, and the chances of such an
attack actually being coded even smaller.

Comment 3 David Woodhouse 2007-01-31 04:10:19 EST
Sorry, I was overly terse in my response. And I shouldn't have put such an
emphasis on performance. 

There is discussion on this subject on the fedora-maintainers list, at
http://www.redhat.com/archives/fedora-maintainers/2007-January/msg00403.html

For the record, I'll repeat what I said there, since it's a better way of
stating my opinion than comment #1 above...

Let's start with an excerpt from the changelog for the package in question...

* Sat Mar 18 2006 David Woodhouse <dwmw2@infradead.org> 0.8.0-5
- Just drop $RPM_OPT_FLAGS. They're too much of a PITA

* Sat Mar 18 2006 David Woodhouse <dwmw2@infradead.org> 0.8.0-4
- Disable stack-protector options which gcc 3.2 doesn't like

* Fri Mar 17 2006 David Woodhouse <dwmw2@infradead.org> 0.8.0-3
- Use -mcpu= instead of -mtune= on x86_64 too
- Disable SPARC targets on x86_64, because dyngen doesn't like fnegs

* Fri Mar 17 2006 David Woodhouse <dwmw2@infradead.org> 0.8.0-2
- Don't use -mtune=pentium4 on i386. GCC 3.2 doesn't like it

* Fri Mar 17 2006 David Woodhouse <dwmw2@infradead.org> 0.8.0-1
- Update to 0.8.0
- Resort to using compat-gcc-32
- Enable ALSA

* Mon May 16 2005 David Woodhouse <dwmw2@infradead.org> 0.7.0-2
- Proper fix for GCC 4 putting 'blr' or 'ret' in the middle of the function,
  for i386, x86_64 and PPC.

* Sat Apr 30 2005 David Woodhouse <dwmw2@infradead.org> 0.7.0-1
- Update to 0.7.0
- Fix dyngen for PPC functions which end in unconditional branch


Qemu is, as stated, a pain in the arse. It's _very_ dependent on
compiler output, because of the way it takes 'snippets' of code from
tiny functions and uses those as the building blocks for its
translation.

In particular, we can't use GCC 4.x for it, because that'll tend to
insert 'blr' instructions in the middle of function calls rather than
jumping to the end and having only a single return -- and that really
makes qemu unhappy when it just knocks the 'blr' off the end and sticks
those blocks of code together sequentially. When we used GCC 4 I think
there was also a problem with global register variables on some crappy
arch with too few registers too.

There's work on making qemu happy with current compilers, but it's not
ready yet. I've poked at it myself on occasion, as evidenced above, but
there's _always_ something more important to do and something else that
goes wrong with it just when you think it's OK. And the work I _have_
done on qemu recently has been on NPTL support in the qemu-user tools,
so you can actually run current userspace (I want i386 flash plugin in
qemu in nspluginwrapper in my ppc firefox, and I actually have it just
about working).

So we can't "just use $RPM_OPT_FLAGS" because some of those flags don't
work with the older compiler. Although there _is_ a possibility of using
one compiler for dyngen and another for qemu proper, which could perhaps
be investigated.

The bit about -fomit-frame-pointer I wasn't really paying much attention
to -- I never debug on i386 anyway, and in fact I rarely debug qemu
itself; more often I want to debug the stuff I run _in_ it.

The above-quoted sentence should be taken with emphasis on the "provide
a tested patch" part rather than the "doesn't affect performance" part.
And in the context of the changelog...

And it should be taken at face value -- really, feel free to provide a
patch. It wasn't just a brush-off. It's not that I don't want to do it.
It's not (just) that I'm lazy. It's that I tried, and I failed, and I
ended up reverting to what we have at the moment.

There is probably room for improvement -- perhaps by using GCC 4 for
most of it but GCC 3 for dyngen. But then you have to deal with that
crappy register-starved arch, I think, which is why I didn't do it
before. Although I have to admit I _was_ tempted to ExcludeArch: i386 :)
Comment 4 Patrice Dumas 2007-02-01 08:53:18 EST
What I do in that case (I still have to compile some code 
with g77 and not gfortran), is the following:

export FFLAGS=`echo "%optflags" | sed -e 's/-mtune=[^ ]\+//' -e
's/-fstack-protector//' -e 's/--param=ssp-buffer-size=[^ ]\+//'`

before %configure. Maybe you could do something similar?

Note You need to log in before you can comment on or make changes to this bug.