On Thu, May 20, 2010 at 10:02:10AM -0400, Alex Williamson wrote:
> This allows libvirt to open the PCI device sysfs config file prior
> to dropping privileges so qemu can access the full config space.
> Without this, a de-privileged qemu can only access the first 64
> bytes of config space.
>
> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> ---
>
> v2:
> - fix qemuargs, two separate args (thanks Chris)
> - don't hardcode pci segment
> - error message of open() fail
>
> src/qemu/qemu_conf.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++-
> src/qemu/qemu_conf.h | 8 ++++
> src/qemu/qemu_driver.c | 2 +
> 3 files changed, 95 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index d7bc798..24c360e 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1345,6 +1345,48 @@ fail:
> return -1;
> }
>
> +void qemudParsePCIDeviceStrs(const char *qemu, unsigned long long *flags)
Minor point, that this function can be declared static.
> +{
> + const char *const qemuarg[] = { qemu, "-device", "pci-assign,?", NULL };
> + const char *const qemuenv[] = { "LC_ALL=C", NULL };
> + pid_t child;
> + int status;
> + int newstderr = -1;
> +
> + if (virExec(qemuarg, qemuenv, NULL,
> + &child, -1, NULL, &newstderr, VIR_EXEC_CLEAR_CAPS) < 0)
> + return;
> +
> + char *pciassign = NULL;
> + enum { MAX_PCI_OUTPUT_SIZE = 1024*4 };
> + int len = virFileReadLimFD(newstderr, MAX_PCI_OUTPUT_SIZE, &pciassign);
> + if (len < 0) {
> + virReportSystemError(errno,
> + _("Unable to read %s pci-assign device output"),
> + qemu);
> + goto cleanup;
> + }
> +
> + if (strstr(pciassign, "pci-assign.configfd"))
> + *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD;
> +
> +cleanup:
> + VIR_FREE(pciassign);
> + close(newstderr);
> +rewait:
> + if (waitpid(child, &status, 0) != child) {
> + if (errno == EINTR)
> + goto rewait;
> +
> + VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"),
> + WEXITSTATUS(status), (unsigned long)child);
> + }
> + if (WEXITSTATUS(status) != 0) {
> + VIR_WARN("Unexpected exit status '%d', qemu probably failed",
> + WEXITSTATUS(status));
> + }
> +}
> +
> int qemudExtractVersionInfo(const char *qemu,
> unsigned int *retversion,
> unsigned long long *retflags) {
> @@ -1378,6 +1420,9 @@ int qemudExtractVersionInfo(const char *qemu,
> &version, &is_kvm, &kvm_version) == -1)
> goto cleanup2;
>
> + if (flags & QEMUD_CMD_FLAG_DEVICE)
> + qemudParsePCIDeviceStrs(qemu, &flags);
> +
> if (retversion)
> *retversion = version;
> if (retflags)
> @@ -2887,8 +2932,33 @@ error:
> }
>
>
> +int
> +qemudOpenPCIConfig(virDomainHostdevDefPtr dev)
> +{
> + char *path = NULL;
> + int configfd = -1;
> +
> + if (virAsprintf(&path, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/config",
> + dev->source.subsys.u.pci.domain,
> + dev->source.subsys.u.pci.bus,
> + dev->source.subsys.u.pci.slot,
> + dev->source.subsys.u.pci.function) < 0) {
> + virReportOOMError();
> + return -1;
> + }
> +
> + configfd = open(path, O_RDWR, 0);
> +
> + if (configfd < 0)
> + virReportSystemError(errno, _("Failed opening %s"), path);
> +
> + VIR_FREE(path);
> +
> + return configfd;
> +}
> +
> char *
> -qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev)
> +qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, int configfd)
> {
> virBuffer buf = VIR_BUFFER_INITIALIZER;
>
> @@ -2898,6 +2968,8 @@ qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev)
> dev->source.subsys.u.pci.slot,
> dev->source.subsys.u.pci.function);
> virBufferVSprintf(&buf, ",id=%s", dev->info.alias);
> + if (configfd >= 0)
> + virBufferVSprintf(&buf, ",configfd=%d", configfd);
For hotplug to work, it will actually be neccessary to make this take
a const char * string and use %s. For CLI args the string will just be
the FD number formatted normally, for hotplug the string will be a
symbolic FD name.
> @@ -4600,8 +4672,21 @@ int qemudBuildCommandLine(virConnectPtr conn,
> if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
> if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
> + int configfd = -1;
const char configfdstr[20];
> + if (qemuCmdFlags & QEMUD_CMD_FLAG_PCI_CONFIGFD) {
> + configfd = qemudOpenPCIConfig(hostdev);
> +
> + if (configfd >= 0) {
snprintf(configfdstr, sizeof(configfdstr), "%d", configfd);
> + if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
> + close(configfd);
> + goto no_memory;
> + }
> +
> + (*vmfds)[(*nvmfds)++] = configfd;
> + }
> + }
> ADD_ARG_LIT("-device");
> - if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev)))
> + if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd)))
And pass the configfdstr here.
> goto error;
> ADD_ARG(devstr);
> } else if (qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE) {
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index a101e47..64fab60 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -88,6 +88,7 @@ enum qemud_cmd_flags {
> QEMUD_CMD_FLAG_NO_HPET = (1LL << 33), /* -no-hpet flag is supported */
> QEMUD_CMD_FLAG_NO_KVM_PIT = (1LL << 34), /* -no-kvm-pit-reinjection supported */
> QEMUD_CMD_FLAG_TDF = (1LL << 35), /* -tdf flag (user-mode pit catchup) */
> + QEMUD_CMD_FLAG_PCI_CONFIGFD = (1LL << 36), /* pci-assign.configfd */
> };
>
> /* Main driver state */
> @@ -190,6 +191,9 @@ int qemudParseHelpStr (const char *qemu,
> unsigned int *is_kvm,
> unsigned int *kvm_version);
>
> +void qemudParsePCIDeviceStrs (const char *qemu,
> + unsigned long long *qemuCmdFlags);
> +
> int qemudBuildCommandLine (virConnectPtr conn,
> struct qemud_driver *driver,
> virDomainDefPtr def,
> @@ -242,7 +246,9 @@ char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound);
> /* Legacy, pre device support */
> char * qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev);
> /* Current, best practice */
> -char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev);
> +char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, int configfd);
> +
> +int qemudOpenPCIConfig(virDomainHostdevDefPtr dev);
>
> /* Current, best practice */
> char * qemuBuildChrChardevStr(virDomainChrDefPtr dev);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index dd5bd24..219a973 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7567,7 +7567,7 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver,
> if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0)
> goto error;
>
> - if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev)))
> + if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, -1)))
> goto error;
> }
This is the only thing still needing work. On the libvirt side there is a
function qemuMonitorSendFileHandle() that can be used to give QEMU the
pre-opened file handle. You the 'fdname' parameter is what's passed into
the qemuBuildPCIHostdevDevStr() earlier as the named file handle.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list