Re: [PATCH qemu-kvm] device-assignment: add config fd qdev property

[Alex Williamson <alex.williamson@redhat.com>]

On Tue, 2010-05-25 at 08:43 +0200, Markus Armbruster wrote:
> Alex Williamson <alex.williamson@redhat.com> writes:
> 
> > On Wed, 2010-05-19 at 12:00 -0700, Chris Wright wrote:
> >> When libvirt launches a guest it first chowns the relevenat
> >> /sys/bus/pci/.../config file for an assigned device then drops privileges.
> >> 
> >> This causes an issue for device assignment because despite being file
> >> owner, the sysfs config space file checks for CAP_SYS_ADMIN before
> >> allowing access to device dependent config space.
> >> 
> >> This adds a new qdev configfd property which allows libvirt to open the
> >> sysfs config space file and give qemu an already opened file descriptor.
> >> Along with a change pending for the 2.6.35 kernel, this allows the
> >> capability check to compare against privileges from when the file was
> >> opened.
> >
> > We need to make configfd be a string option so that we can pass a
> > descriptor from libvirt for the hotplug case.  Here's a rework.
> >
> > Signed-off-by: Chris Wright <chrisw@redhat.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >
> > diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> > index eb31c78..3c5184f 100644
> > --- a/hw/device-assignment.c
> > +++ b/hw/device-assignment.c
> > @@ -37,6 +37,7 @@
> >  #include "console.h"
> >  #include "device-assignment.h"
> >  #include "loader.h"
> > +#include "monitor.h"
> >  #include <pci/pci.h>
> >  
> >  /* From linux/ioport.h */
> > @@ -612,14 +613,28 @@ static int get_real_device(AssignedDevice *pci_dev, uint16_t r_seg,
> >  
> >      snprintf(name, sizeof(name), "%sconfig", dir);
> >  
> > -    fd = open(name, O_RDWR);
> > -    if (fd == -1) {
> > -        fprintf(stderr, "%s: %s: %m\n", __func__, name);
> > -        return 1;
> > +    if (pci_dev->configfd_name && *pci_dev->configfd_name) {
> > +        if (qemu_isdigit(pci_dev->configfd_name[0])) {
> > +            dev->config_fd = strtol(pci_dev->configfd_name, NULL, 0);
> > +        } else {
> > +            dev->config_fd = monitor_get_fd(cur_mon, pci_dev->configfd_name);
> > +            if (dev->config_fd < 0) {
> > +                fprintf(stderr, "%s: (%s) unkown\n", __func__,
> > +                        pci_dev->configfd_name);
> > +                return 1;
> > +            }
> > +        }
> 
> Similar to net_handle_fd_param().  What about factoring into a common
> helper?

Yep, maybe we should move that into monitor.c.  Thanks,

Alex