Re: [libvirt] [PATCH v2 6/6] qemu: Add ability to abort existing console while creating new one
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)