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

Re: [libvirt] [PATCH v2 2/6] virsh: add support for VIR_DOMAIN_CONSOLE_FORCE flag



On 12/07/2011 11:08 AM, Peter Krempa wrote:
> This patch adds support for the newly introduced VIR_DOMAIN_CONSOLE

s/$/_FORCE/

> flag. The console command now has an optional parameter --force that
> specifies that the user wants to forcibly interrupt an ongoing console
> session and create a new one. The behaviour to this point was, that the

s/,//

> daemon openend two streams to the console, that competed for data from

s/openend/opened/

> the pipe and the result was, that both of the consoles ended up

s/pipe and the result was,/pipe, and the result was/

> scrambled.
> 
> * tools/console.c:
>         - add support for flag passthrough
> * tools/console.h:
>         - modify function prototypes to match impl.
> * tools/virsh.c:
>         - add flag --force for the console command
> ---
>  tools/console.c |    5 +++--
>  tools/console.h |    3 ++-
>  tools/virsh.c   |   18 +++++++++++++-----
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 

> +++ b/tools/virsh.c
> @@ -787,11 +787,14 @@ static const vshCmdInfo info_console[] = {
>  static const vshCmdOptDef opts_console[] = {
>      {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>      {"devname", VSH_OT_STRING, 0, N_("character device name")},
> +    {"force", VSH_OT_BOOL, 0, N_("force console connection (disconnect already connected sessions)")},

Might want to wrap this to fit 80 columns.

> @@ -834,7 +839,10 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
>          goto cleanup;
>      }
> 
> -    ret = cmdRunConsole(ctl, dom, name);
> +    if (force)
> +        flags = VIR_DOMAIN_CONSOLE_FORCE;

Okay as-is, but if we add more flags in the future, this is not
maintainer-friendly.  I'd use |= instead of = so this line won't need
future edits.

ACK.

Hmm, wondering about back-compat - what happens if we are talking to an
older libvirt that does not know VIR_DOMAIN_CONSOLE_FORCE, but also
which does not prevent multiple connections when flags==0?  At first
glance, it looks like we're stuck - there's no useful way to tell if a
console was already in use.

But maybe we could add a new API, to allow probing whether a console is
in use.  If the API exists, then we know whether the console is in use
(and we also know whether --force will work); if the API does not exist,
the we know we are talking to an older server that multiplexes
connections.  Maybe we should have:

virsh console domain --safe => refuse to connect unless connection is
safe (fails on older servers)
virsh console domain --force => force connection (fails on older servers)
virsh console domain => existing behavior (connects on older servers,
even if it corrupts connection, and safely connects on newer servers)

that is, I'm thinking it might be worth it to add a new --safe option
that lets the user probe (via a new API virDomainConsoleInUse(dom,
dev_name, flags)) whether the attempt will be safe.

-- 
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]