[libvirt] [PATCH 0/5] Add API to tunnel channels

This series enables the qemu driver to tunnel a virtio channel. This is useful for a remote session to communicate with a guest channel via the streaming API. This was originally fleshed out a while back in this thread: https://www.redhat.com/archives/libvir-list/2011-September/msg01049.html This implements only item (3) in that list. The new API is nearly identical to the existing virDomainOpenConsole API, except it works on channels, and supports UNIX sockets in addition to PTYs for channel source type. This is my first libvirt patch, please be gentle :) John Eckersberg (5): api: Add API to tunnel a guest channel via stream conf: Rename virconsole.* to virchrdev.* conf: Rename console-specific identifiers to be more generic conf: Add unix socket support to virChrdevOpen qemu: Implement virDomainOpenChannel API configure.ac | 48 ++--- include/libvirt/libvirt.h.in | 16 ++ po/POTFILES.in | 2 +- src/Makefile.am | 8 +- src/conf/virchrdev.c | 442 +++++++++++++++++++++++++++++++++++++++++++ src/conf/virchrdev.h | 37 ++++ src/conf/virconsole.c | 414 ---------------------------------------- src/conf/virconsole.h | 36 ---- src/driver.h | 7 + src/libvirt.c | 61 ++++++ src/libvirt_private.syms | 8 +- src/libvirt_public.syms | 5 + src/qemu/qemu_domain.c | 4 +- src/qemu/qemu_domain.h | 4 +- src/qemu/qemu_driver.c | 81 +++++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +- src/remote_protocol-structs | 6 + 18 files changed, 697 insertions(+), 492 deletions(-) create mode 100644 src/conf/virchrdev.c create mode 100644 src/conf/virchrdev.h delete mode 100644 src/conf/virconsole.c delete mode 100644 src/conf/virconsole.h -- 1.7.11.7

