diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c47998aabd..01388b44ef 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4967,86 +4967,6 @@ qemuBuildSCSIHostdevDevProps(const virDomainDef *def, return g_steal_pointer(&props); } -static int -qemuBuildChrChardevFileStr(virLogManager *logManager, - virSecurityManager *secManager, - virQEMUDriverConfig *cfg, - virQEMUCaps *qemuCaps, - const virDomainDef *def, - virCommand *cmd, - virBuffer *buf, - const char *filearg, const char *fileval, - const char *appendarg, int appendval) -{ - /* Technically, to pass an FD via /dev/fdset we don't need - * any capability check because X_QEMU_CAPS_ADD_FD is already - * assumed. But keeping the old style is still handy when - * building a standalone command line (e.g. for tests). */ - if (logManager || - virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) { - g_autofree char *fdset = NULL; - int logfd; - size_t idx; - - if (logManager) { - int flags = 0; - - if (appendval == VIR_TRISTATE_SWITCH_ABSENT || - appendval == VIR_TRISTATE_SWITCH_OFF) - flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; - - if ((logfd = virLogManagerDomainOpenLogFile(logManager, - "qemu", - def->uuid, - def->name, - fileval, - flags, - NULL, NULL)) < 0) - return -1; - } else { - int oflags = O_CREAT | O_WRONLY; - - switch (appendval) { - case VIR_TRISTATE_SWITCH_ABSENT: - case VIR_TRISTATE_SWITCH_OFF: - oflags |= O_TRUNC; - break; - case VIR_TRISTATE_SWITCH_ON: - oflags |= O_APPEND; - break; - case VIR_TRISTATE_SWITCH_LAST: - break; - } - - if ((logfd = qemuDomainOpenFile(cfg, def, fileval, oflags, NULL)) < 0) - return -1; - - if (qemuSecuritySetImageFDLabel(secManager, (virDomainDef*)def, logfd) < 0) { - VIR_FORCE_CLOSE(logfd); - return -1; - } - } - - virCommandPassFDIndex(cmd, logfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT, &idx); - fdset = qemuBuildFDSet(logfd, idx); - - virCommandAddArg(cmd, "-add-fd"); - virCommandAddArg(cmd, fdset); - - virBufferAsprintf(buf, ",%s=/dev/fdset/%zu,%s=on", filearg, idx, appendarg); - } else { - virBufferAsprintf(buf, ",%s=", filearg); - virQEMUBuildBufferEscapeComma(buf, fileval); - if (appendval != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(buf, ",%s=%s", appendarg, - virTristateSwitchTypeToString(appendval)); - } - } - - return 0; -} - - static void qemuBuildChrChardevReconnectStr(virBuffer *buf, const virDomainChrSourceReconnectDef *def) @@ -5125,11 +5045,11 @@ enum { /* This function outputs a -chardev command line option which describes only the * host side of the character device */ static char * -qemuBuildChrChardevStr(virLogManager *logManager, - virSecurityManager *secManager, +qemuBuildChrChardevStr(virLogManager *logManager G_GNUC_UNUSED, + virSecurityManager *secManager G_GNUC_UNUSED, virCommand *cmd, virQEMUDriverConfig *cfg, - const virDomainDef *def, + const virDomainDef *def G_GNUC_UNUSED, const virDomainChrSourceDef *dev, const char *alias, virQEMUCaps *qemuCaps, @@ -5166,12 +5086,27 @@ qemuBuildChrChardevStr(virLogManager *logManager, case VIR_DOMAIN_CHR_TYPE_FILE: virBufferAsprintf(&buf, "file,id=%s", charAlias); - if (qemuBuildChrChardevFileStr(cdevflags & QEMU_BUILD_CHARDEV_FILE_LOGD ? - logManager : NULL, - secManager, cfg, qemuCaps, def, cmd, &buf, - "path", dev->data.file.path, - "append", dev->data.file.append) < 0) - return NULL; + if (chrSourcePriv->fd != -1) { + g_autofree char *fdset = NULL; + size_t idx; + + virCommandPassFDIndex(cmd, chrSourcePriv->fd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT, &idx); + fdset = qemuBuildFDSet(chrSourcePriv->fd, idx); + chrSourcePriv->fd = -1; + + virCommandAddArg(cmd, "-add-fd"); + virCommandAddArg(cmd, fdset); + + virBufferAsprintf(&buf, ",path=/dev/fdset/%zu,append=on", idx); + } else { + virBufferAddLit(&buf, ",path="); + virQEMUBuildBufferEscapeComma(&buf, dev->data.file.path); + if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&buf, ",append=%s", + virTristateSwitchTypeToString(dev->data.file.append)); + } + } break; case VIR_DOMAIN_CHR_TYPE_PIPE: @@ -5255,24 +5190,11 @@ qemuBuildChrChardevStr(virLogManager *logManager, case VIR_DOMAIN_CHR_TYPE_UNIX: virBufferAsprintf(&buf, "socket,id=%s", charAlias); - if (dev->data.nix.listen && - (cdevflags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) { - int fd; + if (chrSourcePriv->fd != -1) { + virBufferAsprintf(&buf, ",fd=%d", chrSourcePriv->fd); - if (qemuSecuritySetSocketLabel(secManager, (virDomainDef *)def) < 0) - return NULL; - fd = qemuOpenChrChardevUNIXSocket(dev); - if (qemuSecurityClearSocketLabel(secManager, (virDomainDef *)def) < 0) { - VIR_FORCE_CLOSE(fd); - return NULL; - } - if (fd < 0) - return NULL; - - virBufferAsprintf(&buf, ",fd=%d", fd); - - virCommandPassFD(cmd, fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + virCommandPassFD(cmd, chrSourcePriv->fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + chrSourcePriv->fd = -1; } else { virBufferAddLit(&buf, ",path="); virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path); @@ -5306,11 +5228,27 @@ qemuBuildChrChardevStr(virLogManager *logManager, } if (dev->logfile) { - if (qemuBuildChrChardevFileStr(logManager, secManager, cfg, - qemuCaps, def, cmd, &buf, - "logfile", dev->logfile, - "logappend", dev->logappend) < 0) - return NULL; + if (chrSourcePriv->logfd != -1) { + g_autofree char *fdset = NULL; + size_t idx; + + virCommandPassFDIndex(cmd, chrSourcePriv->logfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT, &idx); + fdset = qemuBuildFDSet(chrSourcePriv->logfd, idx); + chrSourcePriv->logfd = -1; + + virCommandAddArg(cmd, "-add-fd"); + virCommandAddArg(cmd, fdset); + + virBufferAsprintf(&buf, ",logfile=/dev/fdset/%zu,logappend=on", idx); + } else { + virBufferAddLit(&buf, ",logfile="); + virQEMUBuildBufferEscapeComma(&buf, dev->logfile); + if (dev->logappend != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&buf, ",logappend=%s", + virTristateSwitchTypeToString(dev->logappend)); + } + } } return virBufferContentAndReset(&buf); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4f0d266c73..cbaa39b618 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -850,6 +850,9 @@ qemuDomainChrSourcePrivateNew(void) if (!(priv = virObjectNew(qemuDomainChrSourcePrivateClass))) return NULL; + priv->fd = -1; + priv->logfd = -1; + return (virObject *) priv; } @@ -859,6 +862,9 @@ qemuDomainChrSourcePrivateDispose(void *obj) { qemuDomainChrSourcePrivate *priv = obj; + VIR_FORCE_CLOSE(priv->fd); + VIR_FORCE_CLOSE(priv->logfd); + g_clear_pointer(&priv->secinfo, qemuDomainSecretInfoFree); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3de3f70b94..61704fdae7 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -341,6 +341,9 @@ struct _qemuDomainChrSourcePrivate { /* for char devices using secret * NB: *not* to be written to qemu domain object XML */ qemuDomainSecretInfo *secinfo; + + int fd; /* file descriptor of the chardev source */ + int logfd; /* file descriptor of the logging source */ }; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 804205131a..b8cbba4f3f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -98,6 +98,9 @@ #include "storage_source.h" #include "backup_conf.h" +#include "logging/log_manager.h" +#include "logging/log_protocol.h" + #define VIR_FROM_THIS VIR_FROM_QEMU VIR_LOG_INIT("qemu.qemu_process"); @@ -3069,29 +3072,6 @@ qemuProcessInitPasswords(virQEMUDriver *driver, } -static int -qemuProcessPrepareChardevDevice(virDomainDef *def G_GNUC_UNUSED, - virDomainChrDef *dev, - void *opaque G_GNUC_UNUSED) -{ - int fd; - if (dev->source->type != VIR_DOMAIN_CHR_TYPE_FILE) - return 0; - - if ((fd = open(dev->source->data.file.path, - O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) { - virReportSystemError(errno, - _("Unable to pre-create chardev file '%s'"), - dev->source->data.file.path); - return -1; - } - - VIR_FORCE_CLOSE(fd); - - return 0; -} - - static int qemuProcessCleanupChardevDevice(virDomainDef *def G_GNUC_UNUSED, virDomainChrDef *dev, @@ -6802,6 +6782,200 @@ qemuProcessOpenVhostVsock(virDomainVsockDef *vsock) } +static int +qemuProcessPrepareHostBackendChardevFileHelper(const char *path, + virTristateSwitch append, + int *fd, + virLogManager *logManager, + virSecurityManager *secManager, + virQEMUCaps *qemuCaps, + virQEMUDriverConfig *cfg, + const virDomainDef *def) +{ + /* Technically, to pass an FD via /dev/fdset we don't need + * any capability check because X_QEMU_CAPS_ADD_FD is already + * assumed. But keeping the old style is still handy when + * building a standalone command line (e.g. for tests). */ + if (!logManager && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) + return 0; + + if (logManager) { + int flags = 0; + + if (append == VIR_TRISTATE_SWITCH_ABSENT || + append == VIR_TRISTATE_SWITCH_OFF) + flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; + + if ((*fd = virLogManagerDomainOpenLogFile(logManager, + "qemu", + def->uuid, + def->name, + path, + flags, + NULL, NULL)) < 0) + return -1; + } else { + int oflags = O_CREAT | O_WRONLY; + + switch (append) { + case VIR_TRISTATE_SWITCH_ABSENT: + case VIR_TRISTATE_SWITCH_OFF: + oflags |= O_TRUNC; + break; + case VIR_TRISTATE_SWITCH_ON: + oflags |= O_APPEND; + break; + case VIR_TRISTATE_SWITCH_LAST: + break; + } + + if ((*fd = qemuDomainOpenFile(cfg, def, path, oflags, NULL)) < 0) + return -1; + + if (qemuSecuritySetImageFDLabel(secManager, (virDomainDef*)def, *fd) < 0) { + VIR_FORCE_CLOSE(*fd); + return -1; + } + } + + return 0; +} + + +struct qemuProcessPrepareHostBackendChardevData { + virQEMUCaps *qemuCaps; + virLogManager *logManager; + virSecurityManager *secManager; + virQEMUDriverConfig *cfg; + virDomainDef *def; +}; + + +static int +qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev, + virDomainChrSourceDef *chardev, + void *opaque) +{ + struct qemuProcessPrepareHostBackendChardevData *data = opaque; + qemuDomainChrSourcePrivate *charpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chardev); + + /* this function is also called for the monitor backend which doesn't have + * a 'dev' */ + if (dev) { + if (dev->type == VIR_DOMAIN_DEVICE_NET) { + /* due to a historical bug in qemu we don't use FD passtrhough for + * vhost-sockets for network devices */ + return 0; + } + } + + switch ((virDomainChrType) chardev->type) { + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + break; + + case VIR_DOMAIN_CHR_TYPE_FILE: + if (qemuProcessPrepareHostBackendChardevFileHelper(chardev->data.file.path, + chardev->data.file.append, + &charpriv->fd, + data->logManager, + data->secManager, + data->qemuCaps, + data->cfg, + data->def) < 0) + return -1; + + break; + + case VIR_DOMAIN_CHR_TYPE_UNIX: + if (chardev->data.nix.listen && + virQEMUCapsGet(data->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) { + + if (qemuSecuritySetSocketLabel(data->secManager, data->def) < 0) + return -1; + + charpriv->fd = qemuOpenChrChardevUNIXSocket(chardev); + + if (qemuSecurityClearSocketLabel(data->secManager, data->def) < 0) { + VIR_FORCE_CLOSE(charpriv->fd); + return -1; + } + + if (charpriv->fd < 0) + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_LAST: + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported chardev '%s'"), + virDomainChrTypeToString(chardev->type)); + return -1; + } + + if (chardev->logfile) { + if (qemuProcessPrepareHostBackendChardevFileHelper(chardev->logfile, + chardev->logappend, + &charpriv->logfd, + data->logManager, + data->secManager, + data->qemuCaps, + data->cfg, + data->def) < 0) + return -1; + } + + return 0; +} + + +/* prepare the chardev backends for various devices: + * serial/parallel/channel chardevs, vhost-user disks, vhost-user network + * interfaces, smartcards, shared memory, and redirdevs */ +static int +qemuProcessPrepareHostBackendChardev(virDomainObj *vm, + virQEMUCaps *qemuCaps, + virSecurityManager *secManager) +{ + qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver); + struct qemuProcessPrepareHostBackendChardevData data = { + .qemuCaps = qemuCaps, + .logManager = NULL, + .secManager = secManager, + .cfg = cfg, + .def = vm->def, + }; + g_autoptr(virLogManager) logManager = NULL; + + if (cfg->stdioLogD) { + if (!(logManager = data.logManager = virLogManagerNew(priv->driver->privileged))) + return -1; + } + + if (qemuDomainDeviceBackendChardevForeach(vm->def, + qemuProcessPrepareHostBackendChardevOne, + &data) < 0) + return -1; + + if (qemuProcessPrepareHostBackendChardevOne(NULL, priv->monConfig, &data) < 0) + return -1; + + return 0; +} + + /** * qemuProcessPrepareHost: * @driver: qemu driver @@ -6848,11 +7022,10 @@ qemuProcessPrepareHost(virQEMUDriver *driver, hostdev_flags) < 0) return -1; - VIR_DEBUG("Preparing chr devices"); - if (virDomainChrDefForeach(vm->def, - true, - qemuProcessPrepareChardevDevice, - NULL) < 0) + VIR_DEBUG("Preparing chr device backends"); + if (qemuProcessPrepareHostBackendChardev(vm, + priv->qemuCaps, + driver->securityManager) < 0) return -1; if (qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, true) < 0) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5e4cd7389c..9cece1df91 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -376,6 +376,71 @@ testCheckExclusiveFlags(int flags) } +static int +testPrepareHostBackendChardevOne(virDomainDeviceDef *dev, + virDomainChrSourceDef *chardev, + void *opaque) +{ + virQEMUCaps *qemuCaps = opaque; + qemuDomainChrSourcePrivate *charpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chardev); + + if (dev) { + if (dev->type == VIR_DOMAIN_DEVICE_NET) { + /* due to a historical bug in qemu we don't use FD passtrhough for + * vhost-sockets for network devices */ + return 0; + } + } + + switch ((virDomainChrType) chardev->type) { + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + break; + + case VIR_DOMAIN_CHR_TYPE_FILE: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) { + if (fcntl(1750, F_GETFD) != -1) + abort(); + charpriv->fd = 1750; + } + break; + + case VIR_DOMAIN_CHR_TYPE_UNIX: + if (chardev->data.nix.listen && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) { + + if (fcntl(1729, F_GETFD) != -1) + abort(); + + charpriv->fd = 1729; + } + break; + + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_LAST: + break; + } + + if (chardev->logfile) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) { + if (fcntl(1751, F_GETFD) != -1) + abort(); + charpriv->logfd = 1751; + } + } + + return 0; +} + + static virCommand * testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, virDomainObj *vm, @@ -391,6 +456,20 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, VIR_QEMU_PROCESS_START_COLD) < 0) return NULL; + if (qemuDomainDeviceBackendChardevForeach(vm->def, + testPrepareHostBackendChardevOne, + info->qemuCaps) < 0) + return NULL; + + if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) { + qemuDomainChrSourcePrivate *monpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(priv->monConfig); + + if (fcntl(1729, F_GETFD) != -1) + abort(); + + monpriv->fd = 1729; + } + for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDef *disk = vm->def->disks[i];