Re: [libvirt] [PATCH 3/5] Move qemuProcessLogReadFD and qemuProcessLogFD methods

5 May 2011

      On 05/05/2011 05:51 AM, Daniel P. Berrange wrote:
...
Move the qemuProcessLogReadFD and qemuProcessLogFD methods
into qemu_domain.c, renaming them to qemuDomainCreateLog
and qemuDomainOpenLog.
* src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add
  qemuDomainCreateLog and qemuDomainOpenLog.
* src/qemu/qemu_process.c: Remove qemuProcessLogFD
  and qemuProcessLogReadFD
...
+static int
+qemuDomainOpenLogHelper(struct qemud_driver *driver,
+                        virDomainObjPtr vm,
+                        int flags,
+                        mode_t mode)
+{
+    char *logfile;
+    int fd = -1;
+
+    if (virAsprintf(&logfile, "%s/%s.log", driver->logDir, vm->def->name) < 0) {
+        virReportOOMError();
+        return -1;
+    }
+
+    if ((fd = open(logfile, flags, mode)) < 0) {
+        virReportSystemError(errno, _("failed to create logfile %s"),
+                             logfile);
+        goto cleanup;
+    }
+    if (virSetCloseExec(fd) < 0) {
I still need to implement O_CLOEXEC in gnulib.  Someday...
...
+        virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
+                             logfile);
+        VIR_FORCE_CLOSE(fd);
+        goto cleanup;
+    }
+
+cleanup:
+    VIR_FREE(logfile);
+    return fd;
+
Not quite straight code motion - you rewrote the error handling to be
saner.  But I like it (it's a small enough code motion to be reviewable,
not like some of those thousand-liners in the past).

}
...
+
+
+int
+qemuDomainCreateLog(struct qemud_driver *driver, virDomainObjPtr vm, bool append)
+{
+    int flags;
+
+    flags = O_CREAT | O_WRONLY;
+    /* Only logrotate files in /var/log, so only append if running privileged */
+    if (driver->privileged || append)
+        flags |= O_APPEND;
+    else
+        flags |= O_TRUNC;
+
+    return qemuDomainOpenLogHelper(driver, vm, flags, S_IRUSR | S_IWUSR);
+}
+
+
+int
+qemuDomainOpenLog(struct qemud_driver *driver, virDomainObjPtr vm, off_t pos)
+{
+    int fd;
+    off_t off;
+    int whence;
+
+    if ((fd = qemuDomainOpenLogHelper(driver, vm, O_RDONLY, 0)) < 0)
+        return -1;
+
+    if (pos < 0) {
+        off = 0;
+        whence = SEEK_END;
+    } else {
+        off = pos;
+        whence = SEEK_SET;
+    }
+
+    if (lseek(fd, off, whence) < 0) {
+        virReportSystemError(pos < 0 ? 0 : errno,
+                             _("Unable to seek to %lld from %s in log for %s"),
+                             (long long)off,
+                             whence == SEEK_END ? "end" : "start",
_("end"), _("start")

otherwise the translation looks funny for injecting an English word into
a foreign sentence

In fact, translators would probably prefer that you had two strings:

whence == SEEK_END ?
_("unable to seek to end of log for %$2s") :
_("unable to seek to %lld from start for %s"),
(long long) off, vm->def->name

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org