From: Halil Pasic <pasic@linux.vnet.ibm.com>
To: Cornelia Huck <cohuck@redhat.com>,
Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>,
Richard Henderson <rth@twiddle.net>,
Alexander Graf <agraf@suse.de>,
"Jason J . Herne" <jjherne@linux.vnet.ibm.com>,
qemu-devel@nongnu.org, Halil Pasic <pasic@linux.vnet.ibm.com>
Subject: [Qemu-devel] [PATCH 2/2] s390x/3270: handle writes of arbitrary length
Date: Wed, 20 Sep 2017 19:23:14 +0200 [thread overview]
Message-ID: <20170920172314.102710-3-pasic@linux.vnet.ibm.com> (raw)
In-Reply-To: <20170920172314.102710-1-pasic@linux.vnet.ibm.com>
The problem is, that the current implementation places unrealistic and
arbitrary constraints on the length of writes to the device (that is the
outbound requests), by asserting ccw.count being such that that even the
worst case escaped payload will fit an more or less arbitrary sized
buffer. Actually on protocol level there is nothing to justify such
a limitation.
Another strange thing is the return value which more or less reflects
the size (written) after escaping instead of before escaping. This
is strange, because this return value is used to calculate SCSW.count.
Let us teach 3270 how to deal with arbitrary long writes.
Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Reported-by: Jason J . Herne <jjherne@linux.vnet.ibm.com>
Tested-by: Jason J . Herne <jjherne@linux.vnet.ibm.com>
---
hw/char/terminal3270.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/hw/char/terminal3270.c b/hw/char/terminal3270.c
index c976a63cc2..a109ce5987 100644
--- a/hw/char/terminal3270.c
+++ b/hw/char/terminal3270.c
@@ -30,7 +30,6 @@ typedef struct Terminal3270 {
uint8_t inv[INPUT_BUFFER_SIZE];
uint8_t outv[OUTPUT_BUFFER_SIZE];
int in_len;
- int out_len;
bool handshake_done;
guint timer_tag;
} Terminal3270;
@@ -145,7 +144,6 @@ static void chr_event(void *opaque, int event)
/* Ensure the initial status correct, always reset them. */
t->in_len = 0;
- t->out_len = 0;
t->handshake_done = false;
if (t->timer_tag) {
g_source_remove(t->timer_tag);
@@ -231,8 +229,9 @@ static int write_payload_3270(EmulatedCcw3270Device *dev, uint8_t cmd)
Terminal3270 *t = TERMINAL_3270(dev);
int retval = 0;
int count = ccw_dstream_avail(get_cds(t));
-
- assert(count <= (OUTPUT_BUFFER_SIZE - 3) / 2);
+ int bound = (OUTPUT_BUFFER_SIZE - 3) / 2;
+ int len = MIN(count, bound);
+ int out_len = 0;
if (!t->handshake_done) {
if (!(t->outv[0] == IAC && t->outv[1] != IAC)) {
@@ -247,16 +246,23 @@ static int write_payload_3270(EmulatedCcw3270Device *dev, uint8_t cmd)
/* We just say we consumed all data if there's no backend. */
return count;
}
- t->outv[0] = cmd;
- ccw_dstream_read_buf(get_cds(t), &t->outv[1], count);
- t->out_len = count + 1;
- t->out_len = insert_IAC_escape_char(t->outv, t->out_len);
- t->outv[t->out_len++] = IAC;
- t->outv[t->out_len++] = IAC_EOR;
+ t->outv[out_len++] = cmd;
+ do {
+ ccw_dstream_read_buf(get_cds(t), &t->outv[out_len], len);
+ count = ccw_dstream_avail(get_cds(t));
+ out_len += len;
- retval = qemu_chr_fe_write_all(&t->chr, t->outv, t->out_len);
- return (retval <= 0) ? 0 : (retval - 3);
+ out_len = insert_IAC_escape_char(t->outv, out_len);
+ if (!count) {
+ t->outv[out_len++] = IAC;
+ t->outv[out_len++] = IAC_EOR;
+ }
+ retval = qemu_chr_fe_write_all(&t->chr, t->outv, out_len);
+ len = MIN(count, bound);
+ out_len = 0;
+ } while (len && retval >= 0);
+ return (retval <= 0) ? 0 : get_cds(t)->count;
}
static Property terminal_properties[] = {
--
2.13.5
next prev parent reply other threads:[~2017-09-20 17:23 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-20 17:23 [Qemu-devel] [PATCH 0/2] 3270 improvements Halil Pasic
2017-09-20 17:23 ` [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via CcwDataStream Halil Pasic
2017-09-21 9:15 ` Cornelia Huck
2017-09-21 11:22 ` Halil Pasic
2017-09-21 12:05 ` Cornelia Huck
2017-09-21 16:11 ` Halil Pasic
2017-09-22 13:38 ` Cornelia Huck
2017-09-20 17:23 ` Halil Pasic [this message]
2017-09-21 9:23 ` [Qemu-devel] [PATCH 2/2] s390x/3270: handle writes of arbitrary length Cornelia Huck
2017-09-21 10:30 ` Halil Pasic
2017-09-20 18:22 ` [Qemu-devel] [PATCH 0/2] 3270 improvements no-reply
2017-09-21 9:24 ` Cornelia Huck
2017-09-21 10:22 ` Halil Pasic
2017-09-21 10:48 ` Cornelia Huck
2017-09-21 11:00 ` Halil Pasic
2017-09-27 14:15 ` Halil Pasic
2017-09-28 15:19 ` Cornelia Huck
2017-09-28 15:20 ` Halil Pasic
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=20170920172314.102710-3-pasic@linux.vnet.ibm.com \
--to=pasic@linux.vnet.ibm.com \
--cc=agraf@suse.de \
--cc=bjsdjshi@linux.vnet.ibm.com \
--cc=borntraeger@de.ibm.com \
--cc=cohuck@redhat.com \
--cc=jjherne@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/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).