Re: [Xen-devel] [PATCH] libxl_pci: check that host device is assignable before adding to the domain

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-01-16 at 22:36 +0000, Doug Magee wrote:
> Previously, on ..._pci_add, libxl only checks that a device is not assigned 
> to another domain. This quick patch checks that the device is also owned by 
> pciback, otherwise the call fails.
> 
> Signed-off-by: Doug Magee <djmagee@xxxxxxxxxxxx>

Thanks Doug. Would this be better done by adding a call to
libxl_device_pci_list_assignable and looking for the device in it? In
fact from the looks of things this could replace the existing call to
get_all_assigned_devices from .._pci_add since
libxl_device_pci_list_assignable already omits devices which are
assigned to another domain.

Ian.

> 
> diff -r 5b2676ac1321 -r 301cc006677f tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c Mon Jan 09 16:01:44 2012 +0100
> +++ b/tools/libxl/libxl_pci.c Mon Jan 16 17:31:25 2012 -0500
> @@ -796,6 +796,9 @@ int libxl__device_pci_add(libxl__gc *gc,
>      libxl_device_pci *assigned;
>      int num_assigned, i, rc;
>      int stubdomid = 0;
> +    struct dirent *de;
> +    DIR *dir;
> +    int assignable = 0;
>  
>      rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
>      if ( rc ) {
> @@ -809,6 +812,35 @@ int libxl__device_pci_add(libxl__gc *gc,
>          goto out;
>      }
>  
> +    dir = opendir(SYSFS_PCIBACK_DRIVER);
> +    if ( NULL == dir ) {
> +        if ( errno == ENOENT ) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Looks like pciback driver not 
> loaded");
> +        }else{
> +            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", 
> SYSFS_PCIBACK_DRIVER);
> +        }
> +        rc = ERROR_FAIL;
> +        goto out_closedir;
> +    }
> +
> +    while( (de = readdir(dir)) ) {
> +        unsigned dom, bus, dev, func;
> +        if ( sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
> +            continue;
> +        if ( dom == pcidev->domain && bus == pcidev->bus &&
> +              dev == pcidev->dev && func == pcidev->func ) {
> +            assignable = 1;
> +            break;
> +        }
> +    }
> +
> +    if ( !assignable ) {
> +        rc = ERROR_FAIL;
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device not owned by pciback");
> +        goto out_closedir;
> +    }
> +
> +
>      libxl__device_pci_reset(gc, pcidev->domain, pcidev->bus, pcidev->dev, 
> pcidev->func);
>  
>      stubdomid = libxl_get_stubdom_id(ctx, domid);
> @@ -817,7 +849,7 @@ int libxl__device_pci_add(libxl__gc *gc,
>          /* stubdomain is always running by now, even at create time */
>          rc = do_pci_add(gc, stubdomid, &pcidev_s, 0);
>          if ( rc )
> -            goto out;
> +            goto out_closedir;
>      }
>  
>      orig_vdev = pcidev->vdevfn & ~7U;
> @@ -826,11 +858,11 @@ int libxl__device_pci_add(libxl__gc *gc,
>          if ( !(pcidev->vdevfn >> 3) ) {
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Must specify a v-slot for 
> multi-function devices");
>              rc = ERROR_INVAL;
> -            goto out;
> +            goto out_closedir;
>          }
>          if ( pci_multifunction_check(gc, pcidev, &pfunc_mask) ) {
>              rc = ERROR_FAIL;
> -            goto out;
> +            goto out_closedir;
>          }
>          pcidev->vfunc_mask &= pfunc_mask;
>          /* so now vfunc_mask == pfunc_mask */
> @@ -855,6 +887,8 @@ int libxl__device_pci_add(libxl__gc *gc,
>          }
>      }
>  
> +out_closedir:
> +    closedir(dir);
>  out:
>      return rc;
>  }
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel