[libvirt] [PATCH v2 6/6] qemu: Add ability to abort existing console while creating new one

[Eric Blake]

On 12/07/2011 11:08 AM, Peter Krempa wrote:
> This patch fixes console corruption, that happens if two concurrent
> sessions are opened for a single console on a domain. Result of this
> corruption was, that each of the console streams did recieve just a part

s/was, that/was that/
s/did recieve/received/

> of the data written to the pipe so every console rendered unusable.

s/pipe so every console/pipe, and both sessions were/

> 
> New helper function for safe console handling is used to establis the

s/establis/establish/

> console stream connection. This function ensurest that no other libvirt

s/ensurest/ensures/

> client is using the console (with the ability to disconnect consoles of
> libvirt clients) and that no UUCP style lockfile is placed on the PTY
> device.
> 
> * src/qemu/qemu_domain.h
>         - add data structure to domain's private data dealing with
>           console connections
> * src/qemu/qemu_domain.c:
>         - allocate/free domain's console data structure
> * src/qemu/qemu_driver.c
>         - use the new helper function for console handling
> ---
>  src/qemu/qemu_domain.c |    5 +++++
>  src/qemu/qemu_domain.h |    3 +++
>  src/qemu/qemu_driver.c |   21 ++++++++++++++++-----
>  3 files changed, 24 insertions(+), 5 deletions(-)

> +++ b/src/qemu/qemu_driver.c
> @@ -10847,8 +10847,10 @@ qemuDomainOpenConsole(virDomainPtr dom,
>      int ret = -1;
>      int i;
>      virDomainChrDefPtr chr = NULL;
> +    qemuDomainObjPrivatePtr priv;
> 
> -    virCheckFlags(0, -1);
> +    virCheckFlags(VIR_DOMAIN_CONSOLE_TRY |
> +                  VIR_DOMAIN_CONSOLE_FORCE, -1);
> 

Failed to compile, if you applied my proposed fixups to 1/6
(https://www.redhat.com/archives/libvir-list/2011-December/msg00624.html):

qemu/qemu_driver.c:11284:50: error: 'VIR_DOMAIN_CONSOLE_TRY' undeclared
(first use in this function)

> @@ -10900,11 +10904,18 @@ qemuDomainOpenConsole(virDomainPtr dom,
>          goto cleanup;
>      }
> 
> -    if (virFDStreamOpenFile(st, chr->source.data.file.path,
> -                            0, 0, O_RDWR) < 0)
> -        goto cleanup;
> +    /* handle mutualy exclusive access to console devices */

s/mutualy/mutually/

> +    ret = virDomainSafeConsoleOpen(priv->cons,
> +                                   chr->source.data.file.path,
> +                                   st,
> +                                   (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0);
> +
> +    if (ret == 1) {
> +        qemuReportError(VIR_ERR_OPERATION_FAILED,
> +                        _("Active console session exists for this domain"));
> +        ret = -1;
> +    }
> 
> -    ret = 0;

Looks simple, compared to the bulk of the work in patch 5/6.  Here's
what I would squash in, as part of rebasing the series to latest.

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 0f358df..beffdf5 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -11281,8 +11281,7 @@ qemuDomainOpenConsole(virDomainPtr dom,
     virDomainChrDefPtr chr = NULL;
     qemuDomainObjPrivatePtr priv;

-    virCheckFlags(VIR_DOMAIN_CONSOLE_TRY |
-                  VIR_DOMAIN_CONSOLE_FORCE, -1);
+    virCheckFlags(VIR_DOMAIN_CONSOLE_FORCE, -1);

     qemuDriverLock(driver);
     virUUIDFormat(dom->uuid, uuidstr);

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120117/aa323839/attachment-0001.sig>