External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-06 09:55:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

qemu: Rewrite chardev startup code to use qemuFDPass

Browse Source 
Rewrite the parts which already pass FDs via fdset or directly to use
the new infrastructure.

Apart from simpler code this also adds the appropriate names to the fds
in the fdsets which will allow us to properly remove the fdsets won
hot-unplug of chardevs, which we didn't do for now and resulted in
leaking the FDs.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is contained in:
Peter Krempa 2022-02-02 17:31:29 +01:00
parent 38edcca114
commit 6d161bcc60
14 changed files with 102 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/qemu/qemu_command.c
View File

@ -301,23 +301,6 @@ qemuBuildMasterKeyCommandLine(virCommand *cmd,
}
/**
* qemuBuildFDSet:
* @fd: fd to reassign to the child
* @idx: index in the fd set
*
* Format the parameters for the -add-fd command line option
* for the given file descriptor. The file descriptor must previously
* have been 'transferred' in a virCommandPassFDIndex() call,
* and @idx is the value returned by that call.
*/
static char *
qemuBuildFDSet(int fd, size_t idx)
{
return g_strdup_printf("set=%zu,fd=%d", idx, fd);
}
/**
* qemuVirCommandGetFDSet:
* @cmd: the command to modify
@ -1362,8 +1345,8 @@ qemuBuildChardevStr(const virDomainChrSourceDef *dev,
path = dev->data.file.path;
append = dev->data.file.append;
if (chrSourcePriv->fdset) {
path = chrSourcePriv->fdset;
if (chrSourcePriv->sourcefd) {
path = qemuFDPassGetPath(chrSourcePriv->sourcefd);
append = VIR_TRISTATE_SWITCH_ON;
}
@ -1430,8 +1413,8 @@ qemuBuildChardevStr(const virDomainChrSourceDef *dev,
case VIR_DOMAIN_CHR_TYPE_UNIX:
virBufferAsprintf(&buf, "socket,id=%s", charAlias);
if (chrSourcePriv->passedFD != -1) {
virBufferAsprintf(&buf, ",fd=%d", chrSourcePriv->passedFD);
if (chrSourcePriv->sourcefd) {
virBufferAsprintf(&buf, ",fd=%s", qemuFDPassGetPath(chrSourcePriv->sourcefd));
} else {
virBufferAddLit(&buf, ",path=");
virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
@ -1466,8 +1449,8 @@ qemuBuildChardevStr(const virDomainChrSourceDef *dev,
path = dev->logfile;
append = dev->logappend;
if (chrSourcePriv->logFdset) {
path = chrSourcePriv->logFdset;
if (chrSourcePriv->logfd) {
path = qemuFDPassGetPath(chrSourcePriv->logfd);
append = VIR_TRISTATE_SWITCH_ON;
}
@ -1527,28 +1510,8 @@ qemuBuildChardevCommand(virCommand *cmd,
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
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);
chrSourcePriv->fdset = g_strdup_printf("/dev/fdset/%zu", idx);
}
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
if (chrSourcePriv->fd != -1) {
virCommandPassFD(cmd, chrSourcePriv->fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
chrSourcePriv->passedFD = chrSourcePriv->fd;
chrSourcePriv->fd = -1;
}
qemuFDPassTransferCommand(chrSourcePriv->sourcefd, cmd);
break;
case VIR_DOMAIN_CHR_TYPE_NULL:
@ -1571,20 +1534,7 @@ qemuBuildChardevCommand(virCommand *cmd,
return -1;
}
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);
chrSourcePriv->logFdset = g_strdup_printf("/dev/fdset/%zu", idx);
}
qemuFDPassTransferCommand(chrSourcePriv->logfd, cmd);
if (!(charstr = qemuBuildChardevStr(dev, charAlias)))
return -1;
@ -5001,8 +4951,12 @@ qemuBuildVideoCommandLine(virCommand *cmd,
qemuDomainChrSourcePrivate *chrsrcpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chrsrc);
chrsrc->type = VIR_DOMAIN_CHR_TYPE_UNIX;
chrsrcpriv->fd = videopriv->vhost_user_fd;
videopriv->vhost_user_fd = -1;
chrsrcpriv->sourcefd = qemuFDPassNewDirect(video->info.alias, priv);
if (qemuFDPassAddFD(chrsrcpriv->sourcefd,
&videopriv->vhost_user_fd,
"-vhost-user") < 0)
return -1;
if (qemuBuildChardevCommand(cmd, chrsrc, chrAlias, priv->qemuCaps) < 0)
return -1;

11
src/qemu/qemu_domain.c
View File

@ -904,11 +904,6 @@ qemuDomainChrSourcePrivateNew(void)
if (!(priv = virObjectNew(qemuDomainChrSourcePrivateClass)))
return NULL;
priv->fd = -1;
priv->logfd = -1;
priv->passedFD = -1;
return (virObject *) priv;
}
@ -918,13 +913,11 @@ qemuDomainChrSourcePrivateDispose(void *obj)
{
qemuDomainChrSourcePrivate *priv = obj;
VIR_FORCE_CLOSE(priv->fd);
VIR_FORCE_CLOSE(priv->logfd);
qemuFDPassFree(priv->sourcefd);
qemuFDPassFree(priv->logfd);
g_free(priv->tlsCertPath);
g_free(priv->fdset);
g_free(priv->logFdset);
g_free(priv->tlsCredsAlias);
g_clear_pointer(&priv->secinfo, qemuDomainSecretInfoFree);

8
src/qemu/qemu_domain.h
View File

@ -36,6 +36,7 @@
#include "qemu_capabilities.h"
#include "qemu_migration_params.h"
#include "qemu_slirp.h"
#include "qemu_fd.h"
#include "virmdev.h"
#include "virchrdev.h"
#include "virobject.h"
@ -347,16 +348,13 @@ struct _qemuDomainChrSourcePrivate {
* 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 */
qemuFDPass *sourcefd;
qemuFDPass *logfd;
bool wait; /* wait for incoming connections on chardev */
char *tlsCertPath; /* path to certificates if TLS is requested */
bool tlsVerify; /* whether server should verify client certificates */
char *fdset; /* fdset path corresponding to the passed filedescriptor */
char *logFdset; /* fdset path corresponding to the passed filedescriptor for logfile */
int passedFD; /* filedescriptor number when fdset passing it directly */
char *tlsCredsAlias; /* alias of the x509 tls credentials object */
};