This patch adds a new API, virDomainOpenChannel, that uses streams to connect to a virtio channel on a guest. This creates a secure communication channel between a guest and a libvirt client. This behaves the same as virDomainOpenConsole, except on channels instead of console/serial/parallel devices. --- include/libvirt/libvirt.h.in | 16 ++++++++++++ src/driver.h | 7 +++++ src/libvirt.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 ++++++- src/remote_protocol-structs | 6 +++++ 7 files changed, 104 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c6739d7..cc7ebb9 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4548,6 +4548,22 @@ int virDomainOpenConsole(virDomainPtr dom, virStreamPtr st, unsigned int flags); +/** + * virDomainChannelFlags + * + * Since 1.0.2 + */ +typedef enum { + VIR_DOMAIN_CHANNEL_FORCE = (1 << 0), /* abort a (possibly) active channel + connection to force a new + connection */ +} virDomainChannelFlags; + +int virDomainOpenChannel(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags); + typedef enum { VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH = (1 << 0), } virDomainOpenGraphicsFlags; diff --git a/src/driver.h b/src/driver.h index 64d652f..01c95cf 100644 --- a/src/driver.h +++ b/src/driver.h @@ -717,6 +717,12 @@ typedef int virStreamPtr st, unsigned int flags); typedef int + (*virDrvDomainOpenChannel)(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags); + +typedef int (*virDrvDomainOpenGraphics)(virDomainPtr dom, unsigned int idx, int fd, @@ -1078,6 +1084,7 @@ struct _virDriver { virDrvDomainQemuAttach qemuDomainAttach; virDrvDomainQemuAgentCommand qemuDomainArbitraryAgentCommand; virDrvDomainOpenConsole domainOpenConsole; + virDrvDomainOpenChannel domainOpenChannel; virDrvDomainOpenGraphics domainOpenGraphics; virDrvDomainInjectNMI domainInjectNMI; virDrvDomainMigrateBegin3 domainMigrateBegin3; diff --git a/src/libvirt.c b/src/libvirt.c index 4215971..b656173 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19118,6 +19118,67 @@ error: } /** + * virDomainOpenChannel: + * @dom: a domain object + * @name: the channel name, or NULL + * @st: a stream to associate with the channel + * @flags: bitwise-OR of virDomainChannelFlags + * + * This opens the host interface associated with a channel device on a + * guest, if the host interface is supported. If @name is given, it + * can match either the device alias (e.g. "channel0"), or the virtio + * target name (e.g. "org.qemu.guest_agent.0"). If @name is omitted, + * then the first channel is opened. The channel is associated with + * the passed in @st stream, which should have been opened in + * non-blocking mode for bi-directional I/O. + * + * By default, when @flags is 0, the open will fail if libvirt detects + * that the channel is already in use by another client; passing + * VIR_DOMAIN_CHANNEL_FORCE will cause libvirt to forcefully remove the + * other client prior to opening this channel. + * + * Returns 0 if the channel was opened, -1 on error + */ +int virDomainOpenChannel(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "name=%s, st=%p, flags=%x", + NULLSTR(name), st, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = dom->conn; + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->domainOpenChannel) { + int ret; + ret = conn->driver->domainOpenChannel(dom, name, st, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** * virDomainBlockJobAbort: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index e3d63d3..2107519 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -580,4 +580,9 @@ LIBVIRT_1.0.1 { virDomainSendProcessSignal; } LIBVIRT_1.0.0; +LIBVIRT_1.0.2 { + global: + virDomainOpenChannel; +} LIBVIRT_1.0.1; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5cc7e32..da1f755 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6121,6 +6121,7 @@ static virDriver remote_driver = { .qemuDomainAttach = qemuDomainAttach, /* 0.9.4 */ .qemuDomainArbitraryAgentCommand = qemuDomainAgentCommand, /* 0.10.0 */ .domainOpenConsole = remoteDomainOpenConsole, /* 0.8.6 */ + .domainOpenChannel = remoteDomainOpenChannel, /* 1.0.2 */ .domainOpenGraphics = remoteDomainOpenGraphics, /* 0.9.7 */ .domainInjectNMI = remoteDomainInjectNMI, /* 0.9.2 */ .domainMigrateBegin3 = remoteDomainMigrateBegin3, /* 0.9.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index bdad9f0..9035776 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2439,6 +2439,12 @@ struct remote_domain_open_console_args { unsigned int flags; }; +struct remote_domain_open_channel_args { + remote_nonnull_domain dom; + remote_string name; + unsigned int flags; +}; + struct remote_storage_vol_upload_args { remote_nonnull_storage_vol vol; unsigned hyper offset; @@ -3042,7 +3048,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, /* autogen autogen */ REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_FSTRIM = 294, /* autogen autogen */ - REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295 /* autogen autogen */ + REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295, /* autogen autogen */ + REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296 /* autogen autogen | readstream@2 */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e7d05b8..91414d4 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1873,6 +1873,11 @@ struct remote_domain_open_console_args { remote_string dev_name; u_int flags; }; +struct remote_domain_open_channel_args { + remote_nonnull_domain dom; + remote_string name; + u_int flags; +}; struct remote_storage_vol_upload_args { remote_nonnull_storage_vol vol; uint64_t offset; @@ -2447,4 +2452,5 @@ enum remote_procedure { REMOTE_PROC_NODE_GET_CPU_MAP = 293, REMOTE_PROC_DOMAIN_FSTRIM = 294, REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295, + REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296, }; -- 1.7.11.7

On Thu, Dec 13, 2012 at 11:24:16AM -0500, John Eckersberg wrote:
This patch adds a new API, virDomainOpenChannel, that uses streams to connect to a virtio channel on a guest. This creates a secure communication channel between a guest and a libvirt client.
This behaves the same as virDomainOpenConsole, except on channels instead of console/serial/parallel devices. --- include/libvirt/libvirt.h.in | 16 ++++++++++++ src/driver.h | 7 +++++ src/libvirt.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 ++++++- src/remote_protocol-structs | 6 +++++ 7 files changed, 104 insertions(+), 1 deletion(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This is just code motion, in preparation to rename identifiers to be less console-specific. --- po/POTFILES.in | 2 +- src/Makefile.am | 2 +- src/conf/virchrdev.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/virchrdev.h | 36 +++++ src/conf/virconsole.c | 414 ----------------------------------------------- src/conf/virconsole.h | 36 ----- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.h | 2 +- 8 files changed, 454 insertions(+), 454 deletions(-) create mode 100644 src/conf/virchrdev.c create mode 100644 src/conf/virchrdev.h delete mode 100644 src/conf/virconsole.c delete mode 100644 src/conf/virconsole.h diff --git a/po/POTFILES.in b/po/POTFILES.in index f0cfd7f..af2edae 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -22,7 +22,7 @@ src/conf/secret_conf.c src/conf/snapshot_conf.c src/conf/storage_conf.c src/conf/storage_encryption_conf.c -src/conf/virconsole.c +src/conf/virchrdev.c src/cpu/cpu.c src/cpu/cpu_generic.c src/cpu/cpu_map.c diff --git a/src/Makefile.am b/src/Makefile.am index bb80992..72184b4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -237,7 +237,7 @@ CPU_CONF_SOURCES = \ # Safe console handling helper APIs CONSOLE_CONF_SOURCES = \ - conf/virconsole.c conf/virconsole.h + conf/virchrdev.c conf/virchrdev.h # Device Helper APIs DEVICE_CONF_SOURCES = \ diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c new file mode 100644 index 0000000..666432e --- /dev/null +++ b/src/conf/virchrdev.c @@ -0,0 +1,414 @@ +/** + * virchrdev.c: api to guarantee mutually exclusive + * access to domain's consoles + * + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Peter Krempa <pkrempa@redhat.com> + */ + +#include <config.h> + +#include <fcntl.h> +#include <unistd.h> +#include <sys/types.h> + +#include "virchrdev.h" +#include "virhash.h" +#include "fdstream.h" +#include "internal.h" +#include "threads.h" +#include "memory.h" +#include "virpidfile.h" +#include "logging.h" +#include "virterror_internal.h" +#include "virfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/* structure holding information about consoles + * open in a given domain */ +struct _virConsoles { + virMutex lock; + virHashTablePtr hash; +}; + +typedef struct _virConsoleStreamInfo virConsoleStreamInfo; +typedef virConsoleStreamInfo *virConsoleStreamInfoPtr; +struct _virConsoleStreamInfo { + virConsolesPtr cons; + const char *pty; +}; + +#ifdef VIR_PTY_LOCK_FILE_PATH +/** + * Create a full filename with path to the lock file based on + * name/path of corresponding pty + * + * @pty path of the console device + * + * Returns a modified name that the caller has to free, or NULL + * on error. + */ +static char *virConsoleLockFilePath(const char *pty) +{ + char *path = NULL; + char *sanitizedPath = NULL; + char *ptyCopy; + char *filename; + char *p; + + if (!(ptyCopy = strdup(pty))) { + virReportOOMError(); + goto cleanup; + } + + /* skip the leading "/dev/" */ + filename = STRSKIP(ptyCopy, "/dev"); + if (!filename) + filename = ptyCopy; + + /* substitute path forward slashes for underscores */ + p = filename; + while (*p) { + if (*p == '/') + *p = '_'; + ++p; + } + + if (virAsprintf(&path, "%s/LCK..%s", VIR_PTY_LOCK_FILE_PATH, filename) < 0) + goto cleanup; + + sanitizedPath = virFileSanitizePath(path); + +cleanup: + VIR_FREE(path); + VIR_FREE(ptyCopy); + + return sanitizedPath; +} + +/** + * Verify and create a lock file for a console pty + * + * @pty Path of the console device + * + * Returns 0 on success, -1 on error + */ +static int virConsoleLockFileCreate(const char *pty) +{ + char *path = NULL; + int ret = -1; + int lockfd = -1; + char *pidStr = NULL; + pid_t pid; + + /* build lock file path */ + if (!(path = virConsoleLockFilePath(pty))) + goto cleanup; + + /* check if a log file and process holding the lock still exists */ + if (virPidFileReadPathIfAlive(path, &pid, NULL) == 0 && pid >= 0) { + /* the process exists, the lockfile is valid */ + virReportError(VIR_ERR_OPERATION_FAILED, + _("Requested console pty '%s' is locked by " + "lock file '%s' held by process %lld"), + pty, path, (long long) pid); + goto cleanup; + } else { + /* clean up the stale/corrupted/nonexistent lockfile */ + unlink(path); + } + /* lockfile doesn't (shouldn't) exist */ + + /* ensure correct format according to filesystem hierarchy standard */ + /* http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES */ + if (virAsprintf(&pidStr, "%10lld\n", (long long) getpid()) < 0) + goto cleanup; + + /* create the lock file */ + if ((lockfd = open(path, O_WRONLY | O_CREAT | O_EXCL, 00644)) < 0) { + /* If we run in session mode, we might have no access to the lock + * file directory. We have to check for an permission denied error + * and see if we can reach it. This should cause an error only if + * we run in daemon mode and thus privileged. + */ + if (errno == EACCES && geteuid() != 0) { + VIR_DEBUG("Skipping lock file creation for pty '%s in path '%s'.", + pty, path); + ret = 0; + goto cleanup; + } + virReportSystemError(errno, + _("Couldn't create lock file for " + "pty '%s' in path '%s'"), + pty, path); + goto cleanup; + } + + /* write the pid to the file */ + if (safewrite(lockfd, pidStr, strlen(pidStr)) < 0) { + virReportSystemError(errno, + _("Couldn't write to lock file for " + "pty '%s' in path '%s'"), + pty, path); + VIR_FORCE_CLOSE(lockfd); + unlink(path); + goto cleanup; + } + + /* we hold the lock */ + ret = 0; + +cleanup: + VIR_FORCE_CLOSE(lockfd); + VIR_FREE(path); + VIR_FREE(pidStr); + + return ret; +} + +/** + * Remove a lock file for a pty + * + * @pty Path of the pty device + */ +static void virConsoleLockFileRemove(const char *pty) +{ + char *path = virConsoleLockFilePath(pty); + if (path) + unlink(path); + VIR_FREE(path); +} +#else /* #ifdef VIR_PTY_LOCK_FILE_PATH */ +/* file locking for console devices is disabled */ +static int virConsoleLockFileCreate(const char *pty ATTRIBUTE_UNUSED) +{ + return 0; +} + +static void virConsoleLockFileRemove(const char *pty ATTRIBUTE_UNUSED) +{ + return; +} +#endif /* #ifdef VIR_PTY_LOCK_FILE_PATH */ + +/** + * Frees an entry from the hash containing domain's active consoles + * + * @data Opaque data, struct holding information about the console + * @name Path of the pty. + */ +static void virConsoleHashEntryFree(void *data, + const void *name) +{ + const char *pty = name; + virStreamPtr st = data; + + /* free stream reference */ + virStreamFree(st); + + /* delete lock file */ + virConsoleLockFileRemove(pty); +} + +/** + * Frees opaque data provided for the stream closing callback + * + * @opaque Data to be freed. + */ +static void virConsoleFDStreamCloseCbFree(void *opaque) +{ + virConsoleStreamInfoPtr priv = opaque; + + VIR_FREE(priv->pty); + VIR_FREE(priv); +} + +/** + * Callback being called if a FDstream is closed. Frees console entries + * from data structures and removes lockfiles. + * + * @st Pointer to stream being closed. + * @opaque Domain's console information structure. + */ +static void virConsoleFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, + void *opaque) +{ + virConsoleStreamInfoPtr priv = opaque; + virMutexLock(&priv->cons->lock); + + /* remove entry from hash */ + virHashRemoveEntry(priv->cons->hash, priv->pty); + + virMutexUnlock(&priv->cons->lock); +} + +/** + * Allocate structures for storing information about active console streams + * in domain's private data section. + * + * Returns pointer to the allocated structure or NULL on error + */ +virConsolesPtr virConsoleAlloc(void) +{ + virConsolesPtr cons; + if (VIR_ALLOC(cons) < 0) + return NULL; + + if (virMutexInit(&cons->lock) < 0) { + VIR_FREE(cons); + return NULL; + } + + /* there will hardly be any consoles most of the time, the hash + * does not have to be huge */ + if (!(cons->hash = virHashCreate(3, virConsoleHashEntryFree))) + goto error; + + return cons; +error: + virConsoleFree(cons); + return NULL; +} + +/** + * Helper to clear stream callbacks when freeing the hash + */ +static void virConsoleFreeClearCallbacks(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + virStreamPtr st = payload; + + virFDStreamSetInternalCloseCb(st, NULL, NULL, NULL); +} + +/** + * Free structures for handling open console streams. + * + * @cons Pointer to the private structure. + */ +void virConsoleFree(virConsolesPtr cons) +{ + if (!cons || !cons->hash) + return; + + virMutexLock(&cons->lock); + virHashForEach(cons->hash, virConsoleFreeClearCallbacks, NULL); + virHashFree(cons->hash); + virMutexUnlock(&cons->lock); + virMutexDestroy(&cons->lock); + + VIR_FREE(cons); +} + +/** + * Open a console stream for a domain ensuring that other streams are + * not using the console, nor any lockfiles exist. This ensures that + * the console stream does not get corrupted due to a race on reading + * same FD by two processes. + * + * @cons Pointer to private structure holding data about console streams. + * @pty Path to the pseudo tty to be opened. + * @st Stream the client wishes to use for the console connection. + * @force On true, close active console streams for the selected console pty + * before opening this connection. + * + * Returns 0 on success and st is connected to the selected pty and + * corresponding lock file is created (if configured). Returns -1 on + * error and 1 if the console stream is open and busy. + */ +int virConsoleOpen(virConsolesPtr cons, + const char *pty, + virStreamPtr st, + bool force) +{ + virConsoleStreamInfoPtr cbdata = NULL; + virStreamPtr savedStream; + int ret; + + virMutexLock(&cons->lock); + + if ((savedStream = virHashLookup(cons->hash, pty))) { + if (!force) { + /* entry found, console is busy */ + virMutexUnlock(&cons->lock); + return 1; + } else { + /* terminate existing connection */ + /* The internal close callback handler needs to lock cons->lock to + * remove the aborted stream from the hash. This would cause a + * deadlock as we would try to enter the lock twice from the very + * same thread. We need to unregister the callback and abort the + * stream manually before we create a new console connection. + */ + virFDStreamSetInternalCloseCb(savedStream, NULL, NULL, NULL); + virStreamAbort(savedStream); + virHashRemoveEntry(cons->hash, pty); + /* continue adding a new stream connection */ + } + } + + /* create the lock file */ + if ((ret = virConsoleLockFileCreate(pty)) < 0) { + virMutexUnlock(&cons->lock); + return ret; + } + + /* obtain a reference to the stream */ + if (virStreamRef(st) < 0) { + virMutexUnlock(&cons->lock); + return -1; + } + + if (VIR_ALLOC(cbdata) < 0) { + virReportOOMError(); + goto error; + } + + if (virHashAddEntry(cons->hash, pty, st) < 0) + goto error; + + cbdata->cons = cons; + if (!(cbdata->pty = strdup(pty))) { + virReportOOMError(); + goto error; + } + + /* open the console pty */ + if (virFDStreamOpenFile(st, pty, 0, 0, O_RDWR) < 0) + goto error; + + /* add cleanup callback */ + virFDStreamSetInternalCloseCb(st, + virConsoleFDStreamCloseCb, + cbdata, + virConsoleFDStreamCloseCbFree); + + virMutexUnlock(&cons->lock); + return 0; + +error: + virStreamFree(st); + virHashRemoveEntry(cons->hash, pty); + if (cbdata) + VIR_FREE(cbdata->pty); + VIR_FREE(cbdata); + virMutexUnlock(&cons->lock); + return -1; +} diff --git a/src/conf/virchrdev.h b/src/conf/virchrdev.h new file mode 100644 index 0000000..d5a926a --- /dev/null +++ b/src/conf/virchrdev.h @@ -0,0 +1,36 @@ +/** + * virchrdev.h: api to guarantee mutually exclusive + * access to domain's consoles + * + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Peter Krempa <pkrempa@redhat.com> + */ +#ifndef __VIR_CONSOLE_H__ +# define __VIR_CONSOLE_H__ + +# include "internal.h" + +typedef struct _virConsoles virConsoles; +typedef virConsoles *virConsolesPtr; + +virConsolesPtr virConsoleAlloc(void); +void virConsoleFree(virConsolesPtr cons); + +int virConsoleOpen(virConsolesPtr cons, const char *pty, + virStreamPtr st, bool force); +#endif /*__VIR_DOMAIN_CONSOLE_LOCK_H__*/ diff --git a/src/conf/virconsole.c b/src/conf/virconsole.c deleted file mode 100644 index 143c1a6..0000000 --- a/src/conf/virconsole.c +++ /dev/null @@ -1,414 +0,0 @@ -/** - * virconsole.c: api to guarantee mutually exclusive - * access to domain's consoles - * - * Copyright (C) 2011-2012 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see - * <http://www.gnu.org/licenses/>. - * - * Author: Peter Krempa <pkrempa@redhat.com> - */ - -#include <config.h> - -#include <fcntl.h> -#include <unistd.h> -#include <sys/types.h> - -#include "virconsole.h" -#include "virhash.h" -#include "fdstream.h" -#include "internal.h" -#include "threads.h" -#include "memory.h" -#include "virpidfile.h" -#include "logging.h" -#include "virterror_internal.h" -#include "virfile.h" - -#define VIR_FROM_THIS VIR_FROM_NONE - -/* structure holding information about consoles - * open in a given domain */ -struct _virConsoles { - virMutex lock; - virHashTablePtr hash; -}; - -typedef struct _virConsoleStreamInfo virConsoleStreamInfo; -typedef virConsoleStreamInfo *virConsoleStreamInfoPtr; -struct _virConsoleStreamInfo { - virConsolesPtr cons; - const char *pty; -}; - -#ifdef VIR_PTY_LOCK_FILE_PATH -/** - * Create a full filename with path to the lock file based on - * name/path of corresponding pty - * - * @pty path of the console device - * - * Returns a modified name that the caller has to free, or NULL - * on error. - */ -static char *virConsoleLockFilePath(const char *pty) -{ - char *path = NULL; - char *sanitizedPath = NULL; - char *ptyCopy; - char *filename; - char *p; - - if (!(ptyCopy = strdup(pty))) { - virReportOOMError(); - goto cleanup; - } - - /* skip the leading "/dev/" */ - filename = STRSKIP(ptyCopy, "/dev"); - if (!filename) - filename = ptyCopy; - - /* substitute path forward slashes for underscores */ - p = filename; - while (*p) { - if (*p == '/') - *p = '_'; - ++p; - } - - if (virAsprintf(&path, "%s/LCK..%s", VIR_PTY_LOCK_FILE_PATH, filename) < 0) - goto cleanup; - - sanitizedPath = virFileSanitizePath(path); - -cleanup: - VIR_FREE(path); - VIR_FREE(ptyCopy); - - return sanitizedPath; -} - -/** - * Verify and create a lock file for a console pty - * - * @pty Path of the console device - * - * Returns 0 on success, -1 on error - */ -static int virConsoleLockFileCreate(const char *pty) -{ - char *path = NULL; - int ret = -1; - int lockfd = -1; - char *pidStr = NULL; - pid_t pid; - - /* build lock file path */ - if (!(path = virConsoleLockFilePath(pty))) - goto cleanup; - - /* check if a log file and process holding the lock still exists */ - if (virPidFileReadPathIfAlive(path, &pid, NULL) == 0 && pid >= 0) { - /* the process exists, the lockfile is valid */ - virReportError(VIR_ERR_OPERATION_FAILED, - _("Requested console pty '%s' is locked by " - "lock file '%s' held by process %lld"), - pty, path, (long long) pid); - goto cleanup; - } else { - /* clean up the stale/corrupted/nonexistent lockfile */ - unlink(path); - } - /* lockfile doesn't (shouldn't) exist */ - - /* ensure correct format according to filesystem hierarchy standard */ - /* http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES */ - if (virAsprintf(&pidStr, "%10lld\n", (long long) getpid()) < 0) - goto cleanup; - - /* create the lock file */ - if ((lockfd = open(path, O_WRONLY | O_CREAT | O_EXCL, 00644)) < 0) { - /* If we run in session mode, we might have no access to the lock - * file directory. We have to check for an permission denied error - * and see if we can reach it. This should cause an error only if - * we run in daemon mode and thus privileged. - */ - if (errno == EACCES && geteuid() != 0) { - VIR_DEBUG("Skipping lock file creation for pty '%s in path '%s'.", - pty, path); - ret = 0; - goto cleanup; - } - virReportSystemError(errno, - _("Couldn't create lock file for " - "pty '%s' in path '%s'"), - pty, path); - goto cleanup; - } - - /* write the pid to the file */ - if (safewrite(lockfd, pidStr, strlen(pidStr)) < 0) { - virReportSystemError(errno, - _("Couldn't write to lock file for " - "pty '%s' in path '%s'"), - pty, path); - VIR_FORCE_CLOSE(lockfd); - unlink(path); - goto cleanup; - } - - /* we hold the lock */ - ret = 0; - -cleanup: - VIR_FORCE_CLOSE(lockfd); - VIR_FREE(path); - VIR_FREE(pidStr); - - return ret; -} - -/** - * Remove a lock file for a pty - * - * @pty Path of the pty device - */ -static void virConsoleLockFileRemove(const char *pty) -{ - char *path = virConsoleLockFilePath(pty); - if (path) - unlink(path); - VIR_FREE(path); -} -#else /* #ifdef VIR_PTY_LOCK_FILE_PATH */ -/* file locking for console devices is disabled */ -static int virConsoleLockFileCreate(const char *pty ATTRIBUTE_UNUSED) -{ - return 0; -} - -static void virConsoleLockFileRemove(const char *pty ATTRIBUTE_UNUSED) -{ - return; -} -#endif /* #ifdef VIR_PTY_LOCK_FILE_PATH */ - -/** - * Frees an entry from the hash containing domain's active consoles - * - * @data Opaque data, struct holding information about the console - * @name Path of the pty. - */ -static void virConsoleHashEntryFree(void *data, - const void *name) -{ - const char *pty = name; - virStreamPtr st = data; - - /* free stream reference */ - virStreamFree(st); - - /* delete lock file */ - virConsoleLockFileRemove(pty); -} - -/** - * Frees opaque data provided for the stream closing callback - * - * @opaque Data to be freed. - */ -static void virConsoleFDStreamCloseCbFree(void *opaque) -{ - virConsoleStreamInfoPtr priv = opaque; - - VIR_FREE(priv->pty); - VIR_FREE(priv); -} - -/** - * Callback being called if a FDstream is closed. Frees console entries - * from data structures and removes lockfiles. - * - * @st Pointer to stream being closed. - * @opaque Domain's console information structure. - */ -static void virConsoleFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, - void *opaque) -{ - virConsoleStreamInfoPtr priv = opaque; - virMutexLock(&priv->cons->lock); - - /* remove entry from hash */ - virHashRemoveEntry(priv->cons->hash, priv->pty); - - virMutexUnlock(&priv->cons->lock); -} - -/** - * Allocate structures for storing information about active console streams - * in domain's private data section. - * - * Returns pointer to the allocated structure or NULL on error - */ -virConsolesPtr virConsoleAlloc(void) -{ - virConsolesPtr cons; - if (VIR_ALLOC(cons) < 0) - return NULL; - - if (virMutexInit(&cons->lock) < 0) { - VIR_FREE(cons); - return NULL; - } - - /* there will hardly be any consoles most of the time, the hash - * does not have to be huge */ - if (!(cons->hash = virHashCreate(3, virConsoleHashEntryFree))) - goto error; - - return cons; -error: - virConsoleFree(cons); - return NULL; -} - -/** - * Helper to clear stream callbacks when freeing the hash - */ -static void virConsoleFreeClearCallbacks(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED) -{ - virStreamPtr st = payload; - - virFDStreamSetInternalCloseCb(st, NULL, NULL, NULL); -} - -/** - * Free structures for handling open console streams. - * - * @cons Pointer to the private structure. - */ -void virConsoleFree(virConsolesPtr cons) -{ - if (!cons || !cons->hash) - return; - - virMutexLock(&cons->lock); - virHashForEach(cons->hash, virConsoleFreeClearCallbacks, NULL); - virHashFree(cons->hash); - virMutexUnlock(&cons->lock); - virMutexDestroy(&cons->lock); - - VIR_FREE(cons); -} - -/** - * Open a console stream for a domain ensuring that other streams are - * not using the console, nor any lockfiles exist. This ensures that - * the console stream does not get corrupted due to a race on reading - * same FD by two processes. - * - * @cons Pointer to private structure holding data about console streams. - * @pty Path to the pseudo tty to be opened. - * @st Stream the client wishes to use for the console connection. - * @force On true, close active console streams for the selected console pty - * before opening this connection. - * - * Returns 0 on success and st is connected to the selected pty and - * corresponding lock file is created (if configured). Returns -1 on - * error and 1 if the console stream is open and busy. - */ -int virConsoleOpen(virConsolesPtr cons, - const char *pty, - virStreamPtr st, - bool force) -{ - virConsoleStreamInfoPtr cbdata = NULL; - virStreamPtr savedStream; - int ret; - - virMutexLock(&cons->lock); - - if ((savedStream = virHashLookup(cons->hash, pty))) { - if (!force) { - /* entry found, console is busy */ - virMutexUnlock(&cons->lock); - return 1; - } else { - /* terminate existing connection */ - /* The internal close callback handler needs to lock cons->lock to - * remove the aborted stream from the hash. This would cause a - * deadlock as we would try to enter the lock twice from the very - * same thread. We need to unregister the callback and abort the - * stream manually before we create a new console connection. - */ - virFDStreamSetInternalCloseCb(savedStream, NULL, NULL, NULL); - virStreamAbort(savedStream); - virHashRemoveEntry(cons->hash, pty); - /* continue adding a new stream connection */ - } - } - - /* create the lock file */ - if ((ret = virConsoleLockFileCreate(pty)) < 0) { - virMutexUnlock(&cons->lock); - return ret; - } - - /* obtain a reference to the stream */ - if (virStreamRef(st) < 0) { - virMutexUnlock(&cons->lock); - return -1; - } - - if (VIR_ALLOC(cbdata) < 0) { - virReportOOMError(); - goto error; - } - - if (virHashAddEntry(cons->hash, pty, st) < 0) - goto error; - - cbdata->cons = cons; - if (!(cbdata->pty = strdup(pty))) { - virReportOOMError(); - goto error; - } - - /* open the console pty */ - if (virFDStreamOpenFile(st, pty, 0, 0, O_RDWR) < 0) - goto error; - - /* add cleanup callback */ - virFDStreamSetInternalCloseCb(st, - virConsoleFDStreamCloseCb, - cbdata, - virConsoleFDStreamCloseCbFree); - - virMutexUnlock(&cons->lock); - return 0; - -error: - virStreamFree(st); - virHashRemoveEntry(cons->hash, pty); - if (cbdata) - VIR_FREE(cbdata->pty); - VIR_FREE(cbdata); - virMutexUnlock(&cons->lock); - return -1; -} diff --git a/src/conf/virconsole.h b/src/conf/virconsole.h deleted file mode 100644 index df9dfe8..0000000 --- a/src/conf/virconsole.h +++ /dev/null @@ -1,36 +0,0 @@ -/** - * virconsole.h: api to guarantee mutually exclusive - * access to domain's consoles - * - * Copyright (C) 2011-2012 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see - * <http://www.gnu.org/licenses/>. - * - * Author: Peter Krempa <pkrempa@redhat.com> - */ -#ifndef __VIR_CONSOLE_H__ -# define __VIR_CONSOLE_H__ - -# include "internal.h" - -typedef struct _virConsoles virConsoles; -typedef virConsoles *virConsolesPtr; - -virConsolesPtr virConsoleAlloc(void); -void virConsoleFree(virConsolesPtr cons); - -int virConsoleOpen(virConsolesPtr cons, const char *pty, - virStreamPtr st, bool force); -#endif /*__VIR_DOMAIN_CONSOLE_LOCK_H__*/ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9288ad3..396c2f9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1350,7 +1350,7 @@ virAuditOpen; virAuditSend; -# virconsole.h +# virchrdev.h virConsoleAlloc; virConsoleFree; virConsoleOpen; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 11670b9..3a51874 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -31,7 +31,7 @@ # include "qemu_agent.h" # include "qemu_conf.h" # include "qemu_capabilities.h" -# include "virconsole.h" +# include "virchrdev.h" # define QEMU_EXPECTED_VIRT_TYPES \ ((1 << VIR_DOMAIN_VIRT_QEMU) | \ -- 1.7.11.7

On Thu, Dec 13, 2012 at 11:24:17AM -0500, John Eckersberg wrote:
This is just code motion, in preparation to rename identifiers to be less console-specific. --- po/POTFILES.in | 2 +- src/Makefile.am | 2 +- src/conf/virchrdev.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/virchrdev.h | 36 +++++ src/conf/virconsole.c | 414 ----------------------------------------------- src/conf/virconsole.h | 36 ----- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.h | 2 +- 8 files changed, 454 insertions(+), 454 deletions(-) create mode 100644 src/conf/virchrdev.c create mode 100644 src/conf/virchrdev.h delete mode 100644 src/conf/virconsole.c delete mode 100644 src/conf/virconsole.h
ACK, with rename detection enabled in GIT, the changes are clearly a no-op Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

The functionality provided in virchrdev.c (previously virconsole.c) is applicable to other types of character devices besides consoles, such as channels. This patch is just code motion, renaming things such as "console" or "pty", instead using more general terms such as "character device" or "device path". --- configure.ac | 48 +++++------ src/Makefile.am | 6 +- src/conf/virchrdev.c | 220 +++++++++++++++++++++++------------------------ src/conf/virchrdev.h | 18 ++-- src/libvirt_private.syms | 6 +- src/qemu/qemu_domain.c | 4 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 8 +- 8 files changed, 156 insertions(+), 156 deletions(-) diff --git a/configure.ac b/configure.ac index bf32f95..36f26af 100644 --- a/configure.ac +++ b/configure.ac @@ -425,12 +425,12 @@ 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 +AC_ARG_WITH([chrdev-lock-files], + AC_HELP_STRING([--with-chrdev-lock-files], + [location for UUCP style lock files for character devices (use auto for default paths on some platforms) @<:@default=auto@:>@]), - [],[with_console_lock_files=auto]) + [],[with_chrdev_lock_files=auto]) AC_ARG_WITH([libssh2_transport], AC_HELP_STRING([--with-libssh2_transport], [libssh2 location @<:@default=check@:>@]),[],[with_libssh2_transport=check]) @@ -1423,25 +1423,25 @@ AM_CONDITIONAL([HAVE_AUDIT], [test "$with_audit" = "yes"]) AC_SUBST([AUDIT_CFLAGS]) AC_SUBST([AUDIT_LIBS]) -dnl UUCP style file locks for PTY consoles -if test "$with_console_lock_files" != "no"; then - case $with_console_lock_files in +dnl UUCP style file locks for character devices +if test "$with_chrdev_lock_files" != "no"; then + case $with_chrdev_lock_files in yes | auto) dnl Default locations for platforms, or disable if unknown if test "$with_linux" = "yes"; then - with_console_lock_files=/var/lock - elif test "$with_console_lock_files" = "auto"; then - with_console_lock_files=no + with_chrdev_lock_files=/var/lock + elif test "$with_chrdev_lock_files" = "auto"; then + with_chrdev_lock_files=no fi ;; esac - if test "$with_console_lock_files" = "yes"; then + if test "$with_chrdev_lock_files" = "yes"; then AC_MSG_ERROR([You must specify path for the lock files on this platform]) fi - AC_DEFINE_UNQUOTED([VIR_PTY_LOCK_FILE_PATH], "$with_console_lock_files", - [path to directory containing UUCP pty lock files]) + AC_DEFINE_UNQUOTED([VIR_CHRDEV_LOCK_FILE_PATH], "$with_chrdev_lock_files", + [path to directory containing UUCP device lock files]) fi -AM_CONDITIONAL([VIR_PTY_LOCK_FILE_PATH], [test "$with_console_lock_files" != "no"]) +AM_CONDITIONAL([VIR_CHRDEV_LOCK_FILE_PATH], [test "$with_chrdev_lock_files" != "no"]) dnl SELinux @@ -3279,16 +3279,16 @@ AC_MSG_NOTICE([ Alloc OOM: $enable_oom]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Miscellaneous]) AC_MSG_NOTICE([]) -AC_MSG_NOTICE([ Debug: $enable_debug]) -AC_MSG_NOTICE([ Use -Werror: $set_werror]) -AC_MSG_NOTICE([Warning Flags: $WARN_CFLAGS]) -AC_MSG_NOTICE([ Readline: $lv_use_readline]) -AC_MSG_NOTICE([ Python: $with_python]) -AC_MSG_NOTICE([ DTrace: $with_dtrace]) -AC_MSG_NOTICE([ numad: $with_numad]) -AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) -AC_MSG_NOTICE([ Init script: $with_init_script]) -AC_MSG_NOTICE([Console locks: $with_console_lock_files]) +AC_MSG_NOTICE([ Debug: $enable_debug]) +AC_MSG_NOTICE([ Use -Werror: $set_werror]) +AC_MSG_NOTICE([ Warning Flags: $WARN_CFLAGS]) +AC_MSG_NOTICE([ Readline: $lv_use_readline]) +AC_MSG_NOTICE([ Python: $with_python]) +AC_MSG_NOTICE([ DTrace: $with_dtrace]) +AC_MSG_NOTICE([ numad: $with_numad]) +AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) +AC_MSG_NOTICE([ Init script: $with_init_script]) +AC_MSG_NOTICE([Char device locks: $with_chrdev_lock_files]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Privileges]) AC_MSG_NOTICE([]) diff --git a/src/Makefile.am b/src/Makefile.am index 72184b4..ed86d25 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -235,8 +235,8 @@ ENCRYPTION_CONF_SOURCES = \ CPU_CONF_SOURCES = \ conf/cpu_conf.c conf/cpu_conf.h -# Safe console handling helper APIs -CONSOLE_CONF_SOURCES = \ +# Safe character device handling helper APIs +CHRDEV_CONF_SOURCES = \ conf/virchrdev.c conf/virchrdev.h # Device Helper APIs @@ -255,7 +255,7 @@ CONF_SOURCES = \ $(INTERFACE_CONF_SOURCES) \ $(SECRET_CONF_SOURCES) \ $(CPU_CONF_SOURCES) \ - $(CONSOLE_CONF_SOURCES) \ + $(CHRDEV_CONF_SOURCES) \ $(DEVICE_CONF_SOURCES) # The remote RPC driver, covering domains, storage, networks, etc diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 666432e..2d401d4 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -1,6 +1,6 @@ /** * virchrdev.c: api to guarantee mutually exclusive - * access to domain's consoles + * access to domain's character devices * * Copyright (C) 2011-2012 Red Hat, Inc. * @@ -40,47 +40,47 @@ #define VIR_FROM_THIS VIR_FROM_NONE -/* structure holding information about consoles +/* structure holding information about character devices * open in a given domain */ -struct _virConsoles { +struct _virChrdevs { virMutex lock; virHashTablePtr hash; }; -typedef struct _virConsoleStreamInfo virConsoleStreamInfo; -typedef virConsoleStreamInfo *virConsoleStreamInfoPtr; -struct _virConsoleStreamInfo { - virConsolesPtr cons; - const char *pty; +typedef struct _virChrdevStreamInfo virChrdevStreamInfo; +typedef virChrdevStreamInfo *virChrdevStreamInfoPtr; +struct _virChrdevStreamInfo { + virChrdevsPtr devs; + const char *path; }; -#ifdef VIR_PTY_LOCK_FILE_PATH +#ifdef VIR_CHRDEV_LOCK_FILE_PATH /** * Create a full filename with path to the lock file based on - * name/path of corresponding pty + * name/path of corresponding device * - * @pty path of the console device + * @dev path of the character device * * Returns a modified name that the caller has to free, or NULL * on error. */ -static char *virConsoleLockFilePath(const char *pty) +static char *virChrdevLockFilePath(const char *dev) { char *path = NULL; char *sanitizedPath = NULL; - char *ptyCopy; + char *devCopy; char *filename; char *p; - if (!(ptyCopy = strdup(pty))) { + if (!(devCopy = strdup(dev))) { virReportOOMError(); goto cleanup; } /* skip the leading "/dev/" */ - filename = STRSKIP(ptyCopy, "/dev"); + filename = STRSKIP(devCopy, "/dev"); if (!filename) - filename = ptyCopy; + filename = devCopy; /* substitute path forward slashes for underscores */ p = filename; @@ -90,26 +90,26 @@ static char *virConsoleLockFilePath(const char *pty) ++p; } - if (virAsprintf(&path, "%s/LCK..%s", VIR_PTY_LOCK_FILE_PATH, filename) < 0) + if (virAsprintf(&path, "%s/LCK..%s", VIR_CHRDEV_LOCK_FILE_PATH, filename) < 0) goto cleanup; sanitizedPath = virFileSanitizePath(path); cleanup: VIR_FREE(path); - VIR_FREE(ptyCopy); + VIR_FREE(devCopy); return sanitizedPath; } /** - * Verify and create a lock file for a console pty + * Verify and create a lock file for a character device * - * @pty Path of the console device + * @dev Path of the character device * * Returns 0 on success, -1 on error */ -static int virConsoleLockFileCreate(const char *pty) +static int virChrdevLockFileCreate(const char *dev) { char *path = NULL; int ret = -1; @@ -118,16 +118,16 @@ static int virConsoleLockFileCreate(const char *pty) pid_t pid; /* build lock file path */ - if (!(path = virConsoleLockFilePath(pty))) + if (!(path = virChrdevLockFilePath(dev))) goto cleanup; /* check if a log file and process holding the lock still exists */ if (virPidFileReadPathIfAlive(path, &pid, NULL) == 0 && pid >= 0) { /* the process exists, the lockfile is valid */ virReportError(VIR_ERR_OPERATION_FAILED, - _("Requested console pty '%s' is locked by " + _("Requested device '%s' is locked by " "lock file '%s' held by process %lld"), - pty, path, (long long) pid); + dev, path, (long long) pid); goto cleanup; } else { /* clean up the stale/corrupted/nonexistent lockfile */ @@ -148,15 +148,15 @@ static int virConsoleLockFileCreate(const char *pty) * we run in daemon mode and thus privileged. */ if (errno == EACCES && geteuid() != 0) { - VIR_DEBUG("Skipping lock file creation for pty '%s in path '%s'.", - pty, path); + VIR_DEBUG("Skipping lock file creation for device '%s in path '%s'.", + dev, path); ret = 0; goto cleanup; } virReportSystemError(errno, _("Couldn't create lock file for " - "pty '%s' in path '%s'"), - pty, path); + "device '%s' in path '%s'"), + dev, path); goto cleanup; } @@ -164,8 +164,8 @@ static int virConsoleLockFileCreate(const char *pty) if (safewrite(lockfd, pidStr, strlen(pidStr)) < 0) { virReportSystemError(errno, _("Couldn't write to lock file for " - "pty '%s' in path '%s'"), - pty, path); + "device '%s' in path '%s'"), + dev, path); VIR_FORCE_CLOSE(lockfd); unlink(path); goto cleanup; @@ -183,47 +183,47 @@ cleanup: } /** - * Remove a lock file for a pty + * Remove a lock file for a device * - * @pty Path of the pty device + * @dev Path of the device */ -static void virConsoleLockFileRemove(const char *pty) +static void virChrdevLockFileRemove(const char *dev) { - char *path = virConsoleLockFilePath(pty); + char *path = virChrdevLockFilePath(dev); if (path) unlink(path); VIR_FREE(path); } -#else /* #ifdef VIR_PTY_LOCK_FILE_PATH */ -/* file locking for console devices is disabled */ -static int virConsoleLockFileCreate(const char *pty ATTRIBUTE_UNUSED) +#else /* #ifdef VIR_CHRDEV_LOCK_FILE_PATH */ +/* file locking for character devices is disabled */ +static int virChrdevLockFileCreate(const char *dev ATTRIBUTE_UNUSED) { return 0; } -static void virConsoleLockFileRemove(const char *pty ATTRIBUTE_UNUSED) +static void virChrdevLockFileRemove(const char *dev ATTRIBUTE_UNUSED) { return; } -#endif /* #ifdef VIR_PTY_LOCK_FILE_PATH */ +#endif /* #ifdef VIR_CHRDEV_LOCK_FILE_PATH */ /** - * Frees an entry from the hash containing domain's active consoles + * Frees an entry from the hash containing domain's active devices * - * @data Opaque data, struct holding information about the console - * @name Path of the pty. + * @data Opaque data, struct holding information about the device + * @name Path of the device. */ -static void virConsoleHashEntryFree(void *data, +static void virChrdevHashEntryFree(void *data, const void *name) { - const char *pty = name; + const char *dev = name; virStreamPtr st = data; /* free stream reference */ virStreamFree(st); /* delete lock file */ - virConsoleLockFileRemove(pty); + virChrdevLockFileRemove(dev); } /** @@ -231,65 +231,65 @@ static void virConsoleHashEntryFree(void *data, * * @opaque Data to be freed. */ -static void virConsoleFDStreamCloseCbFree(void *opaque) +static void virChrdevFDStreamCloseCbFree(void *opaque) { - virConsoleStreamInfoPtr priv = opaque; + virChrdevStreamInfoPtr priv = opaque; - VIR_FREE(priv->pty); + VIR_FREE(priv->path); VIR_FREE(priv); } /** - * Callback being called if a FDstream is closed. Frees console entries + * Callback being called if a FDstream is closed. Frees device entries * from data structures and removes lockfiles. * * @st Pointer to stream being closed. - * @opaque Domain's console information structure. + * @opaque Domain's device information structure. */ -static void virConsoleFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, +static void virChrdevFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, void *opaque) { - virConsoleStreamInfoPtr priv = opaque; - virMutexLock(&priv->cons->lock); + virChrdevStreamInfoPtr priv = opaque; + virMutexLock(&priv->devs->lock); /* remove entry from hash */ - virHashRemoveEntry(priv->cons->hash, priv->pty); + virHashRemoveEntry(priv->devs->hash, priv->path); - virMutexUnlock(&priv->cons->lock); + virMutexUnlock(&priv->devs->lock); } /** - * Allocate structures for storing information about active console streams + * Allocate structures for storing information about active device streams * in domain's private data section. * * Returns pointer to the allocated structure or NULL on error */ -virConsolesPtr virConsoleAlloc(void) +virChrdevsPtr virChrdevAlloc(void) { - virConsolesPtr cons; - if (VIR_ALLOC(cons) < 0) + virChrdevsPtr devs; + if (VIR_ALLOC(devs) < 0) return NULL; - if (virMutexInit(&cons->lock) < 0) { - VIR_FREE(cons); + if (virMutexInit(&devs->lock) < 0) { + VIR_FREE(devs); return NULL; } - /* there will hardly be any consoles most of the time, the hash + /* there will hardly be any devices most of the time, the hash * does not have to be huge */ - if (!(cons->hash = virHashCreate(3, virConsoleHashEntryFree))) + if (!(devs->hash = virHashCreate(3, virChrdevHashEntryFree))) goto error; - return cons; + return devs; error: - virConsoleFree(cons); + virChrdevFree(devs); return NULL; } /** * Helper to clear stream callbacks when freeing the hash */ -static void virConsoleFreeClearCallbacks(void *payload, +static void virChrdevFreeClearCallbacks(void *payload, const void *name ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { @@ -299,80 +299,80 @@ static void virConsoleFreeClearCallbacks(void *payload, } /** - * Free structures for handling open console streams. + * Free structures for handling open device streams. * - * @cons Pointer to the private structure. + * @devs Pointer to the private structure. */ -void virConsoleFree(virConsolesPtr cons) +void virChrdevFree(virChrdevsPtr devs) { - if (!cons || !cons->hash) + if (!devs || !devs->hash) return; - virMutexLock(&cons->lock); - virHashForEach(cons->hash, virConsoleFreeClearCallbacks, NULL); - virHashFree(cons->hash); - virMutexUnlock(&cons->lock); - virMutexDestroy(&cons->lock); + virMutexLock(&devs->lock); + virHashForEach(devs->hash, virChrdevFreeClearCallbacks, NULL); + virHashFree(devs->hash); + virMutexUnlock(&devs->lock); + virMutexDestroy(&devs->lock); - VIR_FREE(cons); + VIR_FREE(devs); } /** - * Open a console stream for a domain ensuring that other streams are - * not using the console, nor any lockfiles exist. This ensures that - * the console stream does not get corrupted due to a race on reading + * Open a device stream for a domain ensuring that other streams are + * not using the device, nor any lockfiles exist. This ensures that + * the device stream does not get corrupted due to a race on reading * same FD by two processes. * - * @cons Pointer to private structure holding data about console streams. - * @pty Path to the pseudo tty to be opened. - * @st Stream the client wishes to use for the console connection. - * @force On true, close active console streams for the selected console pty - * before opening this connection. + * @devs Pointer to private structure holding data about device streams. + * @path Path to the character device to be opened. + * @st Stream the client wishes to use for the device connection. + * @force On true, close active device streams for the selected character + * device before opening this connection. * - * Returns 0 on success and st is connected to the selected pty and + * Returns 0 on success and st is connected to the selected device and * corresponding lock file is created (if configured). Returns -1 on - * error and 1 if the console stream is open and busy. + * error and 1 if the device stream is open and busy. */ -int virConsoleOpen(virConsolesPtr cons, - const char *pty, +int virChrdevOpen(virChrdevsPtr devs, + const char *path, virStreamPtr st, bool force) { - virConsoleStreamInfoPtr cbdata = NULL; + virChrdevStreamInfoPtr cbdata = NULL; virStreamPtr savedStream; int ret; - virMutexLock(&cons->lock); + virMutexLock(&devs->lock); - if ((savedStream = virHashLookup(cons->hash, pty))) { + if ((savedStream = virHashLookup(devs->hash, path))) { if (!force) { - /* entry found, console is busy */ - virMutexUnlock(&cons->lock); + /* entry found, device is busy */ + virMutexUnlock(&devs->lock); return 1; } else { /* terminate existing connection */ - /* The internal close callback handler needs to lock cons->lock to + /* The internal close callback handler needs to lock devs->lock to * remove the aborted stream from the hash. This would cause a * deadlock as we would try to enter the lock twice from the very * same thread. We need to unregister the callback and abort the - * stream manually before we create a new console connection. + * stream manually before we create a new device connection. */ virFDStreamSetInternalCloseCb(savedStream, NULL, NULL, NULL); virStreamAbort(savedStream); - virHashRemoveEntry(cons->hash, pty); + virHashRemoveEntry(devs->hash, path); /* continue adding a new stream connection */ } } /* create the lock file */ - if ((ret = virConsoleLockFileCreate(pty)) < 0) { - virMutexUnlock(&cons->lock); + if ((ret = virChrdevLockFileCreate(path)) < 0) { + virMutexUnlock(&devs->lock); return ret; } /* obtain a reference to the stream */ if (virStreamRef(st) < 0) { - virMutexUnlock(&cons->lock); + virMutexUnlock(&devs->lock); return -1; } @@ -381,34 +381,34 @@ int virConsoleOpen(virConsolesPtr cons, goto error; } - if (virHashAddEntry(cons->hash, pty, st) < 0) + if (virHashAddEntry(devs->hash, path, st) < 0) goto error; - cbdata->cons = cons; - if (!(cbdata->pty = strdup(pty))) { + cbdata->devs = devs; + if (!(cbdata->path = strdup(path))) { virReportOOMError(); goto error; } - /* open the console pty */ - if (virFDStreamOpenFile(st, pty, 0, 0, O_RDWR) < 0) + /* open the character device */ + if (virFDStreamOpenFile(st, path, 0, 0, O_RDWR) < 0) goto error; /* add cleanup callback */ virFDStreamSetInternalCloseCb(st, - virConsoleFDStreamCloseCb, + virChrdevFDStreamCloseCb, cbdata, - virConsoleFDStreamCloseCbFree); + virChrdevFDStreamCloseCbFree); - virMutexUnlock(&cons->lock); + virMutexUnlock(&devs->lock); return 0; error: virStreamFree(st); - virHashRemoveEntry(cons->hash, pty); + virHashRemoveEntry(devs->hash, path); if (cbdata) - VIR_FREE(cbdata->pty); + VIR_FREE(cbdata->path); VIR_FREE(cbdata); - virMutexUnlock(&cons->lock); + virMutexUnlock(&devs->lock); return -1; } diff --git a/src/conf/virchrdev.h b/src/conf/virchrdev.h index d5a926a..57d7576 100644 --- a/src/conf/virchrdev.h +++ b/src/conf/virchrdev.h @@ -1,6 +1,6 @@ /** * virchrdev.h: api to guarantee mutually exclusive - * access to domain's consoles + * access to domain's character devices * * Copyright (C) 2011-2012 Red Hat, Inc. * @@ -20,17 +20,17 @@ * * Author: Peter Krempa <pkrempa@redhat.com> */ -#ifndef __VIR_CONSOLE_H__ -# define __VIR_CONSOLE_H__ +#ifndef __VIR_CHRDEV_H__ +# define __VIR_CHRDEV_H__ # include "internal.h" -typedef struct _virConsoles virConsoles; -typedef virConsoles *virConsolesPtr; +typedef struct _virChrdevs virChrdevs; +typedef virChrdevs *virChrdevsPtr; -virConsolesPtr virConsoleAlloc(void); -void virConsoleFree(virConsolesPtr cons); +virChrdevsPtr virChrdevAlloc(void); +void virChrdevFree(virChrdevsPtr devs); -int virConsoleOpen(virConsolesPtr cons, const char *pty, +int virChrdevOpen(virChrdevsPtr devs, const char *path, virStreamPtr st, bool force); -#endif /*__VIR_DOMAIN_CONSOLE_LOCK_H__*/ +#endif /*__VIR_CHRDEV_H__*/ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 396c2f9..b76cad3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1351,9 +1351,9 @@ virAuditSend; # virchrdev.h -virConsoleAlloc; -virConsoleFree; -virConsoleOpen; +virChrdevAlloc; +virChrdevFree; +virChrdevOpen; # virdbus.h diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8d8cf02..2d1007f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -215,7 +215,7 @@ static void *qemuDomainObjPrivateAlloc(void) if (qemuDomainObjInitJob(priv) < 0) goto error; - if (!(priv->cons = virConsoleAlloc())) + if (!(priv->devs = virChrdevAlloc())) goto error; priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; @@ -240,7 +240,7 @@ static void qemuDomainObjPrivateFree(void *data) VIR_FREE(priv->lockState); VIR_FREE(priv->origname); - virConsoleFree(priv->cons); + virChrdevFree(priv->devs); /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3a51874..f9f17cf 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -155,7 +155,7 @@ struct _qemuDomainObjPrivate { unsigned long migMaxBandwidth; char *origname; - virConsolesPtr cons; + virChrdevsPtr devs; qemuDomainCleanupCallback *cleanupCallbacks; size_t ncleanupCallbacks; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6f0849c..824303f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12890,10 +12890,10 @@ qemuDomainOpenConsole(virDomainPtr dom, } /* handle mutually exclusive access to console devices */ - ret = virConsoleOpen(priv->cons, - chr->source.data.file.path, - st, - (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0); + ret = virChrdevOpen(priv->devs, + chr->source.data.file.path, + st, + (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0); if (ret == 1) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", -- 1.7.11.7

