[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 12/07/2011 11:08 AM, Peter Krempa wrote:
> This patch adds a set of functions used in creating console streams for
> domains using PTYs and ensures mutualy exculsive access to the PTYs.
> 
> If mutualy exclusive access is not used, two clients may open the same
> console, which results into corruption on both clients as both of them
> race to read data from the PTY.
> 

> +++ b/configure.ac
> @@ -329,6 +329,10 @@ AC_ARG_WITH([remote],
>    AC_HELP_STRING([--with-remote], [add remote driver support @<:@default=yes@:>@]),[],[with_remote=yes])
>  AC_ARG_WITH([libvirtd],
>    AC_HELP_STRING([--with-libvirtd], [add libvirtd support @<:@default=yes@:>@]),[],[with_libvirtd=yes])
> +AC_ARG_WITH([console-lock-files],
> +  AC_HELP_STRING([--with-console-lock-files],
> +                 [location for UUCP style lock files for console PTYs (use auto for default paths on some platforms)@<:@defult=disabled@:>@]),

s/defult/default/

Pre-existing, but AC_HELP_STRING is deprecated per autoconf
documentation; we should clean this up to use AS_HELP_STRING.  Also
pre-existing - this is underquoted - we should be using

AC_ARG_WITH([arg],
  [AS_HELP_STRING([option], [description])])

but those should be a separate cleanup over all instances of this pattern.

Meanwhile, in your patch, you added a rather long line.
A[CS]_HELP_STRING auto-line-wraps its second argument, so you could
write it like:

  [AC_HELP_STRING([--with-console-lock-files],
     [location for UUCP style lock files for console PTYs
      (use auto for default paths on some platforms)
       @<:@defulat=disables@:>@],
...

> +  fi
> +  AC_DEFINE_UNQUOTED([VIR_PTY_LOCK_FILE_PATH], "$with_console_lock_files",
> +                      [path to directory containig UUCP pty lock files ])

s/containg/containing/
s/files ]/files]/

> +AC_MSG_NOTICE([          Init script: $with_init_script])
> +AC_MSG_NOTICE([Console PTY lock path: $with_console_lock_files])

You'd still have to re-indent, but maybe the shorter:

Console PTY locks:

would be nicer.

> +++ b/src/Makefile.am

Aargh - I need to quit starting the review of this at the end of my
day... More tomorrow, I hope.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]