41
src/qemu/qemu_process.c
View File

@ -6781,6 +6781,7 @@ struct qemuProcessPrepareHostBackendChardevData {
virLogManager *logManager;
virQEMUDriverConfig *cfg;
virDomainDef *def;
const char *fdprefix;
};
@ -6791,10 +6792,14 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
{
struct qemuProcessPrepareHostBackendChardevData *data = opaque;
qemuDomainChrSourcePrivate *charpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chardev);
const char *devalias = NULL;
/* this function is also called for the monitor backend which doesn't have
* a 'dev' */
if (dev) {
virDomainDeviceInfo *info = virDomainDeviceGetInfo(dev);
devalias = info->alias;
/* vhost-user disk doesn't use FD passing */
if (dev->type == VIR_DOMAIN_DEVICE_DISK)
return 0;
@ -6808,6 +6813,8 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
/* TPMs FD passing setup is special and handled separately */
if (dev->type == VIR_DOMAIN_DEVICE_TPM)
return 0;
} else {
devalias = data->fdprefix;
}
switch ((virDomainChrType) chardev->type) {
@ -6823,33 +6830,42 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
case VIR_DOMAIN_CHR_TYPE_FILE: {
VIR_AUTOCLOSE sourcefd = -1;
if (qemuProcessPrepareHostBackendChardevFileHelper(chardev->data.file.path,
chardev->data.file.append,
&charpriv->fd,
&sourcefd,
data->logManager,
data->priv->driver->securityManager,
data->cfg,
data->def) < 0)
return -1;
charpriv->sourcefd = qemuFDPassNew(devalias, data->priv);
if (qemuFDPassAddFD(charpriv->sourcefd, &sourcefd, "-source") < 0)
return -1;
}
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
if (chardev->data.nix.listen &&
virQEMUCapsGet(data->priv->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) {
VIR_AUTOCLOSE sourcefd = -1;
if (qemuSecuritySetSocketLabel(data->priv->driver->securityManager, data->def) < 0)
return -1;
charpriv->fd = qemuOpenChrChardevUNIXSocket(chardev);
sourcefd = qemuOpenChrChardevUNIXSocket(chardev);
if (qemuSecurityClearSocketLabel(data->priv->driver->securityManager, data->def) < 0) {
VIR_FORCE_CLOSE(charpriv->fd);
if (qemuSecurityClearSocketLabel(data->priv->driver->securityManager, data->def) < 0 ||
sourcefd < 0)
return -1;
}
if (charpriv->fd < 0)
charpriv->sourcefd = qemuFDPassNewDirect(devalias, data->priv);
if (qemuFDPassAddFD(charpriv->sourcefd, &sourcefd, "-source") < 0)
return -1;
}
break;
@ -6864,14 +6880,21 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
}
if (chardev->logfile) {
VIR_AUTOCLOSE logfd = -1;
if (qemuProcessPrepareHostBackendChardevFileHelper(chardev->logfile,
chardev->logappend,
&charpriv->logfd,
&logfd,
data->logManager,
data->priv->driver->securityManager,
data->cfg,
data->def) < 0)
return -1;
charpriv->logfd = qemuFDPassNew(devalias, data->priv);
if (qemuFDPassAddFD(charpriv->logfd, &logfd, "-log") < 0)
return -1;
}
return 0;
@ -6904,6 +6927,8 @@ qemuProcessPrepareHostBackendChardev(virDomainObj *vm)
&data) < 0)
return -1;
data.fdprefix = "monitor";
if (qemuProcessPrepareHostBackendChardevOne(NULL, priv->monConfig, &data) < 0)
return -1;