On Thu, Dec 13, 2012 at 11:24:18AM -0500, John Eckersberg wrote:
The functionality provided in virchrdev.c (previously virconsole.c) is applicable to other types of character devices besides consoles, such as channels. This patch is just code motion, renaming things such as "console" or "pty", instead using more general terms such as "character device" or "device path". --- configure.ac | 48 +++++------ src/Makefile.am | 6 +- src/conf/virchrdev.c | 220 +++++++++++++++++++++++------------------------ src/conf/virchrdev.h | 18 ++-- src/libvirt_private.syms | 6 +- src/qemu/qemu_domain.c | 4 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 8 +- 8 files changed, 156 insertions(+), 156 deletions(-)
ACK, just one big setof renames Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This also changes the function signature to take a virDomainChrSourceDefPtr instead of just a path, since it needs to differentiate behavior based on source->type. --- src/conf/virchrdev.c | 38 +++++++++++++++++++++++++++++++++----- src/conf/virchrdev.h | 5 +++-- src/qemu/qemu_driver.c | 2 +- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 2d401d4..4c76900 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -324,7 +324,7 @@ void virChrdevFree(virChrdevsPtr devs) * same FD by two processes. * * @devs Pointer to private structure holding data about device streams. - * @path Path to the character device to be opened. + * @source Pointer to private structure holding data about device source. * @st Stream the client wishes to use for the device connection. * @force On true, close active device streams for the selected character * device before opening this connection. @@ -334,14 +334,29 @@ void virChrdevFree(virChrdevsPtr devs) * error and 1 if the device stream is open and busy. */ int virChrdevOpen(virChrdevsPtr devs, - const char *path, - virStreamPtr st, - bool force) + virDomainChrSourceDefPtr source, + virStreamPtr st, + bool force) { virChrdevStreamInfoPtr cbdata = NULL; virStreamPtr savedStream; + const char *path; int ret; + switch (source->type) { + case VIR_DOMAIN_CHR_TYPE_PTY: + path = source->data.file.path; + break; + case VIR_DOMAIN_CHR_TYPE_UNIX: + path = source->data.nix.path; + break; + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported device type '%s'"), + virDomainChrTypeToString(source->type)); + return -1; + } + virMutexLock(&devs->lock); if ((savedStream = virHashLookup(devs->hash, path))) { @@ -391,8 +406,21 @@ int virChrdevOpen(virChrdevsPtr devs, } /* open the character device */ - if (virFDStreamOpenFile(st, path, 0, 0, O_RDWR) < 0) + switch (source->type) { + case VIR_DOMAIN_CHR_TYPE_PTY: + if (virFDStreamOpenFile(st, path, 0, 0, O_RDWR) < 0) + goto error; + break; + case VIR_DOMAIN_CHR_TYPE_UNIX: + if (virFDStreamConnectUNIX(st, path, false) < 0) + goto error; + break; + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported device type '%s'"), + virDomainChrTypeToString(source->type)); goto error; + } /* add cleanup callback */ virFDStreamSetInternalCloseCb(st, diff --git a/src/conf/virchrdev.h b/src/conf/virchrdev.h index 57d7576..e1990e8 100644 --- a/src/conf/virchrdev.h +++ b/src/conf/virchrdev.h @@ -24,6 +24,7 @@ # define __VIR_CHRDEV_H__ # include "internal.h" +# include "domain_conf.h" typedef struct _virChrdevs virChrdevs; typedef virChrdevs *virChrdevsPtr; @@ -31,6 +32,6 @@ typedef virChrdevs *virChrdevsPtr; virChrdevsPtr virChrdevAlloc(void); void virChrdevFree(virChrdevsPtr devs); -int virChrdevOpen(virChrdevsPtr devs, const char *path, - virStreamPtr st, bool force); +int virChrdevOpen(virChrdevsPtr devs, virDomainChrSourceDefPtr source, + virStreamPtr st, bool force); #endif /*__VIR_CHRDEV_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 824303f..441272d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12891,7 +12891,7 @@ qemuDomainOpenConsole(virDomainPtr dom, /* handle mutually exclusive access to console devices */ ret = virChrdevOpen(priv->devs, - chr->source.data.file.path, + &chr->source, st, (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0); -- 1.7.11.7

On Thu, Dec 13, 2012 at 11:24:19AM -0500, John Eckersberg wrote:
This also changes the function signature to take a virDomainChrSourceDefPtr instead of just a path, since it needs to differentiate behavior based on source->type. --- src/conf/virchrdev.c | 38 +++++++++++++++++++++++++++++++++----- src/conf/virchrdev.h | 5 +++-- src/qemu/qemu_driver.c | 2 +- 3 files changed, 37 insertions(+), 8 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/qemu/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 441272d..bcd9c57 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12907,6 +12907,78 @@ cleanup: return ret; } +static int +qemuDomainOpenChannel(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int ret = -1; + int i; + virDomainChrDefPtr chr = NULL; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(VIR_DOMAIN_CHANNEL_FORCE, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (name) { + for (i = 0 ; !chr && i < vm->def->nchannels ; i++) { + if (STREQ(name, vm->def->channels[i]->info.alias)) + chr = vm->def->channels[i]; + + if (vm->def->channels[i]->targetType == \ + VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + STREQ(name, vm->def->channels[i]->target.name)) + chr = vm->def->channels[i]; + } + } else { + if (vm->def->nchannels) + chr = vm->def->channels[0]; + } + + if (!chr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find channel %s"), + NULLSTR(name)); + goto cleanup; + } + + if (chr->source.type != VIR_DOMAIN_CHR_TYPE_UNIX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("channel %s is not using a UNIX socket"), + NULLSTR(name)); + goto cleanup; + } + + /* handle mutually exclusive access to channel devices */ + ret = virChrdevOpen(priv->devs, + &chr->source, + st, + (flags & VIR_DOMAIN_CHANNEL_FORCE) != 0); + + if (ret == 1) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Active channel stream exists for this domain")); + ret = -1; + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static char * qemuDiskPathToAlias(virDomainObjPtr vm, const char *path, int *idx) { @@ -15062,6 +15134,7 @@ static virDriver qemuDriver = { .nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */ .domainFSTrim = qemuDomainFSTrim, /* 1.0.1 */ + .domainOpenChannel = qemuDomainOpenChannel, /* 1.0.1 */ }; -- 1.7.11.7

On Thu, Dec 13, 2012 at 11:24:20AM -0500, John Eckersberg wrote:
--- src/qemu/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 441272d..bcd9c57 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12907,6 +12907,78 @@ cleanup: return ret; }
+static int +qemuDomainOpenChannel(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int ret = -1; + int i; + virDomainChrDefPtr chr = NULL; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(VIR_DOMAIN_CHANNEL_FORCE, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (name) { + for (i = 0 ; !chr && i < vm->def->nchannels ; i++) { + if (STREQ(name, vm->def->channels[i]->info.alias)) + chr = vm->def->channels[i]; + + if (vm->def->channels[i]->targetType == \ + VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + STREQ(name, vm->def->channels[i]->target.name)) + chr = vm->def->channels[i]; + } + } else { + if (vm->def->nchannels) + chr = vm->def->channels[0]; + } + + if (!chr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find channel %s"), + NULLSTR(name)); + goto cleanup; + } + + if (chr->source.type != VIR_DOMAIN_CHR_TYPE_UNIX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("channel %s is not using a UNIX socket"), + NULLSTR(name)); + goto cleanup; + } + + /* handle mutually exclusive access to channel devices */ + ret = virChrdevOpen(priv->devs, + &chr->source, + st, + (flags & VIR_DOMAIN_CHANNEL_FORCE) != 0); + + if (ret == 1) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Active channel stream exists for this domain")); + ret = -1; + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static char * qemuDiskPathToAlias(virDomainObjPtr vm, const char *path, int *idx) { @@ -15062,6 +15134,7 @@ static virDriver qemuDriver = { .nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */ .domainFSTrim = qemuDomainFSTrim, /* 1.0.1 */ + .domainOpenChannel = qemuDomainOpenChannel, /* 1.0.1 */
s/1.0.1/1.0.2/ ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Thu, Dec 13, 2012 at 11:24:15AM -0500, John Eckersberg wrote:
This series enables the qemu driver to tunnel a virtio channel. This is useful for a remote session to communicate with a guest channel via the streaming API.
This was originally fleshed out a while back in this thread: https://www.redhat.com/archives/libvir-list/2011-September/msg01049.html
This implements only item (3) in that list.
The new API is nearly identical to the existing virDomainOpenConsole API, except it works on channels, and supports UNIX sockets in addition to PTYs for channel source type.
This is my first libvirt patch, please be gentle :)
I've not done a detailed review, but in general it all looks sane to me. It'll have to wait until after our current pending release until we in a position to consider applying it though. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 13.12.2012 17:24, John Eckersberg wrote:
This series enables the qemu driver to tunnel a virtio channel. This is useful for a remote session to communicate with a guest channel via the streaming API.
This was originally fleshed out a while back in this thread: https://www.redhat.com/archives/libvir-list/2011-September/msg01049.html
This implements only item (3) in that list.
The new API is nearly identical to the existing virDomainOpenConsole API, except it works on channels, and supports UNIX sockets in addition to PTYs for channel source type.
This is my first libvirt patch, please be gentle :)
John Eckersberg (5): api: Add API to tunnel a guest channel via stream conf: Rename virconsole.* to virchrdev.* conf: Rename console-specific identifiers to be more generic conf: Add unix socket support to virChrdevOpen qemu: Implement virDomainOpenChannel API
configure.ac | 48 ++--- include/libvirt/libvirt.h.in | 16 ++ po/POTFILES.in | 2 +- src/Makefile.am | 8 +- src/conf/virchrdev.c | 442 +++++++++++++++++++++++++++++++++++++++++++ src/conf/virchrdev.h | 37 ++++ src/conf/virconsole.c | 414 ---------------------------------------- src/conf/virconsole.h | 36 ---- src/driver.h | 7 + src/libvirt.c | 61 ++++++ src/libvirt_private.syms | 8 +- src/libvirt_public.syms | 5 + src/qemu/qemu_domain.c | 4 +- src/qemu/qemu_domain.h | 4 +- src/qemu/qemu_driver.c | 81 +++++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +- src/remote_protocol-structs | 6 + 18 files changed, 697 insertions(+), 492 deletions(-) create mode 100644 src/conf/virchrdev.c create mode 100644 src/conf/virchrdev.h delete mode 100644 src/conf/virconsole.c delete mode 100644 src/conf/virconsole.h
I am not hesitant to this approach. In fact, I think it can be combined with Peter's patch set for tunneling a TCP stream (which you omit in your patch set). Thus we will have an API which will be able to tunnel unix socket, PTY and TCP. Or do we want to keep these two split? Michal

On Fri, Dec 14, 2012 at 12:05:31PM +0100, Michal Privoznik wrote:
On 13.12.2012 17:24, John Eckersberg wrote:
This series enables the qemu driver to tunnel a virtio channel. This is useful for a remote session to communicate with a guest channel via the streaming API.
This was originally fleshed out a while back in this thread: https://www.redhat.com/archives/libvir-list/2011-September/msg01049.html
This implements only item (3) in that list.
The new API is nearly identical to the existing virDomainOpenConsole API, except it works on channels, and supports UNIX sockets in addition to PTYs for channel source type.
This is my first libvirt patch, please be gentle :)
John Eckersberg (5): api: Add API to tunnel a guest channel via stream conf: Rename virconsole.* to virchrdev.* conf: Rename console-specific identifiers to be more generic conf: Add unix socket support to virChrdevOpen qemu: Implement virDomainOpenChannel API
configure.ac | 48 ++--- include/libvirt/libvirt.h.in | 16 ++ po/POTFILES.in | 2 +- src/Makefile.am | 8 +- src/conf/virchrdev.c | 442 +++++++++++++++++++++++++++++++++++++++++++ src/conf/virchrdev.h | 37 ++++ src/conf/virconsole.c | 414 ---------------------------------------- src/conf/virconsole.h | 36 ---- src/driver.h | 7 + src/libvirt.c | 61 ++++++ src/libvirt_private.syms | 8 +- src/libvirt_public.syms | 5 + src/qemu/qemu_domain.c | 4 +- src/qemu/qemu_domain.h | 4 +- src/qemu/qemu_driver.c | 81 +++++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +- src/remote_protocol-structs | 6 + 18 files changed, 697 insertions(+), 492 deletions(-) create mode 100644 src/conf/virchrdev.c create mode 100644 src/conf/virchrdev.h delete mode 100644 src/conf/virconsole.c delete mode 100644 src/conf/virconsole.h
I am not hesitant to this approach. In fact, I think it can be combined with Peter's patch set for tunneling a TCP stream (which you omit in your patch set). Thus we will have an API which will be able to tunnel unix socket, PTY and TCP. Or do we want to keep these two split?
This API John is proposing mirrors the API used for virDomainOpenConsole. I don't much like the general TCP tunnelling API because it is at quite a low level. It is nicer to have the API directly associated with the domain name + channel name, so we can provide access control based on the domain / device. It also isolates the client from needing to know what type of console is configiured. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Attached is a simple test program to exercise virDomainOpenChannel that echoes a toupper()'d string and exits. Edit the #defines as necessary to match your setup. You'll need a channel defined in the guest similar to: <channel type="unix"> <source mode="bind" path="/tmp/guestfsd.sock"/> <target type="virtio" name="org.libguestfs.channel.0"/> </channel> Run the test, and then inside of the VM, you should be able to: [root@f17-minimal ~]# socat - /dev/virtio-ports/org.libguestfs.channel.0 hello world HELLO WORLD [root@f17-minimal ~]# #include <ctype.h> #include <err.h> #include <stdlib.h> #include <string.h> #include <libvirt/libvirt.h> #define CONNECT_URI "qemu+ssh://root@localhost/system" #define DOMAIN "f17-minimal" #define CHANNEL "org.libguestfs.channel.0" #define BUF_SIZE 80 void upper(char *c) { while (*c != '\0') { *c = toupper((unsigned char)*c); c++; } } int main(int argc, char *argv[]) { virConnectPtr conn; virDomainPtr dom; virStreamPtr st; int bytes_read; char buf[BUF_SIZE]; if ((conn = virConnectOpen(CONNECT_URI)) == NULL) errx(1, "virConnectOpen"); if ((dom = virDomainLookupByName(conn, DOMAIN)) == NULL) errx(1, "virDomainLookupByName"); if ((st = virStreamNew(conn, 0)) == NULL) errx(1, "virStreamNew"); if (virDomainOpenChannel(dom, CHANNEL, st, 0) == -1) errx(1, "virDomainOpenChannel"); if ((bytes_read = virStreamRecv(st, buf, BUF_SIZE)) > 0) { buf[bytes_read] = '\0'; upper(buf); virStreamSend(st, buf, strnlen(buf, BUF_SIZE)); } if (virStreamFinish(st) < 0) errx(1, "virStreamFinish"); if (virStreamFree(st) < 0) errx(1, "virStreamFree"); return 0; }

This series enables the qemu driver to tunnel a virtio channel. This is useful for a remote session to communicate with a guest channel via the streaming API. This was originally fleshed out a while back in this thread: https://www.redhat.com/archives/libvir-list/2011-September/msg01049.html This implements only item (3) in that list. The new API is nearly identical to the existing virDomainOpenConsole API, except it works on channels, and supports UNIX sockets in addition to PTYs for channel source type. This is my first libvirt patch, please be gentle :) --- v2: - Rebased against master - Correct version to 1.0.2 in 5/5 --- John Eckersberg (5): api: Add API to tunnel a guest channel via stream conf: Rename virconsole.* to virchrdev.* conf: Rename console-specific identifiers to be more generic conf: Add unix socket support to virChrdevOpen qemu: Implement virDomainOpenChannel API configure.ac | 48 ++--- include/libvirt/libvirt.h.in | 16 ++ po/POTFILES.in | 2 +- src/Makefile.am | 8 +- src/conf/virchrdev.c | 442 +++++++++++++++++++++++++++++++++++++++++++ src/conf/virchrdev.h | 37 ++++ src/conf/virconsole.c | 414 ---------------------------------------- src/conf/virconsole.h | 36 ---- src/driver.h | 7 + src/libvirt.c | 61 ++++++ src/libvirt_private.syms | 8 +- src/libvirt_public.syms | 5 + src/qemu/qemu_domain.c | 4 +- src/qemu/qemu_domain.h | 4 +- src/qemu/qemu_driver.c | 81 +++++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +- src/remote_protocol-structs | 6 + 18 files changed, 697 insertions(+), 492 deletions(-) create mode 100644 src/conf/virchrdev.c create mode 100644 src/conf/virchrdev.h delete mode 100644 src/conf/virconsole.c delete mode 100644 src/conf/virconsole.h -- 1.7.11.7

This patch adds a new API, virDomainOpenChannel, that uses streams to connect to a virtio channel on a guest. This creates a secure communication channel between a guest and a libvirt client. This behaves the same as virDomainOpenConsole, except on channels instead of console/serial/parallel devices. --- include/libvirt/libvirt.h.in | 16 ++++++++++++ src/driver.h | 7 +++++ src/libvirt.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 ++++++- src/remote_protocol-structs | 6 +++++ 7 files changed, 104 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2de6835..78262a2 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4547,6 +4547,22 @@ int virDomainOpenConsole(virDomainPtr dom, virStreamPtr st, unsigned int flags); +/** + * virDomainChannelFlags + * + * Since 1.0.2 + */ +typedef enum { + VIR_DOMAIN_CHANNEL_FORCE = (1 << 0), /* abort a (possibly) active channel + connection to force a new + connection */ +} virDomainChannelFlags; + +int virDomainOpenChannel(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags); + typedef enum { VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH = (1 << 0), } virDomainOpenGraphicsFlags; diff --git a/src/driver.h b/src/driver.h index 64d652f..01c95cf 100644 --- a/src/driver.h +++ b/src/driver.h @@ -717,6 +717,12 @@ typedef int virStreamPtr st, unsigned int flags); typedef int + (*virDrvDomainOpenChannel)(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags); + +typedef int (*virDrvDomainOpenGraphics)(virDomainPtr dom, unsigned int idx, int fd, @@ -1078,6 +1084,7 @@ struct _virDriver { virDrvDomainQemuAttach qemuDomainAttach; virDrvDomainQemuAgentCommand qemuDomainArbitraryAgentCommand; virDrvDomainOpenConsole domainOpenConsole; + virDrvDomainOpenChannel domainOpenChannel; virDrvDomainOpenGraphics domainOpenGraphics; virDrvDomainInjectNMI domainInjectNMI; virDrvDomainMigrateBegin3 domainMigrateBegin3; diff --git a/src/libvirt.c b/src/libvirt.c index bf674d1..6d1da12 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19118,6 +19118,67 @@ error: } /** + * virDomainOpenChannel: + * @dom: a domain object + * @name: the channel name, or NULL + * @st: a stream to associate with the channel + * @flags: bitwise-OR of virDomainChannelFlags + * + * This opens the host interface associated with a channel device on a + * guest, if the host interface is supported. If @name is given, it + * can match either the device alias (e.g. "channel0"), or the virtio + * target name (e.g. "org.qemu.guest_agent.0"). If @name is omitted, + * then the first channel is opened. The channel is associated with + * the passed in @st stream, which should have been opened in + * non-blocking mode for bi-directional I/O. + * + * By default, when @flags is 0, the open will fail if libvirt detects + * that the channel is already in use by another client; passing + * VIR_DOMAIN_CHANNEL_FORCE will cause libvirt to forcefully remove the + * other client prior to opening this channel. + * + * Returns 0 if the channel was opened, -1 on error + */ +int virDomainOpenChannel(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "name=%s, st=%p, flags=%x", + NULLSTR(name), st, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = dom->conn; + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->domainOpenChannel) { + int ret; + ret = conn->driver->domainOpenChannel(dom, name, st, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** * virDomainBlockJobAbort: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index e3d63d3..2107519 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -580,4 +580,9 @@ LIBVIRT_1.0.1 { virDomainSendProcessSignal; } LIBVIRT_1.0.0; +LIBVIRT_1.0.2 { + global: + virDomainOpenChannel; +} LIBVIRT_1.0.1; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ae861cc..c078cb5 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6121,6 +6121,7 @@ static virDriver remote_driver = { .qemuDomainAttach = qemuDomainAttach, /* 0.9.4 */ .qemuDomainArbitraryAgentCommand = qemuDomainAgentCommand, /* 0.10.0 */ .domainOpenConsole = remoteDomainOpenConsole, /* 0.8.6 */ + .domainOpenChannel = remoteDomainOpenChannel, /* 1.0.2 */ .domainOpenGraphics = remoteDomainOpenGraphics, /* 0.9.7 */ .domainInjectNMI = remoteDomainInjectNMI, /* 0.9.2 */ .domainMigrateBegin3 = remoteDomainMigrateBegin3, /* 0.9.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index bdad9f0..9035776 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2439,6 +2439,12 @@ struct remote_domain_open_console_args { unsigned int flags; }; +struct remote_domain_open_channel_args { + remote_nonnull_domain dom; + remote_string name; + unsigned int flags; +}; + struct remote_storage_vol_upload_args { remote_nonnull_storage_vol vol; unsigned hyper offset; @@ -3042,7 +3048,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, /* autogen autogen */ REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_FSTRIM = 294, /* autogen autogen */ - REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295 /* autogen autogen */ + REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295, /* autogen autogen */ + REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296 /* autogen autogen | readstream@2 */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e7d05b8..91414d4 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1873,6 +1873,11 @@ struct remote_domain_open_console_args { remote_string dev_name; u_int flags; }; +struct remote_domain_open_channel_args { + remote_nonnull_domain dom; + remote_string name; + u_int flags; +}; struct remote_storage_vol_upload_args { remote_nonnull_storage_vol vol; uint64_t offset; @@ -2447,4 +2452,5 @@ enum remote_procedure { REMOTE_PROC_NODE_GET_CPU_MAP = 293, REMOTE_PROC_DOMAIN_FSTRIM = 294, REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295, + REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296, }; -- 1.7.11.7

This is just code motion, in preparation to rename identifiers to be less console-specific. --- po/POTFILES.in | 2 +- src/Makefile.am | 2 +- src/conf/virchrdev.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/virchrdev.h | 36 +++++ src/conf/virconsole.c | 414 ----------------------------------------------- src/conf/virconsole.h | 36 ----- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.h | 2 +- 8 files changed, 454 insertions(+), 454 deletions(-) create mode 100644 src/conf/virchrdev.c create mode 100644 src/conf/virchrdev.h delete mode 100644 src/conf/virconsole.c delete mode 100644 src/conf/virconsole.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 4d94799..95619f9 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -22,7 +22,7 @@ src/conf/secret_conf.c src/conf/snapshot_conf.c src/conf/storage_conf.c src/conf/storage_encryption_conf.c -src/conf/virconsole.c +src/conf/virchrdev.c src/cpu/cpu.c src/cpu/cpu_generic.c src/cpu/cpu_map.c diff --git a/src/Makefile.am b/src/Makefile.am index f7a9b91..02bf47d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -246,7 +246,7 @@ CPU_CONF_SOURCES = \ # Safe console handling helper APIs CONSOLE_CONF_SOURCES = \ - conf/virconsole.c conf/virconsole.h + conf/virchrdev.c conf/virchrdev.h # Device Helper APIs DEVICE_CONF_SOURCES = \ diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c new file mode 100644 index 0000000..7b471ae --- /dev/null +++ b/src/conf/virchrdev.c @@ -0,0 +1,414 @@ +/** + * virchrdev.c: api to guarantee mutually exclusive + * access to domain's consoles + * + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Peter Krempa <pkrempa@redhat.com> + */ + +#include <config.h> + +#include <fcntl.h> +#include <unistd.h> +#include <sys/types.h> + +#include "virchrdev.h" +#include "virhash.h" +#include "fdstream.h" +#include "internal.h" +#include "virthread.h" +#include "viralloc.h" +#include "virpidfile.h" +#include "virlog.h" +#include "virerror.h" +#include "virfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/* structure holding information about consoles + * open in a given domain */ +struct _virConsoles { + virMutex lock; + virHashTablePtr hash; +}; + +typedef struct _virConsoleStreamInfo virConsoleStreamInfo; +typedef virConsoleStreamInfo *virConsoleStreamInfoPtr; +struct _virConsoleStreamInfo { + virConsolesPtr cons; + const char *pty; +}; + +#ifdef VIR_PTY_LOCK_FILE_PATH +/** + * Create a full filename with path to the lock file based on + * name/path of corresponding pty + * + * @pty path of the console device + * + * Returns a modified name that the caller has to free, or NULL + * on error. + */ +static char *virConsoleLockFilePath(const char *pty) +{ + char *path = NULL; + char *sanitizedPath = NULL; + char *ptyCopy; + char *filename; + char *p; + + if (!(ptyCopy = strdup(pty))) { + virReportOOMError(); + goto cleanup; + } + + /* skip the leading "/dev/" */ + filename = STRSKIP(ptyCopy, "/dev"); + if (!filename) + filename = ptyCopy; + + /* substitute path forward slashes for underscores */ + p = filename; + while (*p) { + if (*p == '/') + *p = '_'; + ++p; + } + + if (virAsprintf(&path, "%s/LCK..%s", VIR_PTY_LOCK_FILE_PATH, filename) < 0) + goto cleanup; + + sanitizedPath = virFileSanitizePath(path); + +cleanup: + VIR_FREE(path); + VIR_FREE(ptyCopy); + + return sanitizedPath; +} + +/** + * Verify and create a lock file for a console pty + * + * @pty Path of the console device + * + * Returns 0 on success, -1 on error + */ +static int virConsoleLockFileCreate(const char *pty) +{ + char *path = NULL; + int ret = -1; + int lockfd = -1; + char *pidStr = NULL; + pid_t pid; + + /* build lock file path */ + if (!(path = virConsoleLockFilePath(pty))) + goto cleanup; + + /* check if a log file and process holding the lock still exists */ + if (virPidFileReadPathIfAlive(path, &pid, NULL) == 0 && pid >= 0) { + /* the process exists, the lockfile is valid */ + virReportError(VIR_ERR_OPERATION_FAILED, + _("Requested console pty '%s' is locked by " + "lock file '%s' held by process %lld"), + pty, path, (long long) pid); + goto cleanup; + } else { + /* clean up the stale/corrupted/nonexistent lockfile */ + unlink(path); + } + /* lockfile doesn't (shouldn't) exist */ + + /* ensure correct format according to filesystem hierarchy standard */ + /* http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES */ + if (virAsprintf(&pidStr, "%10lld\n", (long long) getpid()) < 0) + goto cleanup; + + /* create the lock file */ + if ((lockfd = open(path, O_WRONLY | O_CREAT | O_EXCL, 00644)) < 0) { + /* If we run in session mode, we might have no access to the lock + * file directory. We have to check for an permission denied error + * and see if we can reach it. This should cause an error only if + * we run in daemon mode and thus privileged. + */ + if (errno == EACCES && geteuid() != 0) { + VIR_DEBUG("Skipping lock file creation for pty '%s in path '%s'.", + pty, path); + ret = 0; + goto cleanup; + } + virReportSystemError(errno, + _("Couldn't create lock file for " + "pty '%s' in path '%s'"), + pty, path); + goto cleanup; + } + + /* write the pid to the file */ + if (safewrite(lockfd, pidStr, strlen(pidStr)) < 0) { + virReportSystemError(errno, + _("Couldn't write to lock file for " + "pty '%s' in path '%s'"), + pty, path); + VIR_FORCE_CLOSE(lockfd); + unlink(path); + goto cleanup; + } + + /* we hold the lock */ + ret = 0; + +cleanup: + VIR_FORCE_CLOSE(lockfd); + VIR_FREE(path); + VIR_FREE(pidStr); + + return ret; +} + +/** + * Remove a lock file for a pty + * + * @pty Path of the pty device + */ +static void virConsoleLockFileRemove(const char *pty) +{ + char *path = virConsoleLockFilePath(pty); + if (path) + unlink(path); + VIR_FREE(path); +} +#else /* #ifdef VIR_PTY_LOCK_FILE_PATH */ +/* file locking for console devices is disabled */ +static int virConsoleLockFileCreate(const char *pty ATTRIBUTE_UNUSED) +{ + return 0; +} + +static void virConsoleLockFileRemove(const char *pty ATTRIBUTE_UNUSED) +{ + return; +} +#endif /* #ifdef VIR_PTY_LOCK_FILE_PATH */ + +/** + * Frees an entry from the hash containing domain's active consoles + * + * @data Opaque data, struct holding information about the console + * @name Path of the pty. + */ +static void virConsoleHashEntryFree(void *data, + const void *name) +{ + const char *pty = name; + virStreamPtr st = data; + + /* free stream reference */ + virStreamFree(st); + + /* delete lock file */ + virConsoleLockFileRemove(pty); +} + +/** + * Frees opaque data provided for the stream closing callback + * + * @opaque Data to be freed. + */ +static void virConsoleFDStreamCloseCbFree(void *opaque) +{ + virConsoleStreamInfoPtr priv = opaque; + + VIR_FREE(priv->pty); + VIR_FREE(priv); +} + +/** + * Callback being called if a FDstream is closed. Frees console entries + * from data structures and removes lockfiles. + * + * @st Pointer to stream being closed. + * @opaque Domain's console information structure. + */ +static void virConsoleFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, + void *opaque) +{ + virConsoleStreamInfoPtr priv = opaque; + virMutexLock(&priv->cons->lock); + + /* remove entry from hash */ + virHashRemoveEntry(priv->cons->hash, priv->pty); + + virMutexUnlock(&priv->cons->lock); +} + +/** + * Allocate structures for storing information about active console streams + * in domain's private data section. + * + * Returns pointer to the allocated structure or NULL on error + */ +virConsolesPtr virConsoleAlloc(void) +{ + virConsolesPtr cons; + if (VIR_ALLOC(cons) < 0) + return NULL; + + if (virMutexInit(&cons->lock) < 0) { + VIR_FREE(cons); + return NULL; + } + + /* there will hardly be any consoles most of the time, the hash + * does not have to be huge */ + if (!(cons->hash = virHashCreate(3, virConsoleHashEntryFree))) + goto error; + + return cons; +error: + virConsoleFree(cons); + return NULL; +} + +/** + * Helper to clear stream callbacks when freeing the hash + */ +static void virConsoleFreeClearCallbacks(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + virStreamPtr st = payload; + + virFDStreamSetInternalCloseCb(st, NULL, NULL, NULL); +} + +/** + * Free structures for handling open console streams. + * + * @cons Pointer to the private structure. + */ +void virConsoleFree(virConsolesPtr cons) +{ + if (!cons || !cons->hash) + return; + + virMutexLock(&cons->lock); + virHashForEach(cons->hash, virConsoleFreeClearCallbacks, NULL); + virHashFree(cons->hash); + virMutexUnlock(&cons->lock); + virMutexDestroy(&cons->lock); + + VIR_FREE(cons); +} + +/** + * Open a console stream for a domain ensuring that other streams are + * not using the console, nor any lockfiles exist. This ensures that + * the console stream does not get corrupted due to a race on reading + * same FD by two processes. + * + * @cons Pointer to private structure holding data about console streams. + * @pty Path to the pseudo tty to be opened. + * @st Stream the client wishes to use for the console connection. + * @force On true, close active console streams for the selected console pty + * before opening this connection. + * + * Returns 0 on success and st is connected to the selected pty and + * corresponding lock file is created (if configured). Returns -1 on + * error and 1 if the console stream is open and busy. + */ +int virConsoleOpen(virConsolesPtr cons, + const char *pty, + virStreamPtr st, + bool force) +{ + virConsoleStreamInfoPtr cbdata = NULL; + virStreamPtr savedStream; + int ret; + + virMutexLock(&cons->lock); + + if ((savedStream = virHashLookup(cons->hash, pty))) { + if (!force) { + /* entry found, console is busy */ + virMutexUnlock(&cons->lock); + return 1; + } else { + /* terminate existing connection */ + /* The internal close callback handler needs to lock cons->lock to + * remove the aborted stream from the hash. This would cause a + * deadlock as we would try to enter the lock twice from the very + * same thread. We need to unregister the callback and abort the + * stream manually before we create a new console connection. + */ + virFDStreamSetInternalCloseCb(savedStream, NULL, NULL, NULL); + virStreamAbort(savedStream); + virHashRemoveEntry(cons->hash, pty); + /* continue adding a new stream connection */ + } + } + + /* create the lock file */ + if ((ret = virConsoleLockFileCreate(pty)) < 0) { + virMutexUnlock(&cons->lock); + return ret; + } + + /* obtain a reference to the stream */ + if (virStreamRef(st) < 0) { + virMutexUnlock(&cons->lock); + return -1; + } + + if (VIR_ALLOC(cbdata) < 0) { + virReportOOMError(); + goto error; + } + + if (virHashAddEntry(cons->hash, pty, st) < 0) + goto error; + + cbdata->cons = cons; + if (!(cbdata->pty = strdup(pty))) { + virReportOOMError(); + goto error; + } + + /* open the console pty */ + if (virFDStreamOpenFile(st, pty, 0, 0, O_RDWR) < 0) + goto error; + + /* add cleanup callback */ + virFDStreamSetInternalCloseCb(st, + virConsoleFDStreamCloseCb, + cbdata, + virConsoleFDStreamCloseCbFree); + + virMutexUnlock(&cons->lock); + return 0; + +error: + virStreamFree(st); + virHashRemoveEntry(cons->hash, pty); + if (cbdata) + VIR_FREE(cbdata->pty); + VIR_FREE(cbdata); + virMutexUnlock(&cons->lock); + return -1; +} diff --git a/src/conf/virchrdev.h b/src/conf/virchrdev.h new file mode 100644 index 0000000..d5a926a --- /dev/null +++ b/src/conf/virchrdev.h @@ -0,0 +1,36 @@ +/** + * virchrdev.h: api to guarantee mutually exclusive + * access to domain's consoles + * + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Peter Krempa <pkrempa@redhat.com> + */ +#ifndef __VIR_CONSOLE_H__ +# define __VIR_CONSOLE_H__ + +# include "internal.h" + +typedef struct _virConsoles virConsoles; +typedef virConsoles *virConsolesPtr; + +virConsolesPtr virConsoleAlloc(void); +void virConsoleFree(virConsolesPtr cons); + +int virConsoleOpen(virConsolesPtr cons, const char *pty, + virStreamPtr st, bool force); +#endif /*__VIR_DOMAIN_CONSOLE_LOCK_H__*/ diff --git a/src/conf/virconsole.c b/src/conf/virconsole.c deleted file mode 100644 index 239e2d2..0000000 --- a/src/conf/virconsole.c +++ /dev/null @@ -1,414 +0,0 @@ -/** - * virconsole.c: api to guarantee mutually exclusive - * access to domain's consoles - * - * Copyright (C) 2011-2012 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see - * <http://www.gnu.org/licenses/>. - * - * Author: Peter Krempa <pkrempa@redhat.com> - */ - -#include <config.h> - -#include <fcntl.h> -#include <unistd.h> -#include <sys/types.h> - -#include "virconsole.h" -#include "virhash.h" -#include "fdstream.h" -#include "internal.h" -#include "virthread.h" -#include "viralloc.h" -#include "virpidfile.h" -#include "virlog.h" -#include "virerror.h" -#include "virfile.h" - -#define VIR_FROM_THIS VIR_FROM_NONE - -/* structure holding information about consoles - * open in a given domain */ -struct _virConsoles { - virMutex lock; - virHashTablePtr hash; -}; - -typedef struct _virConsoleStreamInfo virConsoleStreamInfo; -typedef virConsoleStreamInfo *virConsoleStreamInfoPtr; -struct _virConsoleStreamInfo { - virConsolesPtr cons; - const char *pty; -}; - -#ifdef VIR_PTY_LOCK_FILE_PATH -/** - * Create a full filename with path to the lock file based on - * name/path of corresponding pty - * - * @pty path of the console device - * - * Returns a modified name that the caller has to free, or NULL - * on error. - */ -static char *virConsoleLockFilePath(const char *pty) -{ - char *path = NULL; - char *sanitizedPath = NULL; - char *ptyCopy; - char *filename; - char *p; - - if (!(ptyCopy = strdup(pty))) { - virReportOOMError(); - goto cleanup; - } - - /* skip the leading "/dev/" */ - filename = STRSKIP(ptyCopy, "/dev"); - if (!filename) - filename = ptyCopy; - - /* substitute path forward slashes for underscores */ - p = filename; - while (*p) { - if (*p == '/') - *p = '_'; - ++p; - } - - if (virAsprintf(&path, "%s/LCK..%s", VIR_PTY_LOCK_FILE_PATH, filename) < 0) - goto cleanup; - - sanitizedPath = virFileSanitizePath(path); - -cleanup: - VIR_FREE(path); - VIR_FREE(ptyCopy); - - return sanitizedPath; -} - -/** - * Verify and create a lock file for a console pty - * - * @pty Path of the console device - * - * Returns 0 on success, -1 on error - */ -static int virConsoleLockFileCreate(const char *pty) -{ - char *path = NULL; - int ret = -1; - int lockfd = -1; - char *pidStr = NULL; - pid_t pid; - - /* build lock file path */ - if (!(path = virConsoleLockFilePath(pty))) - goto cleanup; - - /* check if a log file and process holding the lock still exists */ - if (virPidFileReadPathIfAlive(path, &pid, NULL) == 0 && pid >= 0) { - /* the process exists, the lockfile is valid */ - virReportError(VIR_ERR_OPERATION_FAILED, - _("Requested console pty '%s' is locked by " - "lock file '%s' held by process %lld"), - pty, path, (long long) pid); - goto cleanup; - } else { - /* clean up the stale/corrupted/nonexistent lockfile */ - unlink(path); - } - /* lockfile doesn't (shouldn't) exist */ - - /* ensure correct format according to filesystem hierarchy standard */ - /* http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES */ - if (virAsprintf(&pidStr, "%10lld\n", (long long) getpid()) < 0) - goto cleanup; - - /* create the lock file */ - if ((lockfd = open(path, O_WRONLY | O_CREAT | O_EXCL, 00644)) < 0) { - /* If we run in session mode, we might have no access to the lock - * file directory. We have to check for an permission denied error - * and see if we can reach it. This should cause an error only if - * we run in daemon mode and thus privileged. - */ - if (errno == EACCES && geteuid() != 0) { - VIR_DEBUG("Skipping lock file creation for pty '%s in path '%s'.", - pty, path); - ret = 0; - goto cleanup; - } - virReportSystemError(errno, - _("Couldn't create lock file for " - "pty '%s' in path '%s'"), - pty, path); - goto cleanup; - } - - /* write the pid to the file */ - if (safewrite(lockfd, pidStr, strlen(pidStr)) < 0) { - virReportSystemError(errno, - _("Couldn't write to lock file for " - "pty '%s' in path '%s'"), - pty, path); - VIR_FORCE_CLOSE(lockfd); - unlink(path); - goto cleanup; - } - - /* we hold the lock */ - ret = 0; - -cleanup: - VIR_FORCE_CLOSE(lockfd); - VIR_FREE(path); - VIR_FREE(pidStr); - - return ret; -} - -/** - * Remove a lock file for a pty - * - * @pty Path of the pty device - */ -static void virConsoleLockFileRemove(const char *pty) -{ - char *path = virConsoleLockFilePath(pty); - if (path) - unlink(path); - VIR_FREE(path); -} -#else /* #ifdef VIR_PTY_LOCK_FILE_PATH */ -/* file locking for console devices is disabled */ -static int virConsoleLockFileCreate(const char *pty ATTRIBUTE_UNUSED) -{ - return 0; -} - -static void virConsoleLockFileRemove(const char *pty ATTRIBUTE_UNUSED) -{ - return; -} -#endif /* #ifdef VIR_PTY_LOCK_FILE_PATH */ - -/** - * Frees an entry from the hash containing domain's active consoles - * - * @data Opaque data, struct holding information about the console - * @name Path of the pty. - */ -static void virConsoleHashEntryFree(void *data, - const void *name) -{ - const char *pty = name; - virStreamPtr st = data; - - /* free stream reference */ - virStreamFree(st); - - /* delete lock file */ - virConsoleLockFileRemove(pty); -} - -/** - * Frees opaque data provided for the stream closing callback - * - * @opaque Data to be freed. - */ -static void virConsoleFDStreamCloseCbFree(void *opaque) -{ - virConsoleStreamInfoPtr priv = opaque; - - VIR_FREE(priv->pty); - VIR_FREE(priv); -} - -/** - * Callback being called if a FDstream is closed. Frees console entries - * from data structures and removes lockfiles. - * - * @st Pointer to stream being closed. - * @opaque Domain's console information structure. - */ -static void virConsoleFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, - void *opaque) -{ - virConsoleStreamInfoPtr priv = opaque; - virMutexLock(&priv->cons->lock); - - /* remove entry from hash */ - virHashRemoveEntry(priv->cons->hash, priv->pty); - - virMutexUnlock(&priv->cons->lock); -} - -/** - * Allocate structures for storing information about active console streams - * in domain's private data section. - * - * Returns pointer to the allocated structure or NULL on error - */ -virConsolesPtr virConsoleAlloc(void) -{ - virConsolesPtr cons; - if (VIR_ALLOC(cons) < 0) - return NULL; - - if (virMutexInit(&cons->lock) < 0) { - VIR_FREE(cons); - return NULL; - } - - /* there will hardly be any consoles most of the time, the hash - * does not have to be huge */ - if (!(cons->hash = virHashCreate(3, virConsoleHashEntryFree))) - goto error; - - return cons; -error: - virConsoleFree(cons); - return NULL; -} - -/** - * Helper to clear stream callbacks when freeing the hash - */ -static void virConsoleFreeClearCallbacks(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED) -{ - virStreamPtr st = payload; - - virFDStreamSetInternalCloseCb(st, NULL, NULL, NULL); -} - -/** - * Free structures for handling open console streams. - * - * @cons Pointer to the private structure. - */ -void virConsoleFree(virConsolesPtr cons) -{ - if (!cons || !cons->hash) - return; - - virMutexLock(&cons->lock); - virHashForEach(cons->hash, virConsoleFreeClearCallbacks, NULL); - virHashFree(cons->hash); - virMutexUnlock(&cons->lock); - virMutexDestroy(&cons->lock); - - VIR_FREE(cons); -} - -/** - * Open a console stream for a domain ensuring that other streams are - * not using the console, nor any lockfiles exist. This ensures that - * the console stream does not get corrupted due to a race on reading - * same FD by two processes. - * - * @cons Pointer to private structure holding data about console streams. - * @pty Path to the pseudo tty to be opened. - * @st Stream the client wishes to use for the console connection. - * @force On true, close active console streams for the selected console pty - * before opening this connection. - * - * Returns 0 on success and st is connected to the selected pty and - * corresponding lock file is created (if configured). Returns -1 on - * error and 1 if the console stream is open and busy. - */ -int virConsoleOpen(virConsolesPtr cons, - const char *pty, - virStreamPtr st, - bool force) -{ - virConsoleStreamInfoPtr cbdata = NULL; - virStreamPtr savedStream; - int ret; - - virMutexLock(&cons->lock); - - if ((savedStream = virHashLookup(cons->hash, pty))) { - if (!force) { - /* entry found, console is busy */ - virMutexUnlock(&cons->lock); - return 1; - } else { - /* terminate existing connection */ - /* The internal close callback handler needs to lock cons->lock to - * remove the aborted stream from the hash. This would cause a - * deadlock as we would try to enter the lock twice from the very - * same thread. We need to unregister the callback and abort the - * stream manually before we create a new console connection. - */ - virFDStreamSetInternalCloseCb(savedStream, NULL, NULL, NULL); - virStreamAbort(savedStream); - virHashRemoveEntry(cons->hash, pty); - /* continue adding a new stream connection */ - } - } - - /* create the lock file */ - if ((ret = virConsoleLockFileCreate(pty)) < 0) { - virMutexUnlock(&cons->lock); - return ret; - } - - /* obtain a reference to the stream */ - if (virStreamRef(st) < 0) { - virMutexUnlock(&cons->lock); - return -1; - } - - if (VIR_ALLOC(cbdata) < 0) { - virReportOOMError(); - goto error; - } - - if (virHashAddEntry(cons->hash, pty, st) < 0) - goto error; - - cbdata->cons = cons; - if (!(cbdata->pty = strdup(pty))) { - virReportOOMError(); - goto error; - } - - /* open the console pty */ - if (virFDStreamOpenFile(st, pty, 0, 0, O_RDWR) < 0) - goto error; - - /* add cleanup callback */ - virFDStreamSetInternalCloseCb(st, - virConsoleFDStreamCloseCb, - cbdata, - virConsoleFDStreamCloseCbFree); - - virMutexUnlock(&cons->lock); - return 0; - -error: - virStreamFree(st); - virHashRemoveEntry(cons->hash, pty); - if (cbdata) - VIR_FREE(cbdata->pty); - VIR_FREE(cbdata); - virMutexUnlock(&cons->lock); - return -1; -} diff --git a/src/conf/virconsole.h b/src/conf/virconsole.h deleted file mode 100644 index df9dfe8..0000000 --- a/src/conf/virconsole.h +++ /dev/null @@ -1,36 +0,0 @@ -/** - * virconsole.h: api to guarantee mutually exclusive - * access to domain's consoles - * - * Copyright (C) 2011-2012 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see - * <http://www.gnu.org/licenses/>. - * - * Author: Peter Krempa <pkrempa@redhat.com> - */ -#ifndef __VIR_CONSOLE_H__ -# define __VIR_CONSOLE_H__ - -# include "internal.h" - -typedef struct _virConsoles virConsoles; -typedef virConsoles *virConsolesPtr; - -virConsolesPtr virConsoleAlloc(void); -void virConsoleFree(virConsolesPtr cons); - -int virConsoleOpen(virConsolesPtr cons, const char *pty, - virStreamPtr st, bool force); -#endif /*__VIR_DOMAIN_CONSOLE_LOCK_H__*/ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 497d5d3..b09287d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1355,7 +1355,7 @@ virArchGetWordSize; virArchToString; -# virconsole.h +# virchrdev.h virConsoleAlloc; virConsoleFree; virConsoleOpen; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 00648cf..465dc55 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -31,7 +31,7 @@ # include "qemu_agent.h" # include "qemu_conf.h" # include "qemu_capabilities.h" -# include "virconsole.h" +# include "virchrdev.h" # define QEMU_EXPECTED_VIRT_TYPES \ ((1 << VIR_DOMAIN_VIRT_QEMU) | \ -- 1.7.11.7

The functionality provided in virchrdev.c (previously virconsole.c) is applicable to other types of character devices besides consoles, such as channels. This patch is just code motion, renaming things such as "console" or "pty", instead using more general terms such as "character device" or "device path". --- configure.ac | 48 +++++------ src/Makefile.am | 6 +- src/conf/virchrdev.c | 220 +++++++++++++++++++++++------------------------ src/conf/virchrdev.h | 18 ++-- src/libvirt_private.syms | 6 +- src/qemu/qemu_domain.c | 4 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 8 +- 8 files changed, 156 insertions(+), 156 deletions(-) diff --git a/configure.ac b/configure.ac index 3c97e4f..0f5d455 100644 --- a/configure.ac +++ b/configure.ac @@ -426,12 +426,12 @@ 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 +AC_ARG_WITH([chrdev-lock-files], + AC_HELP_STRING([--with-chrdev-lock-files], + [location for UUCP style lock files for character devices (use auto for default paths on some platforms) @<:@default=auto@:>@]), - [],[with_console_lock_files=auto]) + [],[with_chrdev_lock_files=auto]) AC_ARG_WITH([libssh2_transport], AC_HELP_STRING([--with-libssh2_transport], [libssh2 location @<:@default=check@:>@]),[],[with_libssh2_transport=check]) @@ -1431,25 +1431,25 @@ AM_CONDITIONAL([HAVE_AUDIT], [test "$with_audit" = "yes"]) AC_SUBST([AUDIT_CFLAGS]) AC_SUBST([AUDIT_LIBS]) -dnl UUCP style file locks for PTY consoles -if test "$with_console_lock_files" != "no"; then - case $with_console_lock_files in +dnl UUCP style file locks for character devices +if test "$with_chrdev_lock_files" != "no"; then + case $with_chrdev_lock_files in yes | auto) dnl Default locations for platforms, or disable if unknown if test "$with_linux" = "yes"; then - with_console_lock_files=/var/lock - elif test "$with_console_lock_files" = "auto"; then - with_console_lock_files=no + with_chrdev_lock_files=/var/lock + elif test "$with_chrdev_lock_files" = "auto"; then + with_chrdev_lock_files=no fi ;; esac - if test "$with_console_lock_files" = "yes"; then + if test "$with_chrdev_lock_files" = "yes"; then AC_MSG_ERROR([You must specify path for the lock files on this platform]) fi - AC_DEFINE_UNQUOTED([VIR_PTY_LOCK_FILE_PATH], "$with_console_lock_files", - [path to directory containing UUCP pty lock files]) + AC_DEFINE_UNQUOTED([VIR_CHRDEV_LOCK_FILE_PATH], "$with_chrdev_lock_files", + [path to directory containing UUCP device lock files]) fi -AM_CONDITIONAL([VIR_PTY_LOCK_FILE_PATH], [test "$with_console_lock_files" != "no"]) +AM_CONDITIONAL([VIR_CHRDEV_LOCK_FILE_PATH], [test "$with_chrdev_lock_files" != "no"]) dnl SELinux @@ -3287,16 +3287,16 @@ AC_MSG_NOTICE([ Alloc OOM: $enable_oom]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Miscellaneous]) AC_MSG_NOTICE([]) -AC_MSG_NOTICE([ Debug: $enable_debug]) -AC_MSG_NOTICE([ Use -Werror: $set_werror]) -AC_MSG_NOTICE([Warning Flags: $WARN_CFLAGS]) -AC_MSG_NOTICE([ Readline: $lv_use_readline]) -AC_MSG_NOTICE([ Python: $with_python]) -AC_MSG_NOTICE([ DTrace: $with_dtrace]) -AC_MSG_NOTICE([ numad: $with_numad]) -AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) -AC_MSG_NOTICE([ Init script: $with_init_script]) -AC_MSG_NOTICE([Console locks: $with_console_lock_files]) +AC_MSG_NOTICE([ Debug: $enable_debug]) +AC_MSG_NOTICE([ Use -Werror: $set_werror]) +AC_MSG_NOTICE([ Warning Flags: $WARN_CFLAGS]) +AC_MSG_NOTICE([ Readline: $lv_use_readline]) +AC_MSG_NOTICE([ Python: $with_python]) +AC_MSG_NOTICE([ DTrace: $with_dtrace]) +AC_MSG_NOTICE([ numad: $with_numad]) +AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) +AC_MSG_NOTICE([ Init script: $with_init_script]) +AC_MSG_NOTICE([Char device locks: $with_chrdev_lock_files]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Privileges]) AC_MSG_NOTICE([]) diff --git a/src/Makefile.am b/src/Makefile.am index 02bf47d..194a7a2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -244,8 +244,8 @@ ENCRYPTION_CONF_SOURCES = \ CPU_CONF_SOURCES = \ conf/cpu_conf.c conf/cpu_conf.h -# Safe console handling helper APIs -CONSOLE_CONF_SOURCES = \ +# Safe character device handling helper APIs +CHRDEV_CONF_SOURCES = \ conf/virchrdev.c conf/virchrdev.h # Device Helper APIs @@ -264,7 +264,7 @@ CONF_SOURCES = \ $(INTERFACE_CONF_SOURCES) \ $(SECRET_CONF_SOURCES) \ $(CPU_CONF_SOURCES) \ - $(CONSOLE_CONF_SOURCES) \ + $(CHRDEV_CONF_SOURCES) \ $(DEVICE_CONF_SOURCES) # The remote RPC driver, covering domains, storage, networks, etc diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 7b471ae..dcb064f 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -1,6 +1,6 @@ /** * virchrdev.c: api to guarantee mutually exclusive - * access to domain's consoles + * access to domain's character devices * * Copyright (C) 2011-2012 Red Hat, Inc. * @@ -40,47 +40,47 @@ #define VIR_FROM_THIS VIR_FROM_NONE -/* structure holding information about consoles +/* structure holding information about character devices * open in a given domain */ -struct _virConsoles { +struct _virChrdevs { virMutex lock; virHashTablePtr hash; }; -typedef struct _virConsoleStreamInfo virConsoleStreamInfo; -typedef virConsoleStreamInfo *virConsoleStreamInfoPtr; -struct _virConsoleStreamInfo { - virConsolesPtr cons; - const char *pty; +typedef struct _virChrdevStreamInfo virChrdevStreamInfo; +typedef virChrdevStreamInfo *virChrdevStreamInfoPtr; +struct _virChrdevStreamInfo { + virChrdevsPtr devs; + const char *path; }; -#ifdef VIR_PTY_LOCK_FILE_PATH +#ifdef VIR_CHRDEV_LOCK_FILE_PATH /** * Create a full filename with path to the lock file based on - * name/path of corresponding pty + * name/path of corresponding device * - * @pty path of the console device + * @dev path of the character device * * Returns a modified name that the caller has to free, or NULL * on error. */ -static char *virConsoleLockFilePath(const char *pty) +static char *virChrdevLockFilePath(const char *dev) { char *path = NULL; char *sanitizedPath = NULL; - char *ptyCopy; + char *devCopy; char *filename; char *p; - if (!(ptyCopy = strdup(pty))) { + if (!(devCopy = strdup(dev))) { virReportOOMError(); goto cleanup; } /* skip the leading "/dev/" */ - filename = STRSKIP(ptyCopy, "/dev"); + filename = STRSKIP(devCopy, "/dev"); if (!filename) - filename = ptyCopy; + filename = devCopy; /* substitute path forward slashes for underscores */ p = filename; @@ -90,26 +90,26 @@ static char *virConsoleLockFilePath(const char *pty) ++p; } - if (virAsprintf(&path, "%s/LCK..%s", VIR_PTY_LOCK_FILE_PATH, filename) < 0) + if (virAsprintf(&path, "%s/LCK..%s", VIR_CHRDEV_LOCK_FILE_PATH, filename) < 0) goto cleanup; sanitizedPath = virFileSanitizePath(path); cleanup: VIR_FREE(path); - VIR_FREE(ptyCopy); + VIR_FREE(devCopy); return sanitizedPath; } /** - * Verify and create a lock file for a console pty + * Verify and create a lock file for a character device * - * @pty Path of the console device + * @dev Path of the character device * * Returns 0 on success, -1 on error */ -static int virConsoleLockFileCreate(const char *pty) +static int virChrdevLockFileCreate(const char *dev) { char *path = NULL; int ret = -1; @@ -118,16 +118,16 @@ static int virConsoleLockFileCreate(const char *pty) pid_t pid; /* build lock file path */ - if (!(path = virConsoleLockFilePath(pty))) + if (!(path = virChrdevLockFilePath(dev))) goto cleanup; /* check if a log file and process holding the lock still exists */ if (virPidFileReadPathIfAlive(path, &pid, NULL) == 0 && pid >= 0) { /* the process exists, the lockfile is valid */ virReportError(VIR_ERR_OPERATION_FAILED, - _("Requested console pty '%s' is locked by " + _("Requested device '%s' is locked by " "lock file '%s' held by process %lld"), - pty, path, (long long) pid); + dev, path, (long long) pid); goto cleanup; } else { /* clean up the stale/corrupted/nonexistent lockfile */ @@ -148,15 +148,15 @@ static int virConsoleLockFileCreate(const char *pty) * we run in daemon mode and thus privileged. */ if (errno == EACCES && geteuid() != 0) { - VIR_DEBUG("Skipping lock file creation for pty '%s in path '%s'.", - pty, path); + VIR_DEBUG("Skipping lock file creation for device '%s in path '%s'.", + dev, path); ret = 0; goto cleanup; } virReportSystemError(errno, _("Couldn't create lock file for " - "pty '%s' in path '%s'"), - pty, path); + "device '%s' in path '%s'"), + dev, path); goto cleanup; } @@ -164,8 +164,8 @@ static int virConsoleLockFileCreate(const char *pty) if (safewrite(lockfd, pidStr, strlen(pidStr)) < 0) { virReportSystemError(errno, _("Couldn't write to lock file for " - "pty '%s' in path '%s'"), - pty, path); + "device '%s' in path '%s'"), + dev, path); VIR_FORCE_CLOSE(lockfd); unlink(path); goto cleanup; @@ -183,47 +183,47 @@ cleanup: } /** - * Remove a lock file for a pty + * Remove a lock file for a device * - * @pty Path of the pty device + * @dev Path of the device */ -static void virConsoleLockFileRemove(const char *pty) +static void virChrdevLockFileRemove(const char *dev) { - char *path = virConsoleLockFilePath(pty); + char *path = virChrdevLockFilePath(dev); if (path) unlink(path); VIR_FREE(path); } -#else /* #ifdef VIR_PTY_LOCK_FILE_PATH */ -/* file locking for console devices is disabled */ -static int virConsoleLockFileCreate(const char *pty ATTRIBUTE_UNUSED) +#else /* #ifdef VIR_CHRDEV_LOCK_FILE_PATH */ +/* file locking for character devices is disabled */ +static int virChrdevLockFileCreate(const char *dev ATTRIBUTE_UNUSED) { return 0; } -static void virConsoleLockFileRemove(const char *pty ATTRIBUTE_UNUSED) +static void virChrdevLockFileRemove(const char *dev ATTRIBUTE_UNUSED) { return; } -#endif /* #ifdef VIR_PTY_LOCK_FILE_PATH */ +#endif /* #ifdef VIR_CHRDEV_LOCK_FILE_PATH */ /** - * Frees an entry from the hash containing domain's active consoles + * Frees an entry from the hash containing domain's active devices * - * @data Opaque data, struct holding information about the console - * @name Path of the pty. + * @data Opaque data, struct holding information about the device + * @name Path of the device. */ -static void virConsoleHashEntryFree(void *data, +static void virChrdevHashEntryFree(void *data, const void *name) { - const char *pty = name; + const char *dev = name; virStreamPtr st = data; /* free stream reference */ virStreamFree(st); /* delete lock file */ - virConsoleLockFileRemove(pty); + virChrdevLockFileRemove(dev); } /** @@ -231,65 +231,65 @@ static void virConsoleHashEntryFree(void *data, * * @opaque Data to be freed. */ -static void virConsoleFDStreamCloseCbFree(void *opaque) +static void virChrdevFDStreamCloseCbFree(void *opaque) { - virConsoleStreamInfoPtr priv = opaque; + virChrdevStreamInfoPtr priv = opaque; - VIR_FREE(priv->pty); + VIR_FREE(priv->path); VIR_FREE(priv); } /** - * Callback being called if a FDstream is closed. Frees console entries + * Callback being called if a FDstream is closed. Frees device entries * from data structures and removes lockfiles. * * @st Pointer to stream being closed. - * @opaque Domain's console information structure. + * @opaque Domain's device information structure. */ -static void virConsoleFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, +static void virChrdevFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, void *opaque) { - virConsoleStreamInfoPtr priv = opaque; - virMutexLock(&priv->cons->lock); + virChrdevStreamInfoPtr priv = opaque; + virMutexLock(&priv->devs->lock); /* remove entry from hash */ - virHashRemoveEntry(priv->cons->hash, priv->pty); + virHashRemoveEntry(priv->devs->hash, priv->path); - virMutexUnlock(&priv->cons->lock); + virMutexUnlock(&priv->devs->lock); } /** - * Allocate structures for storing information about active console streams + * Allocate structures for storing information about active device streams * in domain's private data section. * * Returns pointer to the allocated structure or NULL on error */ -virConsolesPtr virConsoleAlloc(void) +virChrdevsPtr virChrdevAlloc(void) { - virConsolesPtr cons; - if (VIR_ALLOC(cons) < 0) + virChrdevsPtr devs; + if (VIR_ALLOC(devs) < 0) return NULL; - if (virMutexInit(&cons->lock) < 0) { - VIR_FREE(cons); + if (virMutexInit(&devs->lock) < 0) { + VIR_FREE(devs); return NULL; } - /* there will hardly be any consoles most of the time, the hash + /* there will hardly be any devices most of the time, the hash * does not have to be huge */ - if (!(cons->hash = virHashCreate(3, virConsoleHashEntryFree))) + if (!(devs->hash = virHashCreate(3, virChrdevHashEntryFree))) goto error; - return cons; + return devs; error: - virConsoleFree(cons); + virChrdevFree(devs); return NULL; } /** * Helper to clear stream callbacks when freeing the hash */ -static void virConsoleFreeClearCallbacks(void *payload, +static void virChrdevFreeClearCallbacks(void *payload, const void *name ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { @@ -299,80 +299,80 @@ static void virConsoleFreeClearCallbacks(void *payload, } /** - * Free structures for handling open console streams. + * Free structures for handling open device streams. * - * @cons Pointer to the private structure. + * @devs Pointer to the private structure. */ -void virConsoleFree(virConsolesPtr cons) +void virChrdevFree(virChrdevsPtr devs) { - if (!cons || !cons->hash) + if (!devs || !devs->hash) return; - virMutexLock(&cons->lock); - virHashForEach(cons->hash, virConsoleFreeClearCallbacks, NULL); - virHashFree(cons->hash); - virMutexUnlock(&cons->lock); - virMutexDestroy(&cons->lock); + virMutexLock(&devs->lock); + virHashForEach(devs->hash, virChrdevFreeClearCallbacks, NULL); + virHashFree(devs->hash); + virMutexUnlock(&devs->lock); + virMutexDestroy(&devs->lock); - VIR_FREE(cons); + VIR_FREE(devs); } /** - * Open a console stream for a domain ensuring that other streams are - * not using the console, nor any lockfiles exist. This ensures that - * the console stream does not get corrupted due to a race on reading + * Open a device stream for a domain ensuring that other streams are + * not using the device, nor any lockfiles exist. This ensures that + * the device stream does not get corrupted due to a race on reading * same FD by two processes. * - * @cons Pointer to private structure holding data about console streams. - * @pty Path to the pseudo tty to be opened. - * @st Stream the client wishes to use for the console connection. - * @force On true, close active console streams for the selected console pty - * before opening this connection. + * @devs Pointer to private structure holding data about device streams. + * @path Path to the character device to be opened. + * @st Stream the client wishes to use for the device connection. + * @force On true, close active device streams for the selected character + * device before opening this connection. * - * Returns 0 on success and st is connected to the selected pty and + * Returns 0 on success and st is connected to the selected device and * corresponding lock file is created (if configured). Returns -1 on - * error and 1 if the console stream is open and busy. + * error and 1 if the device stream is open and busy. */ -int virConsoleOpen(virConsolesPtr cons, - const char *pty, +int virChrdevOpen(virChrdevsPtr devs, + const char *path, virStreamPtr st, bool force) { - virConsoleStreamInfoPtr cbdata = NULL; + virChrdevStreamInfoPtr cbdata = NULL; virStreamPtr savedStream; int ret; - virMutexLock(&cons->lock); + virMutexLock(&devs->lock); - if ((savedStream = virHashLookup(cons->hash, pty))) { + if ((savedStream = virHashLookup(devs->hash, path))) { if (!force) { - /* entry found, console is busy */ - virMutexUnlock(&cons->lock); + /* entry found, device is busy */ + virMutexUnlock(&devs->lock); return 1; } else { /* terminate existing connection */ - /* The internal close callback handler needs to lock cons->lock to + /* The internal close callback handler needs to lock devs->lock to * remove the aborted stream from the hash. This would cause a * deadlock as we would try to enter the lock twice from the very * same thread. We need to unregister the callback and abort the - * stream manually before we create a new console connection. + * stream manually before we create a new device connection. */ virFDStreamSetInternalCloseCb(savedStream, NULL, NULL, NULL); virStreamAbort(savedStream); - virHashRemoveEntry(cons->hash, pty); + virHashRemoveEntry(devs->hash, path); /* continue adding a new stream connection */ } } /* create the lock file */ - if ((ret = virConsoleLockFileCreate(pty)) < 0) { - virMutexUnlock(&cons->lock); + if ((ret = virChrdevLockFileCreate(path)) < 0) { + virMutexUnlock(&devs->lock); return ret; } /* obtain a reference to the stream */ if (virStreamRef(st) < 0) { - virMutexUnlock(&cons->lock); + virMutexUnlock(&devs->lock); return -1; } @@ -381,34 +381,34 @@ int virConsoleOpen(virConsolesPtr cons, goto error; } - if (virHashAddEntry(cons->hash, pty, st) < 0) + if (virHashAddEntry(devs->hash, path, st) < 0) goto error; - cbdata->cons = cons; - if (!(cbdata->pty = strdup(pty))) { + cbdata->devs = devs; + if (!(cbdata->path = strdup(path))) { virReportOOMError(); goto error; } - /* open the console pty */ - if (virFDStreamOpenFile(st, pty, 0, 0, O_RDWR) < 0) + /* open the character device */ + if (virFDStreamOpenFile(st, path, 0, 0, O_RDWR) < 0) goto error; /* add cleanup callback */ virFDStreamSetInternalCloseCb(st, - virConsoleFDStreamCloseCb, + virChrdevFDStreamCloseCb, cbdata, - virConsoleFDStreamCloseCbFree); + virChrdevFDStreamCloseCbFree); - virMutexUnlock(&cons->lock); + virMutexUnlock(&devs->lock); return 0; error: virStreamFree(st); - virHashRemoveEntry(cons->hash, pty); + virHashRemoveEntry(devs->hash, path); if (cbdata) - VIR_FREE(cbdata->pty); + VIR_FREE(cbdata->path); VIR_FREE(cbdata); - virMutexUnlock(&cons->lock); + virMutexUnlock(&devs->lock); return -1; } diff --git a/src/conf/virchrdev.h b/src/conf/virchrdev.h index d5a926a..57d7576 100644 --- a/src/conf/virchrdev.h +++ b/src/conf/virchrdev.h @@ -1,6 +1,6 @@ /** * virchrdev.h: api to guarantee mutually exclusive - * access to domain's consoles + * access to domain's character devices * * Copyright (C) 2011-2012 Red Hat, Inc. * @@ -20,17 +20,17 @@ * * Author: Peter Krempa <pkrempa@redhat.com> */ -#ifndef __VIR_CONSOLE_H__ -# define __VIR_CONSOLE_H__ +#ifndef __VIR_CHRDEV_H__ +# define __VIR_CHRDEV_H__ # include "internal.h" -typedef struct _virConsoles virConsoles; -typedef virConsoles *virConsolesPtr; +typedef struct _virChrdevs virChrdevs; +typedef virChrdevs *virChrdevsPtr; -virConsolesPtr virConsoleAlloc(void); -void virConsoleFree(virConsolesPtr cons); +virChrdevsPtr virChrdevAlloc(void); +void virChrdevFree(virChrdevsPtr devs); -int virConsoleOpen(virConsolesPtr cons, const char *pty, +int virChrdevOpen(virChrdevsPtr devs, const char *path, virStreamPtr st, bool force); -#endif /*__VIR_DOMAIN_CONSOLE_LOCK_H__*/ +#endif /*__VIR_CHRDEV_H__*/ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b09287d..5636661 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1356,9 +1356,9 @@ virArchToString; # virchrdev.h -virConsoleAlloc; -virConsoleFree; -virConsoleOpen; +virChrdevAlloc; +virChrdevFree; +virChrdevOpen; # virdbus.h diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5d11f83..08774b9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -215,7 +215,7 @@ static void *qemuDomainObjPrivateAlloc(void) if (qemuDomainObjInitJob(priv) < 0) goto error; - if (!(priv->cons = virConsoleAlloc())) + if (!(priv->devs = virChrdevAlloc())) goto error; priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; @@ -240,7 +240,7 @@ static void qemuDomainObjPrivateFree(void *data) VIR_FREE(priv->lockState); VIR_FREE(priv->origname); - virConsoleFree(priv->cons); + virChrdevFree(priv->devs); /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 465dc55..68cf295 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -155,7 +155,7 @@ struct _qemuDomainObjPrivate { unsigned long migMaxBandwidth; char *origname; - virConsolesPtr cons; + virChrdevsPtr devs; qemuDomainCleanupCallback *cleanupCallbacks; size_t ncleanupCallbacks; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b8bb36b..992af02 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12576,10 +12576,10 @@ qemuDomainOpenConsole(virDomainPtr dom, } /* handle mutually exclusive access to console devices */ - ret = virConsoleOpen(priv->cons, - chr->source.data.file.path, - st, - (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0); + ret = virChrdevOpen(priv->devs, + chr->source.data.file.path, + st, + (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0); if (ret == 1) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", -- 1.7.11.7

