From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:54768)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <borntraeger@de.ibm.com>) id 1Svr44-0004PO-Up
	for qemu-devel@nongnu.org; Mon, 30 Jul 2012 10:35:30 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <borntraeger@de.ibm.com>) id 1Svr3x-0004JM-8b
	for qemu-devel@nongnu.org; Mon, 30 Jul 2012 10:35:20 -0400
Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:54715)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <borntraeger@de.ibm.com>) id 1Svr3x-0004JE-02
	for qemu-devel@nongnu.org; Mon, 30 Jul 2012 10:35:13 -0400
Received: from /spool/local
	by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use
	Only! Violators will be prosecuted
	for <qemu-devel@nongnu.org> from <borntraeger@de.ibm.com>;
	Mon, 30 Jul 2012 15:35:11 +0100
Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com
	[9.149.37.249])
	by d06nrmr1806.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with
	ESMTP id q6UEYdoV2252826
	for <qemu-devel@nongnu.org>; Mon, 30 Jul 2012 15:34:39 +0100
Received: from d06av08.portsmouth.uk.ibm.com (loopback [127.0.0.1])
	by d06av08.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with
	ESMTP id q6UEYcsC007723
	for <qemu-devel@nongnu.org>; Mon, 30 Jul 2012 08:34:39 -0600
Message-ID: <50169B7E.2080408@de.ibm.com>
Date: Mon, 30 Jul 2012 16:34:38 +0200
From: Christian Borntraeger <borntraeger@de.ibm.com>
MIME-Version: 1.0
References: <1343115430-34285-1-git-send-email-borntraeger@de.ibm.com>
	<1343115430-34285-4-git-send-email-borntraeger@de.ibm.com>
	<B840FE45-0F56-4D99-A0B1-D5DFA39EF72C@suse.de>
In-Reply-To: <B840FE45-0F56-4D99-A0B1-D5DFA39EF72C@suse.de>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 3/7] s390: sclp base support
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alexander Graf <agraf@suse.de>
Cc: qemu-devel <qemu-devel@nongnu.org>, blauwirbel@gmail.com, Heinz Graalfs <graalfs@linux.vnet.ibm.com>, Jens Freimann <jfrei@linux.vnet.ibm.com>, afaerber@suse.de

On 30/07/12 14:38, Alexander Graf wrote:

>> +/* There is one SCLP bus per machine */
>> +static SCLPS390Bus *sbus;
> 
> ... but there isn't necessarily one machine per qemu instance. Today there is, 
> but we shouldn't rely on that fact. Please move the bus variable into a machine
> struct that only the machine knows about.

Could do. However, we have the same issue with the virtio bus. Would it make sense to leave it
here as is and the do a cleanup later on?

[...]
>> +void sclp_service_interrupt(uint32_t sccb)
> 
> Does this have to be globally visible? If all the sclp source ends up in this file, it should only be necessary internal to it, right?

It will be called by the console and quiesce code

> 
>> +{
>> +    if (!sccb) {
> 
> Please add a comment when this would trigger. In fact, I'm not sure I fully understand the reason for this function :).

It will be enhanced by patch 4. Will move that hunk completely there. See also the explanation for patch 4.