2
tests/qemuxml2argvdata/aarch64-pci-serial.args
View File

@ -29,7 +29,7 @@ QEMU_AUDIO_DRV=none \
-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
-device pcie-root-port,port=16,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \
-add-fd set=0,fd=1751 \
-add-fd set=0,fd=1751,opaque=serial0-log \
-chardev pty,id=charserial0,logfile=/dev/fdset/0,logappend=on \
-device pci-serial,chardev=charserial0,id=serial0,bus=pci.2,addr=0x1 \
-msg timestamp=on

2
tests/qemuxml2argvdata/name-escape.x86_64-2.11.0.args
View File

@ -33,7 +33,7 @@ QEMU_AUDIO_DRV=spice \
-device ccid-card-emulated,backend=certificates,cert1=cert1,,foo,cert2=cert2,cert3=cert3,db=/etc/pki/nssdb,,foo,id=smartcard0,bus=ccid0.0 \
-chardev tty,id=charserial0,path=/dev/ttyS2,,foo \
-device isa-serial,chardev=charserial0,id=serial0,index=1 \
-add-fd set=0,fd=1750 \
-add-fd set=0,fd=1750,opaque=serial1-source \
-chardev file,id=charserial1,path=/dev/fdset/0,append=on \
-device isa-serial,chardev=charserial1,id=serial1,index=0 \
-chardev pipe,id=charchannel0,path=/tmp/guestfwd,,foo \

2
tests/qemuxml2argvdata/name-escape.x86_64-latest.args
View File

@ -35,7 +35,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-foo=1,bar=2/.config \
-device '{"driver":"ccid-card-emulated","backend":"certificates","cert1":"cert1,foo","cert2":"cert2","cert3":"cert3","db":"/etc/pki/nssdb,foo","id":"smartcard0","bus":"ccid0.0"}' \
-chardev tty,id=charserial0,path=/dev/ttyS2,,foo \
-device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":1}' \
-add-fd set=0,fd=1750 \
-add-fd set=0,fd=1750,opaque=serial1-source \
-chardev file,id=charserial1,path=/dev/fdset/0,append=on \
-device '{"driver":"isa-serial","chardev":"charserial1","id":"serial1","index":0}' \
-chardev pipe,id=charchannel0,path=/tmp/guestfwd,,foo \

2
tests/qemuxml2argvdata/serial-file-chardev.args
View File

@ -29,7 +29,7 @@ QEMU_AUDIO_DRV=none \
-usb \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-add-fd set=0,fd=1750 \
-add-fd set=0,fd=1750,opaque=serial0-source \
-chardev file,id=charserial0,path=/dev/fdset/0,append=on \
-device isa-serial,chardev=charserial0,id=serial0,index=0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \

2
tests/qemuxml2argvdata/serial-file-chardev.x86_64-latest.args
View File

@ -31,7 +31,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \
-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \
-add-fd set=0,fd=1750 \
-add-fd set=0,fd=1750,opaque=serial0-source \
-chardev file,id=charserial0,path=/dev/fdset/0,append=on \
-device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":0}' \
-audiodev '{"id":"audio1","driver":"none"}' \

4
tests/qemuxml2argvdata/serial-file-log.args
View File

@ -29,8 +29,8 @@ QEMU_AUDIO_DRV=none \
-usb \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-add-fd set=0,fd=1750 \
-add-fd set=1,fd=1751 \
-add-fd set=0,fd=1750,opaque=serial0-source \
-add-fd set=1,fd=1751,opaque=serial0-log \
-chardev file,id=charserial0,path=/dev/fdset/0,append=on,logfile=/dev/fdset/1,logappend=on \
-device isa-serial,chardev=charserial0,id=serial0,index=0 \
-msg timestamp=on

