Re: [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs

[Halil Pasic]

On 08/24/2017 11:09 AM, Cornelia Huck wrote:
> On Thu, 24 Aug 2017 09:43:48 +0200
> Christian Borntraeger <address@hidden> wrote:
> 
>> On 08/24/2017 09:38 AM, Christian Borntraeger wrote:
>>>
>>>
>>> On 08/23/2017 05:54 PM, Cornelia Huck wrote:  
>>>> Some non-pci code calls into zpci code. Provide some stubs for builds
>>>> without pci.
>>>>
>>>> Reviewed-by: Thomas Huth <address@hidden>
>>>> Signed-off-by: Cornelia Huck <address@hidden>
>>>> ---
>>>>  hw/s390x/Makefile.objs   |  3 +-
>>>>  hw/s390x/s390-pci-stub.c | 74 
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 76 insertions(+), 1 deletion(-)
>>>>  create mode 100644 hw/s390x/s390-pci-stub.c
> 
>>>> +/* hw/s390x/sclp.c */
>>>> +void s390_pci_sclp_configure(SCCB *sccb)
>>>> +{
>>>> +}
>>>> +
>>>> +void s390_pci_sclp_deconfigure(SCCB *sccb)
>>>> +{
>>>> +}  
>>>
>>> shouldnt these function set an error code in the sccb, e.g.
>>> something like
>>>
>>>    sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
>>>
>>>
>>>   
>>
>> Oh you have something like that in patch 7. Maybe move?
> 
> Does not really change anything in practice, but I can move it.
> 

You mean these stubs are not supposed to be reachable and are just
for making the linker happy, or? If that's the case I would prefer
having that expressed by something like assert(false) or even 
#define NOT_REACHABLE assert(false).

Otherwise patch looks good, but I did not a full review on it,
so let's try this:
Acked-by: Halil Pasic <address@hidden>