On 2016/01/28 18:48, Xie, Huawei wrote:
> On 1/28/2016 10:47 AM, Tetsuya Mukawa wrote:
>> On 2016/01/28 0:58, Xie, Huawei wrote:
>>> On 1/21/2016 7:09 PM, Tetsuya Mukawa wrote:
>>> [snip]
>>>> +
>>>> +static int
>>>> +qtest_raw_recv(int fd, char *buf, size_t count)
>>>> +{
>>>> + size_t len = count;
>>>> + size_t total_len = 0;
>>>> + int ret = 0;
>>>> +
>>>> + while (len > 0) {
>>>> + ret = read(fd, buf, len);
>>>> + if (ret == (int)len)
>>>> + break;
>>>> + if (*(buf + ret - 1) == '\n')
>>>> + break;
>>> The above two lines should be put after the below if block.
>> Yes, it should be so.
>>
>>>> + if (ret == -1) {
>>>> + if (errno == EINTR)
>>>> + continue;
>>>> + return ret;
>>>> + }
>>>> + total_len += ret;
>>>> + buf += ret;
>>>> + len -= ret;
>>>> + }
>>>> + return total_len + ret;
>>>> +}
>>>> +
>>> [snip]
>>>
>>>> +
>>>> +static void
>>>> +qtest_handle_one_message(struct qtest_session *s, char *buf)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + if (strncmp(buf, interrupt_message, strlen(interrupt_message)) == 0) {
>>>> + if (rte_atomic16_read(&s->enable_intr) == 0)
>>>> + return;
>>>> +
>>>> + /* relay interrupt to pipe */
>>>> + ret = write(s->irqfds.writefd, "1", 1);
>>>> + if (ret < 0)
>>>> + rte_panic("cannot relay interrupt\n");
>>>> + } else {
>>>> + /* relay normal message to pipe */
>>>> + ret = qtest_raw_send(s->msgfds.writefd, buf, strlen(buf));
>>>> + if (ret < 0)
>>>> + rte_panic("cannot relay normal message\n");
>>>> + }
>>>> +}
>>>> +
>>>> +static char *
>>>> +qtest_get_next_message(char *p)
>>>> +{
>>>> + p = strchr(p, '\n');
>>>> + if ((p == NULL) || (*(p + 1) == '\0'))
>>>> + return NULL;
>>>> + return p + 1;
>>>> +}
>>>> +
>>>> +static void
>>>> +qtest_close_one_socket(int *fd)
>>>> +{
>>>> + if (*fd > 0) {
>>>> + close(*fd);
>>>> + *fd = -1;
>>>> + }
>>>> +}
>>>> +
>>>> +static void
>>>> +qtest_close_sockets(struct qtest_session *s)
>>>> +{
>>>> + qtest_close_one_socket(&s->qtest_socket);
>>>> + qtest_close_one_socket(&s->msgfds.readfd);
>>>> + qtest_close_one_socket(&s->msgfds.writefd);
>>>> + qtest_close_one_socket(&s->irqfds.readfd);
>>>> + qtest_close_one_socket(&s->irqfds.writefd);
>>>> + qtest_close_one_socket(&s->ivshmem_socket);
>>>> +}
>>>> +
>>>> +/*
>>>> + * This thread relays QTest response using pipe.
>>>> + * The function is needed because we need to separate IRQ message from
>>>> others.
>>>> + */
>>>> +static void *
>>>> +qtest_event_handler(void *data) {
>>>> + struct qtest_session *s = (struct qtest_session *)data;
>>>> + char buf[1024];
>>>> + char *p;
>>>> + int ret;
>>>> +
>>>> + for (;;) {
>>>> + memset(buf, 0, sizeof(buf));
>>>> + ret = qtest_raw_recv(s->qtest_socket, buf, sizeof(buf));
>>>> + if (ret < 0) {
>>>> + qtest_close_sockets(s);
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + /* may receive multiple messages at the same time */
>>> From the qtest_raw_recv implementation, if at some point one message is
>>> received by two qtest_raw_recv calls, then is that message discarded?
>>> We could save the last incomplete message in buffer, and combine the
>>> message received next time together.
>> I guess we don't lose replies from QEMU.
>> Please let me describe more.
>>
>> According to the qtest specification, after sending a message, we need
>> to receive a reply like below.
>> APP: ---command---> QEMU
>> APP: <-----------OK---- QEMU
>>
>> But, to handle interrupt message, we need to take care below case.
>> APP: ---command---> QEMU
>> APP: <---interrupt---- QEMU
>> APP: <-----------OK---- QEMU
>>
>> Also, we need to handle a case like multiple threads tries to send a
>> qtest message.
>> Anyway, here is current implementation.
>>
>> So far, we have 3 types of sockets.
>> 1. socket for qtest messaging.
>> 2. socket for relaying normal message.
>> 3. socket for relaying interrupt message.
>>
>> About read direction:
>> The qtest socket is only read by "qtest_event_handler". The handler may
>> receive multiple messages at once.
> I think there are two assumptions that all messages are ended with "\n"
> and the sizeof(buf) could hold the maximum length of sum of all multiple
> messages that QEMU could send at one time.
> Otherwise in the last read call of qtest_raw_receive, you might receive
> only part of the a message.
I've got your point. I will fix above case.
Thanks,
Tetsuya
