Re: [Qemu-devel] [PATCH 3/7] s390: sclp base support

[Christian Borntraeger]

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.