[libvirt] [PATCH v2 07/13] qemu: introduce a qemuDomainLogContext object

[John Ferlan]

On 11/12/2015 12:19 PM, Daniel P. Berrange wrote:
> Introduce a qemuDomainLogContext object to encapsulate
> handling of I/O to/from the domain log file. This will
> hide details of the log file implementation from the
> rest of the driver, making it easier to introduce
> support for virtlogd later.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/qemu/qemu_domain.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_domain.h |  22 ++++++
>  2 files changed, 200 insertions(+)
> 

[...]

>  
> +qemuDomainLogContextPtr qemuDomainLogContextNew(virQEMUDriverPtr driver,
> +                                                virDomainObjPtr vm,
> +                                                qemuDomainLogContextMode mode)
> +{
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    qemuDomainLogContextPtr ctxt = NULL;
> +    char *logfile = NULL;
> +
> +    if (VIR_ALLOC(ctxt) < 0)
> +        goto error;
> +
> +    ctxt->writefd = -1;
> +    ctxt->readfd = -1;
> +    virAtomicIntSet(&ctxt->refs, 1);
> +
> +    if (virAsprintf(&logfile, "%s/%s.log", cfg->logDir, vm->def->name) < 0)
> +        goto error;
> +
> +    if ((ctxt->writefd = open(logfile, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) {
> +        virReportSystemError(errno, _("failed to create logfile %s"),
> +                             logfile);
> +        goto error;
> +    }
> +    if (virSetCloseExec(ctxt->writefd) < 0) {
> +        virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
> +                             logfile);
> +        goto error;
> +    }
> +
> +    /* For unprivileged startup we must truncate the file since
> +     * we can't rely on logrotate. We don't use O_TRUNC since
> +     * it is better for SELinux policy if we truncate afterwards */
> +    if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START &&
> +        !virQEMUDriverIsPrivileged(driver) &&
> +        ftruncate(ctxt->writefd, 0) < 0) {
> +        virReportSystemError(errno, _("failed to truncate %s"),
> +                             logfile);
> +        goto error;
> +    }
> +
> +    if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START) {
> +        if ((ctxt->readfd = open(logfile, O_RDONLY, S_IRUSR | S_IWUSR)) < 0) {
> +            virReportSystemError(errno, _("failed to open logfile %s"),
> +                                 logfile);
> +            goto error;
> +        }
> +        if (virSetCloseExec(ctxt->readfd) < 0) {
> +            virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
> +                                 logfile);
> +            goto error;
> +        }
> +    }

should 'pos' be initialized here?  Otherwise the first ContextRead would
use ctxt->pos == 0


> +
> +    virObjectUnref(cfg);
> +    return ctxt;
> +
> + error:
> +    virObjectUnref(cfg);
> +    qemuDomainLogContextFree(ctxt);
> +    return NULL;
> +}
> +
> +

[...]

> +
> +ssize_t qemuDomainLogContextRead(qemuDomainLogContextPtr ctxt,
> +                                 char **msg)
> +{
> +    char *buf;
> +    size_t buflen = 1024 * 128;
> +    ssize_t got;
> +
> +    /* Best effort jump to start of messages */
> +    ignore_value(lseek(ctxt->readfd, ctxt->pos, SEEK_SET));
> +
> +    if (VIR_ALLOC_N(buf, buflen) < 0)
> +        return -1;
> +
> +    got = saferead(ctxt->readfd, buf, buflen - 1);
> +    if (got < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to read from log file"));

Coverity points out buf is leaked.

> +        return -1;
> +    }
> +
> +    buf[got] = '\0';
> +
> +    ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1));
> +    *msg = buf;
> +
> +    return got;
> +}
> +
> +

ACK with the obvious adjustment - not quite sure how to handle 'pos'
(yet) - although perhaps future patches will allow the light to shine.

John