Re: [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic"

[John Snow]

On 9/19/19 6:37 AM, Stefan Hajnoczi wrote:
> On Wed, Sep 18, 2019 at 11:19:40PM +0000, Oleinik, Alexander wrote:
>> When using qtest "in-process" communication, qtest_sendf directly calls
>> a function in the server (qtest.c). Combining the contents of the
>> subsequent socket_sends into the qtest_sendf, makes it so the server can
>> immediately handle the command, without building a local buffer and
>> waiting for a newline.
>>
>> Signed-off-by: Alexander Oleinik <address@hidden>
>> ---
>>  tests/libqtest.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index 19feea9e17..d770462869 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, 
>> const void *data, size_t size)
>>      gchar *bdata;
>>  
>>      bdata = g_base64_encode(data, size);
>> -    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
>> -    socket_send(s->fd, bdata, strlen(bdata));
>> -    socket_send(s->fd, "\n", 1);
>> +    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx %s\n", addr, size, bdata);
>>      qtest_rsp(s, 0);
>>      g_free(bdata);
>>  }
>> -- 
>> 2.23.0
> 
> Cc John Snow, who added the b64write command.
> 
> The downside to doing this is that sprintf-formatting needs to be
> performed on the entire base64 buffer.  This slows things down slightly
> and a larger temporary buffer needs to be allocated, but I'm not sure it
> matters.
> 

*struggles to remember*

I guess I wanted something that had some space savings while maintaining
some semblance of debuggability. This is almost certainly meant for AHCI
tests where it's writing various patterns to large blocks of memory.

I doubt I really measured the performance of it, but it seemed like the
way to go for transferring medium amounts of data at the time via the
qtest protocol.

Looks like I am the only user of it, still:

tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx, bufsize);
tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx, bufsize);
tests/libqos/ahci.c:        qtest_bufwrite(ahci->parent->qts, ptr,
buffer, bufsize);

The buffers can be quite large, so you might be re-buffering a decent
amount of data from the sender now.

1, Are large transfers like this guaranteed to be atomic anyway? What
kind of socket is it? we're probably eclipsing frame and packet sizes here.

2, I am not sure what being "atomic" affords us in terms of allowing the
server to not wait for newlines, how does this change help?

--js