[libvirt] [PATCH 00/10] Console coruption with two or more clients series

[Dave Allan]

On Thu, Dec 01, 2011 at 04:35:10PM +0100, Peter Krempa wrote:
> On 10/18/2011 12:34 PM, Daniel P. Berrange wrote:
> >On Wed, Oct 12, 2011 at 03:43:10PM +0200, Peter Krempa wrote:
> >>This series fixes anoying console corruption if two clients try to connect
> >>at same time to the console. The current state of this is, that two/more
> >>of libvirt iohelpers are spawned on the same time that compete for data
> >>from the pty. This causes that each of the consoles get scrambled and
> >>unusable.
> >>
> >>Patches
> >>   fdstream: Emit stream abort callback even if poll() doesnt.
> >>   virnetclientstream: Propagate stream error messages to callback
> >>   daemon: Subscribe the stream event callback for error events.
> >>
> >>add the ability to abort a stream from the daemon side.
> >>
> >>   fdstream: Add internal function to check if a fdstream is open
> >>This patch adds a helper function that checks internally if a fdstream
> >>is open and working.
> >>
> >>   virsh: fix console stream error reporting
> >>   Add flags for virDomainOpenConsole
> >>   virsh: add support for VIR_DOMAIN_CONSOLE_FORCE flag
> >>This patches add instrumentation to virsh that supports the new ability to
> >>abort streams from the daemon side and adapts the console callback to handle
> >>this. These patches also add a new flag set for virDomainOpenConsole API
> >>call that allow users to control the if the existing console connection should
> >>be left in place or killed in favor of the new one
> >>
> >>   qemu: Add ability to abort existing console while creating new one
> >>   lxc: Add ability to abort existing console when creating a new one
> >>   uml: Add ability to abort existing console when creating a new one
> >>These patches modify the hypervisor drivers so that they are aware of existing
> >>console connections and refuse to create a new one (or kill the old).
> >>
> >>The xen driver also supports console in a similar way like the previously
> >>mentioned drivers, but lacks the means to store a stream reference
> >>permanently. I'll look in if it's possible to modify this driver to support
> >>this new functionality.
> >
> >The problem with doing the  console checks by looking for an in use
> >virStreamPtr is that it only solves it for apps using libvirt. If
> >someone connects using 'xm console' or 'minicom' then we're not
> >protected.
> >
> >The traditional way to protect PTYs from concurrent usage is to place
> >a lock file in /var/lock with a special standardized naming scheme.
> >Should we perhaps be doing that instead ?
> >
> >Daniel
> 
> I've prepared an updated version of this series taking into account
> recent changes to console code and Daniel's comments on this
> version.
> 
> I've got some questions regarding implementation of lock files on
> PTY's suggested in this thread I'd like to discuss:
> 
> 1) When using lock files, there's the risk of having them left
> behind when the daemon crashes/is killed. According to the
> filesystem hierarchy standard
> (http://www.pathname.com/fhs/2.2/fhs-5.9.html) lock files should
> contain PID of process holding the lock. This enables to check if
> the process holding the lock still exists. I'm not sure if we want
> to check if the process exists and remove lock files, that were left
> behind, or just warn the user that a lock exists and refuse to open
> console.

That seems like a reasonable approach to me.

> 2) When a non-good-mannered program opens the PTY and does not check
> for lock files or create them, we will still end up with a corrupted
> console. Is there an elegant solution for this?

I don't see any elegant way to protect against ill-mannered programs
ignoring the lock files.

Dave


> I'd appreciate your comments on this topic.
> 
> Thanks,
> Peter
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list