This also changes the function signature to take a virDomainChrSourceDefPtr instead of just a path, since it needs to differentiate behavior based on source->type. --- src/conf/virchrdev.c | 38 +++++++++++++++++++++++++++++++++----- src/conf/virchrdev.h | 5 +++-- src/qemu/qemu_driver.c | 2 +- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index dcb064f..1802324 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -324,7 +324,7 @@ void virChrdevFree(virChrdevsPtr devs) * same FD by two processes. * * @devs Pointer to private structure holding data about device streams. - * @path Path to the character device to be opened. + * @source Pointer to private structure holding data about device source. * @st Stream the client wishes to use for the device connection. * @force On true, close active device streams for the selected character * device before opening this connection. @@ -334,14 +334,29 @@ void virChrdevFree(virChrdevsPtr devs) * error and 1 if the device stream is open and busy. */ int virChrdevOpen(virChrdevsPtr devs, - const char *path, - virStreamPtr st, - bool force) + virDomainChrSourceDefPtr source, + virStreamPtr st, + bool force) { virChrdevStreamInfoPtr cbdata = NULL; virStreamPtr savedStream; + const char *path; int ret; + switch (source->type) { + case VIR_DOMAIN_CHR_TYPE_PTY: + path = source->data.file.path; + break; + case VIR_DOMAIN_CHR_TYPE_UNIX: + path = source->data.nix.path; + break; + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported device type '%s'"), + virDomainChrTypeToString(source->type)); + return -1; + } + virMutexLock(&devs->lock); if ((savedStream = virHashLookup(devs->hash, path))) { @@ -391,8 +406,21 @@ int virChrdevOpen(virChrdevsPtr devs, } /* open the character device */ - if (virFDStreamOpenFile(st, path, 0, 0, O_RDWR) < 0) + switch (source->type) { + case VIR_DOMAIN_CHR_TYPE_PTY: + if (virFDStreamOpenFile(st, path, 0, 0, O_RDWR) < 0) + goto error; + break; + case VIR_DOMAIN_CHR_TYPE_UNIX: + if (virFDStreamConnectUNIX(st, path, false) < 0) + goto error; + break; + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported device type '%s'"), + virDomainChrTypeToString(source->type)); goto error; + } /* add cleanup callback */ virFDStreamSetInternalCloseCb(st, diff --git a/src/conf/virchrdev.h b/src/conf/virchrdev.h index 57d7576..e1990e8 100644 --- a/src/conf/virchrdev.h +++ b/src/conf/virchrdev.h @@ -24,6 +24,7 @@ # define __VIR_CHRDEV_H__ # include "internal.h" +# include "domain_conf.h" typedef struct _virChrdevs virChrdevs; typedef virChrdevs *virChrdevsPtr; @@ -31,6 +32,6 @@ typedef virChrdevs *virChrdevsPtr; virChrdevsPtr virChrdevAlloc(void); void virChrdevFree(virChrdevsPtr devs); -int virChrdevOpen(virChrdevsPtr devs, const char *path, - virStreamPtr st, bool force); +int virChrdevOpen(virChrdevsPtr devs, virDomainChrSourceDefPtr source, + virStreamPtr st, bool force); #endif /*__VIR_CHRDEV_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 992af02..20e2001 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12577,7 +12577,7 @@ qemuDomainOpenConsole(virDomainPtr dom, /* handle mutually exclusive access to console devices */ ret = virChrdevOpen(priv->devs, - chr->source.data.file.path, + &chr->source, st, (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0); -- 1.7.11.7

--- v2: correct version comment to 1.0.2 src/qemu/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 20e2001..cdbe30d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12593,6 +12593,78 @@ cleanup: return ret; } +static int +qemuDomainOpenChannel(virDomainPtr dom, + const char *name, + virStreamPtr st, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int ret = -1; + int i; + virDomainChrDefPtr chr = NULL; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(VIR_DOMAIN_CHANNEL_FORCE, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (name) { + for (i = 0 ; !chr && i < vm->def->nchannels ; i++) { + if (STREQ(name, vm->def->channels[i]->info.alias)) + chr = vm->def->channels[i]; + + if (vm->def->channels[i]->targetType == \ + VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + STREQ(name, vm->def->channels[i]->target.name)) + chr = vm->def->channels[i]; + } + } else { + if (vm->def->nchannels) + chr = vm->def->channels[0]; + } + + if (!chr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find channel %s"), + NULLSTR(name)); + goto cleanup; + } + + if (chr->source.type != VIR_DOMAIN_CHR_TYPE_UNIX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("channel %s is not using a UNIX socket"), + NULLSTR(name)); + goto cleanup; + } + + /* handle mutually exclusive access to channel devices */ + ret = virChrdevOpen(priv->devs, + &chr->source, + st, + (flags & VIR_DOMAIN_CHANNEL_FORCE) != 0); + + if (ret == 1) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Active channel stream exists for this domain")); + ret = -1; + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static char * qemuDiskPathToAlias(virDomainObjPtr vm, const char *path, int *idx) { @@ -14694,6 +14766,7 @@ static virDriver qemuDriver = { .nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */ .domainFSTrim = qemuDomainFSTrim, /* 1.0.1 */ + .domainOpenChannel = qemuDomainOpenChannel, /* 1.0.2 */ }; -- 1.7.11.7

On 01/02/2013 08:38 AM, John Eckersberg wrote:
This series enables the qemu driver to tunnel a virtio channel. This is useful for a remote session to communicate with a guest channel via the streaming API.
This was originally fleshed out a while back in this thread: https://www.redhat.com/archives/libvir-list/2011-September/msg01049.html
This implements only item (3) in that list.
The new API is nearly identical to the existing virDomainOpenConsole API, except it works on channels, and supports UNIX sockets in addition to PTYs for channel source type.
This is my first libvirt patch, please be gentle :)
--- v2: - Rebased against master - Correct version to 1.0.2 in 5/5
You already had ACKs on v1, so I've pushed the series. Congratulations on your first libvirt patch :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
John Eckersberg
-
Michal Privoznik