Re: [kvm-unit-tests PATCH v3 08/13] s390x: Add linemode console

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18.12.18 12:50, Thomas Huth wrote:
> On 2018-12-18 10:26, Janosch Frank wrote:
>> z/VM isn't fond of vt220, so we need line mode when running under z/VM.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
>>  lib/s390x/sclp-console.c | 195 ++++++++++++++++++++++++++++++++++++++++++-----
>>  lib/s390x/sclp.h         |  67 ++++++++++++++++
>>  2 files changed, 243 insertions(+), 19 deletions(-)
> [...]
>> +
>> +static bool initialized;
>> +
>> +static void sclp_print_ascii(const char *str)
>> +{
>> +	int len = strlen(str);
>> +	WriteEventData *sccb = (void *)_sccb;
>> +
>> +	sclp_wait_busy();
>> +	sclp_busy = true;
> 
> What happened to sclp_mark_busy() ?

It seems this got lost in a rebase where a search/replace took place -_-

> 
>> +	memset(sccb, 0, sizeof(*sccb));
>> +	sccb->h.length = sizeof(WriteEventData) + len;
>> +	sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
>> +	sccb->ebh.length = sizeof(EventBufferHeader) + len;
>> +	sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
>> +	memcpy(sccb->data, str, len);
>> +
>> +	sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
>> +	sclp_wait_busy();
> 
> Hmm, if the functions do a sclp_wait_busy() at the beginning already, do
> we still need it at the end of the functions, too?
> 
> ... I've got the feeling that we need some clearer conventions here.
> Could you maybe add some comments to the source code?

I'll integrate it into sclp_service_call

> 
>> +}
>> +
>> +static void sclp_print_lm(const char *str)
>> +{
>> +	unsigned char *ptr, *end, ch;
>> +	unsigned int count, offset, len;
>> +	struct write_sccb *sccb;
>> +	struct msg_buf *msg;
>> +	struct mdb *mdb;
>> +	struct mto *mto;
>> +	struct go *go;
>> +
>> +	sclp_wait_busy();
>> +	sclp_busy = true;
> 
> sclp_mark_busy() ?
> 
>> +	sccb = (struct write_sccb *) _sccb;
>> +	end = (unsigned char *) sccb + 4096 - 1;
>> +	memset(sccb, 0, sizeof(*sccb));
>> +	ptr = (unsigned char *) &sccb->msg.mdb.mto;
>> +	len = strlen(str);
>> +	offset = 0;
[...]
>> +	}
> 
> Maybe rather assert(sccb->h.response_code == SCLP_RC_NORMAL_COMPLETION) ?
> It does not make much sense to continue otherwise, does it?

Will the assert abort without any further prints?

> 
>  Thomas
> 


Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux