Re: [libvirt] [PATCH v5 6/7] util: Add helpers for safe domain console operations

This patch adds a set of functions used in creating console streams for domains using PTYs and ensures mutually exclusive access to the PTYs. If mutualy exclusive access is not used, two clients may open the same

console, which results in corruption on both clients as both of them race to read data from the PTY.

Diff to v4: - fixed typos (traditionally) - fixed configure.ac to work with automaticaly used values - tweaked configure output line to line up with others without moving them - using STRSKIP to skip the start of the string instead of possibly skipping in the middle of a string - fixed whitespace problems - changed data type for pids to pid_t and tweaked printing formatters - On failure to initialise mutex in virConsoleAlloc don't skip to virConsoleFree - added comment to clarify why it's needed to unregister the callback handler - Added OOM error reporting on some error paths.

Note to v4 review: I changed the default value for the path of the lock files to "auto" so it will get built with "/var/lock" on linux machines, so no change to the spec file is needed.

+dnl UUCP style file locks for PTY consoles +if test "$with_console_lock_files" != "no"; then + if test "$with_console_lock_files" = "yes"; then + AC_MSG_ERROR([console lock files requested, but no path given]) + elif test "$with_console_lock_files" = "auto"; then + dnl Default locations for platforms + if test "$with_linux" = "yes"; then + with_console_lock_files=/var/lock + fi + fi + if test "$with_console_lock_files" = "auto"; then + AC_MSG_ERROR([You must specify path for the lock files on this platform]) + fi

+int virConsoleOpen(virConsolesPtr cons, + const char *pty, + virStreamPtr st, + bool force) +{ + virConsoleStreamInfoPtr cbdata = NULL; + virStreamPtr savedStream; + int ret; + + virMutexLock(&cons->lock); + + if ((savedStream = virHashLookup(cons->hash, pty))) { + if (!force) { + /* entry found, console is busy */ + virMutexUnlock(&cons->lock); + return 1; + } else { + /* terminate existing connection */ + /* The internal close callback handler needs to lock cons->lock to + * remove the aborted stream from the hash. This would cause a + * deadlock as we would try to enter the lock twice from the very + * same thread. We need to unregister the callback and abort the + * stream manualy before we create a new console connection.