[libvirt] [PATCH] qemu driver: use lseek64 for setting position in logfile

[Daniel Veillard]

On Tue, Jan 04, 2011 at 09:46:07AM -0500, Stefan Berger wrote:
> While doing some testing with Qemu and creating huge logfiles I
> encountered the case where the VM could not start anymore due to the
> lseek() to the end of the Qemu VM's log file failing. The patch
> below replaces the two occurrences of lseek() in the relevant path
> with lseek64() and solves this problem. It may be a good idea to
> look at other occurrences of lseek() as well whether they should be
> replaced. off_t is 8 bytes long (64 bit), so it doesn't need to be
> replaced with the explicit off64_t.
> 
> To reproduce this error, you could do the following:
> 
> dd if=/dev/zero of=/var/log/libvirt/qemu/<name of VM>.log bs=1024
> count=$((1024*2048))
> 
> and you should get an error like this:
> 
> error: Failed to start domain <name of VM>
> error: Unable to seek to -2147482651 in /var/log/libvirt/qemu/<name
> of VM>.log: Success
> 
> Signed-off-by: Stefan Berger <stefanb at us.ibm.com>
> 
> ---
>  src/qemu/qemu_driver.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Index: libvirt-acl/src/qemu/qemu_driver.c
> ===================================================================
> --- libvirt-acl.orig/src/qemu/qemu_driver.c
> +++ libvirt-acl/src/qemu/qemu_driver.c
> @@ -238,7 +238,7 @@ qemudLogReadFD(const char* logDir, const
>          VIR_FREE(logfile);
>          return -1;
>      }
> -    if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) {
> +    if (pos < 0 || lseek64(fd, pos, SEEK_SET) < 0) {
>          virReportSystemError(pos < 0 ? 0 : errno,
>                               _("Unable to seek to %lld in %s"),
>                               (long long) pos, logfile);
> @@ -2624,7 +2624,7 @@ static int qemudStartVMDaemon(virConnect
>                                enum virVMOperationType vmop) {
>      int ret;
>      unsigned long long qemuCmdFlags;
> -    int pos = -1;
> +    off_t pos = -1;
>      char ebuf[1024];
>      char *pidfile = NULL;
>      int logfile = -1;
> @@ -2839,7 +2839,7 @@ static int qemudStartVMDaemon(virConnect
> 
>      virCommandWriteArgLog(cmd, logfile);
> 
> -    if ((pos = lseek(logfile, 0, SEEK_END)) < 0)
> +    if ((pos = lseek64(logfile, 0, SEEK_END)) < 0)
>          VIR_WARN("Unable to seek to end of logfile: %s",
>                   virStrerror(errno, ebuf, sizeof ebuf));

  Looks fine to me, ACK

But the real problem is getting huge log files, there is a trade off
between the default log level and the amount of information we want
to have handy in case something goes wrong.
One of the thing I plan to implement is to change the default behaviour
to log all message to an in-memory ring buffer, and in case of an error
at runtime log that buffer collecting the past recent detailed informations
of what happened before the current error. I assume this will have to
be tuned too to avoid excessive default logging, but this should improve
at least the usefulness of those logs,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/