From: "Andreas Färber" <afaerber@suse.de>
To: Xuebing Wang <xbing6@gmail.com>, qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Eduardo Habkost <ehabkost@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [Discussion 00/10] about API hierarchy
Date: Tue, 04 Mar 2014 04:45:57 +0100 [thread overview]
Message-ID: <53154C75.9040506@suse.de> (raw)
In-Reply-To: <1393901250-3922-1-git-send-email-xbing6@gmail.com>
Hi Xuebing,
Am 04.03.2014 03:47, schrieb Xuebing Wang:
> Q2) Does it make sense to remove NEED_CPU_H from qemu-common.h?
IMO not in this way. Work has been ongoing to obsolete NEED_CPU_H by
introducing CPUState in addition to CPUArchState and moving fields into
CPUState, so that less files need to include target-xxx/cpu.h. Your
approach seems to be rather spreading it to more and different files.
Please explain in more detail: Why is code being moved from qom/cpu.h to
vmstate.h for target-alpha?
Concerning gdbstub.h, have you investigated replacing remaining
CPUArchState usages with CPUState, like I started in a previous series?
Concerning memory_mapping.h, have you checked the mailing list archives
for reasons why I didn't use my vaddr there myself? Pointing me to my
HACKING entry does not answer that. ;) (Either vaddr didn't exist yet at
the time or there was some reason not to use it, I would hope!)
An important thing to watch out for when moving includes around is
avoiding (more) circular dependencies. CC'ing Eduardo.
Also note that I still haven't fully rebased my qom-cpu-13 branch yet,
which includes a number of field movements and is likely to clash with
some of your refactorings in CPU/exec/translate files.
Your observation is correct that not all things are in the best shape.
The question is in which way and in which order to address them.
To better judge, what are the immediate gains of your proposals? For
example, I don't spot any Makefile changes in your diffstat that benefit
from a dropped cpu.h include somewhere in terms of build dependencies.
Nor does your series include a proof of concept that something becomes
possible that wasn't before. Understanding the exact advantages would
help me in determining what priority to assign such changes. Thanks.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
next prev parent reply other threads:[~2014-03-04 3:46 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-04 2:47 [Qemu-devel] [Discussion 00/10] about API hierarchy Xuebing Wang
2014-03-04 2:47 ` [Qemu-devel] [Discussion 01/10] docs: add docs/api-hierarchy.txt Xuebing Wang
2014-03-04 9:42 ` Stefan Hajnoczi
2014-03-04 9:58 ` Xuebing wang
2014-03-04 11:57 ` Stefan Hajnoczi
2014-03-04 2:47 ` [Qemu-devel] [Discussion 02/10] NEED_CPU_H: remove '#include "cpu.h"' from include/qemu-common.h Xuebing Wang
2014-03-04 10:19 ` Paolo Bonzini
2014-03-04 11:54 ` Xuebing wang
2014-03-04 12:02 ` Xuebing wang
2014-03-04 12:09 ` Paolo Bonzini
2014-03-04 12:09 ` Xuebing wang
2014-03-04 12:34 ` Peter Maydell
2014-03-04 12:40 ` Xuebing wang
2014-03-04 12:19 ` Xuebing wang
2014-03-04 12:23 ` Paolo Bonzini
2014-03-04 12:26 ` Xuebing wang
2014-03-04 12:29 ` Paolo Bonzini
2014-03-04 2:47 ` [Qemu-devel] [Discussion 03/10] NEED_CPU_H: remove unnecessary use of NEED_CPU_H Xuebing Wang
2014-03-04 10:20 ` Paolo Bonzini
2014-03-04 2:47 ` [Qemu-devel] [Discussion 04/10] memory_mapping: make this architecture-independent Xuebing Wang
2014-03-04 10:22 ` Paolo Bonzini
2014-03-04 11:05 ` Peter Maydell
2014-03-04 2:47 ` [Qemu-devel] [Discussion 05/10] NEED_CPU_H: remove unnecessary inclusion of "cpu.h" in root Xuebing Wang
2014-03-04 10:24 ` Paolo Bonzini
2014-03-04 2:47 ` [Qemu-devel] [Discussion 06/10] memory: move contents in include/exec/address-spaces.h => memory.h Xuebing Wang
2014-03-04 10:26 ` Paolo Bonzini
2014-03-04 2:47 ` [Qemu-devel] [Discussion 07/10] memory: remove file include/exec/address-spaces.h Xuebing Wang
2014-03-04 2:47 ` [Qemu-devel] [Discussion 08/10] exec: move TranslationBlock API from exec-all.h => translate.h Xuebing Wang
2014-03-04 10:27 ` Paolo Bonzini
2014-03-04 2:47 ` [Qemu-devel] [Discussion 09/10] exec: remove the unnecessary include of "exec-all.h" Xuebing Wang
2014-03-04 10:27 ` Paolo Bonzini
2014-03-04 11:11 ` Peter Maydell
2014-03-04 11:16 ` Peter Maydell
2014-03-04 2:47 ` [Qemu-devel] [Discussion 10/10] translate: remove file translate-all.h Xuebing Wang
2014-03-04 10:29 ` Paolo Bonzini
2014-03-04 3:45 ` Andreas Färber [this message]
2014-03-04 5:37 ` [Qemu-devel] [Discussion 00/10] about API hierarchy Xuebing wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53154C75.9040506@suse.de \
--to=afaerber@suse.de \
--cc=ehabkost@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=xbing6@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).