External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-11 04:15:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

qemu: Move creation and opening of chardev backend FDs to host prepare step

Browse Source 
The opening of files for FD passing for a chardev backend was
historically done in the function which is formatting the commandline.

This has multiple problems. Firstly the function takes a lot of
parameters which need to be passed through the commandline formatters.
This made the 'qemuBuildChrChardevStr' extremely unappealing to the
extent that we have multiple other custom formatters in places which
didn't really want to use the function.

Additionally the function is also creating files in the host in certain
configurations which is wrong for a commandline formatter to do. This
meant that e.g. not all chardev test cases can be converted to use
DO_TEST_CAPS_LATEST as we attempt to use such code path and attempt to
create files outside of the test directory.

This patch moves the opening of the filedescriptors from
'qemuBuildChrChardevFileStr' into a new helper
'qemuProcessPrepareHostBackendChardevOne' which is called using
'qemuDomainDeviceBackendChardevForeach'.

To preserve test behaviour we also have another instance
'testPrepareHostBackendChardevOne' which is populating mock
filedescriptors.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is contained in:
Peter Krempa 2021-10-25 12:42:16 +02:00
parent 728f0079ea
commit ff024b60cc
5 changed files with 338 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

160
src/qemu/qemu_command.c
View File

@ -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);

6
src/qemu/qemu_domain.c
View File

@ -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);
}

3
src/qemu/qemu_domain.h
View File

@ -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 */
};

229
src/qemu/qemu_process.c
View File

@ -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)

79
tests/qemuxml2argvtest.c
View File

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