Patchwork [1/2] char: introduce a blocking version of qemu_chr_fe_write

login
register
mail settings
Submitter Anthony Liguori
Date March 26, 2013, 3:11 p.m.
Message ID <1364310706-10851-1-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/231253/
State New
Headers show

Comments

Anthony Liguori - March 26, 2013, 3:11 p.m.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 include/char/char.h | 15 +++++++++++++++
 qemu-char.c         | 27 +++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)
Peter Maydell - March 26, 2013, 3:21 p.m.
On 26 March 2013 15:11, Anthony Liguori <aliguori@us.ibm.com> wrote:
> +int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len)
> +{
> +    int offset = 0;
> +    int res;
> +
> +    while (offset < len) {
> +        do {
> +            res = s->chr_write(s, buf + offset, len - offset);
> +            if (res == -1 && errno == EAGAIN) {
> +                g_usleep(100);
> +            }
> +        } while (res == -1 && errno == EAGAIN);

   for (;;) {
       res = s->chr_write(s, buf + offset, len - offset);
       if (!(res == -1 && errno == EAGAIN)) {
           break;
       }
       g_usleep(100);
   }

would avoid the duplication of the retry condition.

-- PMM
Wayne Xia - March 27, 2013, 7:21 a.m.
于 2013-3-26 23:21, Peter Maydell 写道:
> On 26 March 2013 15:11, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> +int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len)
>> +{
>> +    int offset = 0;
>> +    int res;
>> +
>> +    while (offset < len) {
>> +        do {
>> +            res = s->chr_write(s, buf + offset, len - offset);
>> +            if (res == -1 && errno == EAGAIN) {
>> +                g_usleep(100);
>> +            }
>> +        } while (res == -1 && errno == EAGAIN);
>
>     for (;;) {
>         res = s->chr_write(s, buf + offset, len - offset);
>         if (!(res == -1 && errno == EAGAIN)) {
>             break;
>         }
>         g_usleep(100);
>     }
>
> would avoid the duplication of the retry condition.
>
> -- PMM
>
I think adjust like following make code easier to read:

     while (offset < len) {
         res = s->chr_write(s, buf + offset, len - offset);
         if (res == -1 && errno == EAGAIN) {
                 g_usleep(100)
                 continue;
         }
         .....
     }
Anthony Liguori - March 27, 2013, 9:15 p.m.
Applied.  Thanks.

Regards,

Anthony Liguori
Peter Maydell - March 27, 2013, 10:05 p.m.
On 27 March 2013 21:15, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Applied.  Thanks.

Replied to wrong email by accident, or applied ignoring
the review comments?

-- PMM
Anthony Liguori - March 27, 2013, 11:01 p.m.
Peter Maydell <peter.maydell@linaro.org> writes:

> On 27 March 2013 21:15, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> Applied.  Thanks.
>
> Replied to wrong email by accident, or applied ignoring
> the review comments?

I interpreted your comment as a suggestion.  I'm not a fan of while
(true) loops so I left it as is.  Since it's a pretty important bug fix
(make check was failing), I tried to get it applied quickly.  I should
have responded to your mail before applying it though.

Regards,

Anthony Liguori

>
> -- PMM

Patch

diff --git a/include/char/char.h b/include/char/char.h
index 0326b2a..5c3a7a5 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -170,6 +170,21 @@  int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
 int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len);
 
 /**
+ * @qemu_chr_fe_write_all:
+ *
+ * Write data to a character backend from the front end.  This function will
+ * send data from the front end to the back end.  Unlike @qemu_chr_fe_write,
+ * this function will block if the back end cannot consume all of the data
+ * attempted to be written.
+ *
+ * @buf the data
+ * @len the number of bytes to send
+ *
+ * Returns: the number of bytes consumed
+ */
+int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len);
+
+/**
  * @qemu_chr_fe_ioctl:
  *
  * Issue a device specific ioctl to a backend.
diff --git a/qemu-char.c b/qemu-char.c
index 4e011df..936150f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -140,6 +140,33 @@  int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len)
     return s->chr_write(s, buf, len);
 }
 
+int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len)
+{
+    int offset = 0;
+    int res;
+
+    while (offset < len) {
+        do {
+            res = s->chr_write(s, buf + offset, len - offset);
+            if (res == -1 && errno == EAGAIN) {
+                g_usleep(100);
+            }
+        } while (res == -1 && errno == EAGAIN);
+
+        if (res == 0) {
+            break;
+        }
+
+        if (res < 0) {
+            return res;
+        }
+
+        offset += res;
+    }
+
+    return offset;
+}
+
 int qemu_chr_fe_ioctl(CharDriverState *s, int cmd, void *arg)
 {
     if (!s->chr_ioctl)