[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 8/9] virpcimock: Add PCI driver which always fails



On 17.01.2014 11:39, Jiri Denemark wrote:
> Such driver can be used to make sure PCI APIs fail properly.
> 
> Signed-off-by: Jiri Denemark <jdenemar redhat com>
> ---
>  tests/virpcimock.c | 33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/virpcimock.c b/tests/virpcimock.c
> index f8ea9c7..b514619 100644
> --- a/tests/virpcimock.c
> +++ b/tests/virpcimock.c
> @@ -107,11 +107,19 @@ char *fakesysfsdir;
>   *
>   */
>  
> +enum driverActions {
> +    PCI_ACTION_BIND         = 1 << 0,
> +    PCI_ACTION_UNBIND       = 1 << 1,
> +    PCI_ACTION_NEW_ID       = 1 << 2,
> +    PCI_ACTION_REMOVE_ID    = 1 << 3,
> +};
> +
>  struct pciDriver {
>      char *name;
>      int *vendor;        /* List of vendor:device IDs the driver can handle */
>      int *device;
>      size_t len;            /* @len is used for both @vendor and @device */
> +    int fail;           /* Actions on this driver which should fail? */
>  };
>  
>  struct pciDevice {
> @@ -143,7 +151,7 @@ static void pci_device_new_from_stub(const struct pciDevice *data);
>  static struct pciDevice *pci_device_find_by_id(const char *id);
>  static struct pciDevice *pci_device_find_by_content(const char *path);
>  
> -static void pci_driver_new(const char *name, ...);
> +static void pci_driver_new(const char *name, int fail, ...);
>  static struct pciDriver *pci_driver_find_by_dev(struct pciDevice *dev);
>  static struct pciDriver *pci_driver_find_by_path(const char *path);
>  static int pci_driver_bind(struct pciDriver *driver, struct pciDevice *dev);
> @@ -415,7 +423,7 @@ pci_device_autobind(struct pciDevice *dev)
>   * PCI Driver functions
>   */
>  static void
> -pci_driver_new(const char *name, ...)
> +pci_driver_new(const char *name, int fail, ...)
>  {
>      struct pciDriver *driver;
>      va_list args;
> @@ -427,10 +435,12 @@ pci_driver_new(const char *name, ...)
>          virAsprintfQuiet(&driverpath, "%s/drivers/%s", fakesysfsdir, name) < 0)
>          ABORT_OOM();
>  
> +    driver->fail = fail;
> +
>      if (virFileMakePath(driverpath) < 0)
>          ABORT("Unable to create: %s", driverpath);
>  
> -    va_start(args, name);
> +    va_start(args, fail);
>  
>      while ((vendor = va_arg(args, int)) != -1) {
>          if ((device = va_arg(args, int)) == -1)
> @@ -497,8 +507,8 @@ pci_driver_bind(struct pciDriver *driver,
>      int ret = -1;
>      char *devpath = NULL, *driverpath = NULL;
>  
> -    if (dev->driver) {
> -        /* Device already bound */
> +    if (dev->driver || PCI_ACTION_BIND & driver->fail) {
> +        /* Device already bound or failing driver requested */
>          errno = ENODEV;
>          return ret;
>      }
> @@ -544,8 +554,8 @@ pci_driver_unbind(struct pciDriver *driver,
>      int ret = -1;
>      char *devpath = NULL, *driverpath = NULL;
>  
> -    if (dev->driver != driver) {
> -        /* Device not bound to the @driver */
> +    if (dev->driver != driver || PCI_ACTION_UNBIND & driver->fail) {
> +        /* Device not bound to the @driver or failing driver used */

You got me there for a second until I realized what driver->fail is
meant for. It's not a bool but a bit field to make fail only certain
actions. That's clever. But then I'd suggest making driver->fail of type
'unsigned int' instead of 'int'. It doesn't make any difference to the
complier but as long as @flags are 'unsigned int' then by the same token
@fail should be.

ACK if you change that.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]