[libvirt] [RFC PATCH v3] qemu: Add suport for pci-assign.configfd option

[Daniel P. Berrange]

On Mon, May 24, 2010 at 01:39:37PM -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 at redhat.com>
> ---
> 
> Note: this is still dependent on the configfd option being accepted
> into qemu/kvm.
> 
>  v2:
>   - fix qemuargs, two separate args (thanks Chris)
>   - don't hardcode pci segment
>   - error message of open() fail
> 
>  v3:
>   - add hotplug support
>   - incorporate Daniel's comments
> 
>  src/qemu/qemu_conf.c   |   98 +++++++++++++++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_conf.h   |    6 ++-
>  src/qemu/qemu_driver.c |   28 +++++++++++++-
>  3 files changed, 128 insertions(+), 4 deletions(-)
> 
> @@ -7685,8 +7687,26 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver,
>              goto error;
>          if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0)
>              goto error;
> +        if (qemuCmdFlags & QEMUD_CMD_FLAG_PCI_CONFIGFD) {
> +            configfd = qemudOpenPCIConfig(hostdev);
> +            if (configfd >= 0) {
> +                if (virAsprintf(&configfd_name, "fd-%s",
> +                                hostdev->info.alias) < 0) {
> +                    virReportOOMError();
> +                    goto error;
> +                }
> +
> +                qemuDomainObjEnterMonitorWithDriver(driver, vm);
> +                if (qemuMonitorSendFileHandle(priv->mon, configfd_name,
> +                                              configfd) < 0) {
> +                    qemuDomainObjExitMonitorWithDriver(driver, vm);
> +                    goto error;
> +                }
> +                qemuDomainObjExitMonitorWithDriver(driver, vm);
> +            }
> +        }

Subtle threading bug. At this point you need to add a check

+        if (!virDomainObjIsActive(vm)) {
+            qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                            _("guest unexpectedly quit during hotplug");
+            goto cleanup;
+        }

This checks for QEMU crashing after we send the file handle,
which means its not valid to send the subsequent device_add
command


>  
> -        if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev)))
> +        if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name)))
>              goto error;
>  
>          qemuDomainObjEnterMonitorWithDriver(driver, vm);

Aside from that addition, this patch looks good to me

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 :|