On Thu, Jan 30, 2020 at 06:06:26PM +0100, Ján Tomko wrote: > Start virtiofsd for each <filesystem> device using it. > > Pre-create the socket for communication with QEMU and pass it > to virtiofsd. > > Note that virtiofsd needs to run as root. > > https://bugzilla.redhat.com/show_bug.cgi?id=1694166 > > Introduced by QEMU commit a43efa34c7d7b628cbf1ec0fe60043e5c91043ea > > Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx> > --- > po/POTFILES.in | 1 + > src/qemu/Makefile.inc.am | 2 + > src/qemu/qemu_domain.c | 5 +- > src/qemu/qemu_domain.h | 2 +- > src/qemu/qemu_extdevice.c | 20 ++- > src/qemu/qemu_virtiofs.c | 290 ++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_virtiofs.h | 38 +++++ > tests/qemuxml2argvtest.c | 11 ++ > 8 files changed, 366 insertions(+), 3 deletions(-) > create mode 100644 src/qemu/qemu_virtiofs.c > create mode 100644 src/qemu/qemu_virtiofs.h > > diff --git a/po/POTFILES.in b/po/POTFILES.in > index c18e21615f..813fb24199 100644 > --- a/po/POTFILES.in > +++ b/po/POTFILES.in > @@ -168,6 +168,7 @@ > @SRCDIR@/src/qemu/qemu_tpm.c > @SRCDIR@/src/qemu/qemu_vhost_user.c > @SRCDIR@/src/qemu/qemu_vhost_user_gpu.c > +@SRCDIR@/src/qemu/qemu_virtiofs.c > @SRCDIR@/src/remote/remote_daemon.c > @SRCDIR@/src/remote/remote_daemon_config.c > @SRCDIR@/src/remote/remote_daemon_dispatch.c > diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am > index d04a87e659..7a205b4da6 100644 > --- a/src/qemu/Makefile.inc.am > +++ b/src/qemu/Makefile.inc.am > @@ -67,6 +67,8 @@ QEMU_DRIVER_SOURCES = \ > qemu/qemu_vhost_user.h \ > qemu/qemu_vhost_user_gpu.c \ > qemu/qemu_vhost_user_gpu.h \ > + qemu/qemu_virtiofs.c \ > + qemu/qemu_virtiofs.h \ > qemu/qemu_checkpoint.c \ > qemu/qemu_checkpoint.h \ > qemu/qemu_backup.c \ > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index b5d5812ff8..3064c33ca8 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -1440,8 +1440,11 @@ qemuDomainFSPrivateNew(void) > > > static void > -qemuDomainFSPrivateDispose(void *obj G_GNUC_UNUSED) > +qemuDomainFSPrivateDispose(void *obj) > { > + qemuDomainFSPrivatePtr priv = obj; > + > + g_free(priv->vhostuser_fs_sock); > } How about adding close the logfile fd here? That is because virtlogd keeps opening the fd even after the guest is shutdowned. # virsh shutdown guest Domain guest is being shutdown # virsh list Id Name State -------------------- # virsh start guest error: Failed to start domain guest error: Cannot open log file: '/var/log/libvirt/qemu/guest-fs0-virtiofsd.log': Device or resource busy # lsof | grep /var/log/libvirt/qemu/guest-fs0-virtiofsd.log virtlogd 5133 root 19w REG 253,0 0 35189059 /var/log/libvirt/qemu/guest-fs0-virtiofsd.log # Sample patch to close logfile fd: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b3b5cb2054..7d5b7775fa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1439,6 +1439,7 @@ qemuDomainFSPrivateDispose(void *obj) qemuDomainFSPrivatePtr priv = obj; g_free(priv->vhostuser_fs_sock); + VIR_FORCE_CLOSE(priv->logfd); } static virClassPtr qemuDomainVideoPrivateClass; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 83150e4e6d..e4b3ac471f 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -569,6 +569,7 @@ struct _qemuDomainFSPrivate { virObject parent; char *vhostuser_fs_sock; + int logfd; }; diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index 4ea8f23fd5..17532bac20 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -217,7 +217,7 @@ qemuVirtioFSStart(virLogManagerPtr logManager, goto cleanup; rc = virCommandRun(cmd, NULL); - logfd = -1; + QEMU_DOMAIN_FS_PRIVATE(fs)->logfd = logfd; if (rc < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- Thanks, Masa > > static virClassPtr qemuDomainVideoPrivateClass; > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h > index c581b3a162..83150e4e6d 100644 > --- a/src/qemu/qemu_domain.h > +++ b/src/qemu/qemu_domain.h > @@ -568,7 +568,7 @@ typedef qemuDomainFSPrivate *qemuDomainFSPrivatePtr; > struct _qemuDomainFSPrivate { > virObject parent; > > - int dummy; > + char *vhostuser_fs_sock; > }; > > > diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c > index 7f3bb104d9..5103d4921c 100644 > --- a/src/qemu/qemu_extdevice.c > +++ b/src/qemu/qemu_extdevice.c > @@ -20,11 +20,13 @@ > > #include <config.h> > > +#include "qemu_command.h" > #include "qemu_extdevice.h" > #include "qemu_vhost_user_gpu.h" > #include "qemu_domain.h" > #include "qemu_tpm.h" > #include "qemu_slirp.h" > +#include "qemu_virtiofs.h" > > #include "viralloc.h" > #include "virlog.h" > @@ -153,7 +155,7 @@ qemuExtDevicesCleanupHost(virQEMUDriverPtr driver, > int > qemuExtDevicesStart(virQEMUDriverPtr driver, > virDomainObjPtr vm, > - virLogManagerPtr logManager G_GNUC_UNUSED, > + virLogManagerPtr logManager, > bool incomingMigration) > { > virDomainDefPtr def = vm->def; > @@ -183,6 +185,15 @@ qemuExtDevicesStart(virQEMUDriverPtr driver, > return -1; > } > > + for (i = 0; i < def->nfss; i++) { > + virDomainFSDefPtr fs = def->fss[i]; > + > + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { > + if (qemuVirtioFSStart(logManager, driver, vm, fs) < 0) > + return -1; > + } > + } > + > return 0; > } > > @@ -214,6 +225,13 @@ qemuExtDevicesStop(virQEMUDriverPtr driver, > if (slirp) > qemuSlirpStop(slirp, vm, driver, net, false); > } > + > + for (i = 0; i < def->nfss; i++) { > + virDomainFSDefPtr fs = def->fss[i]; > + > + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) > + qemuVirtioFSStop(driver, vm, fs); > + } > } > > > diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c > new file mode 100644 > index 0000000000..4aa8eaed2c > --- /dev/null > +++ b/src/qemu/qemu_virtiofs.c > @@ -0,0 +1,290 @@ > +/* > + * qemu_virtiofs.c: virtiofs support > + * > + * 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/>. > + */ > + > +#include <config.h> > + > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <fcntl.h> > + > +#include "logging/log_manager.h" > +#include "virlog.h" > +#include "qemu_command.h" > +#include "qemu_conf.h" > +#include "qemu_extdevice.h" > +#include "qemu_security.h" > +#include "qemu_virtiofs.h" > +#include "virpidfile.h" > + > +#define VIR_FROM_THIS VIR_FROM_QEMU > + > + > +char * > +qemuVirtioFSCreatePidFilename(virQEMUDriverConfigPtr cfg, > + const virDomainDef *def, > + const char *alias) > +{ > + g_autofree char *shortName = NULL; > + g_autofree char *name = NULL; > + > + if (!(shortName = virDomainDefGetShortName(def))) > + return NULL; > + > + name = g_strdup_printf("%s-%s-virtiofsd", shortName, alias); > + > + return virPidFileBuildPath(cfg->stateDir, name); > +} > + > + > +char * > +qemuVirtioFSCreateSocketFilename(virDomainObjPtr vm, > + const char *alias) > +{ > + qemuDomainObjPrivatePtr priv = vm->privateData; > + > + return virFileBuildPath(priv->libDir, alias, "-virtiofsd.sock"); > +} > + > + > +static char * > +qemuVirtioFSCreateLogFilename(virQEMUDriverConfigPtr cfg, > + const virDomainDef *def, > + const char *alias) > +{ > + g_autofree char *name = NULL; > + > + name = g_strdup_printf("%s-%s", def->name, alias); > + > + return virFileBuildPath(cfg->logDir, name, "-virtiofsd.log"); > +} > + > + > +static int > +qemuVirtioFSOpenChardev(virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + const char *socket_path) > +{ > + virDomainChrSourceDefPtr chrdev = virDomainChrSourceDefNew(NULL); > + virDomainChrDef chr = { .source = chrdev }; > + VIR_AUTOCLOSE fd = -1; > + int ret = -1; > + > + chrdev->type = VIR_DOMAIN_CHR_TYPE_UNIX; > + chrdev->data.nix.listen = true; > + chrdev->data.nix.path = g_strdup(socket_path); > + > + if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def) < 0) > + goto cleanup; > + fd = qemuOpenChrChardevUNIXSocket(chrdev); > + if (fd < 0) { > + ignore_value(qemuSecurityClearSocketLabel(driver->securityManager, vm->def)); > + goto cleanup; > + } > + if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0) > + goto cleanup; > + > + if (qemuSecuritySetChardevLabel(driver, vm, &chr) < 0) > + goto cleanup; > + > + ret = fd; > + fd = -1; > + > + cleanup: > + virObjectUnref(chrdev); > + return ret; > +} > + > +static virCommandPtr > +qemuVirtioFSBuildCommandLine(virQEMUDriverConfigPtr cfg, > + virDomainFSDefPtr fs, > + int *fd) > +{ > + g_autoptr(virCommand) cmd = NULL; > + g_auto(virBuffer) opts = VIR_BUFFER_INITIALIZER; > + > + if (!(cmd = virCommandNew(fs->binary))) > + return NULL; > + > + virCommandAddArgFormat(cmd, "--fd=%d", *fd); > + virCommandPassFD(cmd, *fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); > + *fd = -1; > + > + virCommandAddArg(cmd, "-o"); > + virBufferAsprintf(&opts, "source=%s,", fs->src->path); > + if (fs->cache) > + virBufferAsprintf(&opts, "cache=%s,", virDomainFSCacheModeTypeToString(fs->cache)); > + > + if (fs->xattr == VIR_TRISTATE_SWITCH_ON) > + virBufferAddLit(&opts, "xattr,"); > + else if (fs->xattr == VIR_TRISTATE_SWITCH_OFF) > + virBufferAddLit(&opts, "no_xattr,"); > + > + if (fs->flock == VIR_TRISTATE_SWITCH_ON) > + virBufferAddLit(&opts, "flock,"); > + else if (fs->flock == VIR_TRISTATE_SWITCH_OFF) > + virBufferAddLit(&opts, "no_flock,"); > + > + if (fs->posix_lock == VIR_TRISTATE_SWITCH_ON) > + virBufferAddLit(&opts, "posix_lock,"); > + else if (fs->posix_lock == VIR_TRISTATE_SWITCH_OFF) > + virBufferAddLit(&opts, "no_posix_lock,"); > + > + virBufferTrim(&opts, ",", -1); > + > + virCommandAddArgBuffer(cmd, &opts); > + if (cfg->virtiofsdDebug) > + virCommandAddArg(cmd, "-d"); > + > + return g_steal_pointer(&cmd); > +} > + > +int > +qemuVirtioFSStart(virLogManagerPtr logManager, > + virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + virDomainFSDefPtr fs) > +{ > + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); > + g_autoptr(virCommand) cmd = NULL; > + g_autofree char *socket_path = NULL; > + g_autofree char *pidfile = NULL; > + g_autofree char *logpath = NULL; > + pid_t pid = (pid_t) -1; > + VIR_AUTOCLOSE fd = -1; > + VIR_AUTOCLOSE logfd = -1; > + int ret = -1; > + int rc; > + > + if (!(pidfile = qemuVirtioFSCreatePidFilename(cfg, vm->def, fs->info.alias))) > + goto cleanup; > + > + if (!(socket_path = qemuVirtioFSCreateSocketFilename(vm, fs->info.alias))) > + goto cleanup; > + > + if ((fd = qemuVirtioFSOpenChardev(driver, vm, socket_path)) < 0) > + goto cleanup; > + > + logpath = qemuVirtioFSCreateLogFilename(cfg, vm->def, fs->info.alias); > + > + if (cfg->stdioLogD) { > + if ((logfd = virLogManagerDomainOpenLogFile(logManager, > + "qemu", > + vm->def->uuid, > + vm->def->name, > + logpath, > + 0, > + NULL, NULL)) < 0) > + goto cleanup; > + } else { > + if ((logfd = open(logpath, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) { > + virReportSystemError(errno, _("failed to create logfile %s"), > + logpath); > + goto cleanup; > + } > + if (virSetCloseExec(logfd) < 0) { > + virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), > + logpath); > + goto error; > + } > + } > + > + if (!(cmd = qemuVirtioFSBuildCommandLine(cfg, fs, &fd))) > + goto cleanup; > + > + virCommandSetPidFile(cmd, pidfile); > + virCommandSetOutputFD(cmd, &logfd); > + virCommandSetErrorFD(cmd, &logfd); > + virCommandNonblockingFDs(cmd); > + virCommandDaemonize(cmd); > + > + if (qemuExtDeviceLogCommand(driver, vm, cmd, "virtiofsd") < 0) > + goto cleanup; > + > + rc = virCommandRun(cmd, NULL); > + logfd = -1; > + > + if (rc < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Could not start 'virtiofsd'")); > + goto error; > + } > + > + rc = virPidFileReadPath(pidfile, &pid); > + if (rc < 0) { > + virReportSystemError(-rc, > + _("Unable to read virtiofsd pidfile '%s'"), > + pidfile); > + goto error; > + } > + > + if (virProcessKill(pid, 0) != 0) { > + virReportSystemError(errno, "%s", > + _("virtiofsd died unexpectedly")); > + goto error; > + } > + > + QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = g_steal_pointer(&socket_path); > + ret = 0; > + > + cleanup: > + if (socket_path) > + unlink(socket_path); > + return ret; > + > + error: > + if (pid != -1) > + virProcessKillPainfully(pid, true); > + if (pidfile) > + unlink(pidfile); > + goto cleanup; > +} > + > + > +void > +qemuVirtioFSStop(virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + virDomainFSDefPtr fs) > +{ > + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); > + g_autofree char *pidfile = NULL; > + virErrorPtr orig_err; > + pid_t pid = -1; > + int rc; > + > + virErrorPreserveLast(&orig_err); > + > + if (!(pidfile = qemuVirtioFSCreatePidFilename(cfg, vm->def, fs->info.alias))) > + goto cleanup; > + > + rc = virPidFileReadPathIfAlive(pidfile, &pid, NULL); > + if (rc >= 0 && pid != (pid_t) -1) > + virProcessKillPainfully(pid, true); > + > + if (unlink(pidfile) < 0 && > + errno != ENOENT) { > + virReportSystemError(errno, > + _("Unable to remove stale pidfile %s"), > + pidfile); > + } > + > + if (QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock) > + unlink(QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock); > + > + cleanup: > + virErrorRestore(&orig_err); > +} > diff --git a/src/qemu/qemu_virtiofs.h b/src/qemu/qemu_virtiofs.h > new file mode 100644 > index 0000000000..49db807b19 > --- /dev/null > +++ b/src/qemu/qemu_virtiofs.h > @@ -0,0 +1,38 @@ > +/* > + * qemu_virtiofs.h: virtiofs support > + * > + * 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/>. > + */ > + > +#pragma once > + > + > +char * > +qemuVirtioFSCreatePidFilename(virQEMUDriverConfigPtr cfg, > + const virDomainDef *def, > + const char *alias); > +char * > +qemuVirtioFSCreateSocketFilename(virDomainObjPtr vm, > + const char *alias); > + > +int > +qemuVirtioFSStart(virLogManagerPtr logManager, > + virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + virDomainFSDefPtr fs); > +void > +qemuVirtioFSStop(virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + virDomainFSDefPtr fs); > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index a36183bf34..f268d336a6 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -496,6 +496,17 @@ testCompareXMLToArgv(const void *data) > } > } > > + for (i = 0; i < vm->def->nfss; i++) { > + virDomainFSDefPtr fs = vm->def->fss[i]; > + char *s; > + > + if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) > + continue; > + > + s = g_strdup_printf("/tmp/lib/domain--1-guest/fs%zu.vhost-fs.sock", i); > + QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = s; > + } > + > if (vm->def->vsock) { > virDomainVsockDefPtr vsock = vm->def->vsock; > qemuDomainVsockPrivatePtr vsockPriv = > -- > 2.21.0 >
