[libvirt] Re: [PATCH] use qemudReadLogOutput

[Jim Meyering]

Guido Günther <agx at sigxcpu.org> wrote:
...
>>From 6655474aed6e601a6c2e16e7020589f51f58893b Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx at sigxcpu.org>
> Date: Sat, 7 Feb 2009 17:16:39 +0100
> Subject: [PATCH] usleep to wait for domain logfile to fill up
>
> ---
>  src/qemu_driver.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 57 insertions(+), 8 deletions(-)
>
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 47ca6c7..09be3fb 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -601,6 +601,7 @@ qemudReadMonitorOutput(virConnectPtr conn,
>  {
>      int got = 0;
>      buf[0] = '\0';
> +    timeout *= 1000; /* poll wants milli seconds */
>
>      /* Consume & discard the initial greeting */
>      while (got < (buflen-1)) {
> @@ -662,6 +663,56 @@ qemudReadMonitorOutput(virConnectPtr conn,
>
>  }
>
> +
> +/*
> + * Returns -1 for error, 0 on success
> + */
> +static int
> +qemudReadLogOutput(virConnectPtr conn,
> +                   virDomainObjPtr vm,
> +                   int fd,
> +                   char *buf,
> +                   int buflen,
> +                   qemudHandlerMonitorOutput func,
> +                   const char *what,
> +                   int timeout)
> +{
> +    int got = 0;
> +    int ret;
> +    int retries = timeout*10;
> +    buf[0] = '\0';
> +
> +    while (retries) {
> +        while((ret = read(fd, buf+got, buflen-got-1)) > 0) {
> +            got += ret;
> +            buf[got] = '\0';
> +            if ((buflen-got-1) == 0) {
> +                qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> +                                 _("Out of space while reading %s log output"), what);
> +                return -1;
> +            }
> +        }

Hi Guido,
Sorry I didn't review this sooner,
but I looked just today after you committed it.

Here's a proposed patch to make it use better types
(always suspect that using "int" is wrong ;-).

Also, shouldn't it handle read failing with EAGAIN?

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 09be3fb..ae393be 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -672,17 +672,17 @@ qemudReadLogOutput(virConnectPtr conn,
                    virDomainObjPtr vm,
                    int fd,
                    char *buf,
-                   int buflen,
+                   size_t buflen,
                    qemudHandlerMonitorOutput func,
                    const char *what,
                    int timeout)
 {
-    int got = 0;
-    int ret;
     int retries = timeout*10;
     buf[0] = '\0';

     while (retries) {
+        ssize_t ret;
+        size_t got = 0;
         while((ret = read(fd, buf+got, buflen-got-1)) > 0) {
             got += ret;
             buf[got] = '\0';