4
tests/qemuxml2argvdata/serial-file-log.x86_64-latest.args
View File

@ -31,8 +31,8 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \
-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \
-add-fd set=0,fd=1750 \
-add-fd set=1,fd=1751 \
-add-fd set=0,fd=1750,opaque=serial0-source \
-add-fd set=1,fd=1751,opaque=serial0-log \
-chardev file,id=charserial0,path=/dev/fdset/0,append=on,logfile=/dev/fdset/1,logappend=on \
-device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":0}' \
-audiodev '{"id":"audio1","driver":"none"}' \

2
tests/qemuxml2argvdata/serial-many-chardev.args
View File

@ -31,7 +31,7 @@ QEMU_AUDIO_DRV=none \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-chardev pty,id=charserial0 \
-device isa-serial,chardev=charserial0,id=serial0,index=0 \
-add-fd set=0,fd=1750 \
-add-fd set=0,fd=1750,opaque=serial1-source \
-chardev file,id=charserial1,path=/dev/fdset/0,append=on \
-device isa-serial,chardev=charserial1,id=serial1,index=1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \

2
tests/qemuxml2argvdata/serial-many-chardev.x86_64-latest.args
View File

@ -33,7 +33,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \
-chardev pty,id=charserial0 \
-device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":0}' \
-add-fd set=0,fd=1750 \
-add-fd set=0,fd=1750,opaque=serial1-source \
-chardev file,id=charserial1,path=/dev/fdset/0,append=on \
-device '{"driver":"isa-serial","chardev":"charserial1","id":"serial1","index":1}' \
-audiodev '{"id":"audio1","driver":"none"}' \

61
tests/qemuxml2argvtest.c
View File

@ -383,10 +383,17 @@ testPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
virDomainChrSourceDef *chardev,
void *opaque)
{
virQEMUCaps *qemuCaps = opaque;
virDomainObj *vm = opaque;
qemuDomainObjPrivate *priv = vm->privateData;
qemuDomainChrSourcePrivate *charpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chardev);
int fakesourcefd = -1;
const char *devalias = NULL;
bool usefdset = true;
if (dev) {
virDomainDeviceInfo *info = virDomainDeviceGetInfo(dev);
devalias = info->alias;
/* vhost-user disk doesn't use FD passing */
if (dev->type == VIR_DOMAIN_DEVICE_DISK)
return 0;
@ -400,6 +407,8 @@ testPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
/* TPMs FD passing setup is special and handled separately */
if (dev->type == VIR_DOMAIN_DEVICE_TPM)
return 0;
} else {
devalias = "monitor";
}
switch ((virDomainChrType) chardev->type) {
@ -416,20 +425,15 @@ testPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
if (fcntl(1750, F_GETFD) != -1)
abort();
charpriv->fd = 1750;
fakesourcefd = 1750;
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
if (chardev->data.nix.listen &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE)) {
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE))
fakesourcefd = 1729;
if (fcntl(1729, F_GETFD) != -1)
abort();
charpriv->fd = 1729;
}
usefdset = false;
break;
case VIR_DOMAIN_CHR_TYPE_NMDM:
@ -437,10 +441,29 @@ testPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
break;
}
if (chardev->logfile) {
if (fcntl(1751, F_GETFD) != -1)
if (fakesourcefd != -1) {
if (fcntl(fakesourcefd, F_GETFD) != -1)
abort();
charpriv->logfd = 1751;
if (usefdset)
charpriv->sourcefd = qemuFDPassNew(devalias, priv);
else
charpriv->sourcefd = qemuFDPassNewDirect(devalias, priv);
if (qemuFDPassAddFD(charpriv->sourcefd, &fakesourcefd, "-source") < 0)
return -1;
}
if (chardev->logfile) {
int fd = 1751;
if (fcntl(fd, F_GETFD) != -1)
abort();
charpriv->logfd = qemuFDPassNew(devalias, priv);
if (qemuFDPassAddFD(charpriv->logfd, &fd, "-log") < 0)
return -1;
}
return 0;
@ -464,17 +487,11 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv,
if (qemuDomainDeviceBackendChardevForeach(vm->def,
testPrepareHostBackendChardevOne,
info->qemuCaps) < 0)
vm) < 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;
}
if (testPrepareHostBackendChardevOne(NULL, priv->monConfig, vm) < 0)
return NULL;
for (i = 0; i < vm->def->ndisks; i++) {
virDomainDiskDef *disk = vm->def->disks[i];