[libvirt] [PATCH 6/8] Improve PCI host device reset error message

[Daniel P. Berrange]

On Thu, Aug 13, 2009 at 05:44:35PM +0100, Mark McLoughlin wrote:
> Currently, if we are unable to reset a PCI device we return a fairly
> generic 'No PCI reset capability available' error message.
> 
> Fix that by returning an error from the individual reset messages and
> using that error to construct the higher level error mesage.
> 
> * src/pci.c: set errors in pciTryPowerManagementReset() and
>   pciTrySecondaryBusReset() on failure; use those error messages
>   in pciResetDevice(), or explain that no reset support is available
> ---
>  src/pci.c         |   44 +++++++++++++++++++++++++++++++-------------
>  src/qemu_driver.c |    4 ++--
>  2 files changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/src/pci.c b/src/pci.c
> index 11b3e8b..74f7ef0 100644
> --- a/src/pci.c
> +++ b/src/pci.c
> @@ -456,15 +456,18 @@ pciTrySecondaryBusReset(virConnectPtr conn, pciDevice *dev)
>       * are not in use by the host or other guests.
>       */
>      if (pciBusContainsOtherDevices(conn, dev)) {
> -        VIR_WARN("Other devices on bus with %s, not doing bus reset",
> -                 dev->name);
> +        pciReportError(conn, VIR_ERR_NO_SUPPORT,
> +                       _("Other devices on bus with %s, not doing bus reset"),
> +                       dev->name);
>          return -1;
>      }
>  
>      /* Find the parent bus */
>      parent = pciGetParentDevice(conn, dev);
>      if (!parent) {
> -        VIR_WARN("Failed to find parent device for %s", dev->name);
> +        pciReportError(conn, VIR_ERR_NO_SUPPORT,
> +                       _("Failed to find parent device for %s"),
> +                       dev->name);
>          return -1;
>      }
>  
> @@ -475,7 +478,9 @@ pciTrySecondaryBusReset(virConnectPtr conn, pciDevice *dev)
>       * are multiple devices/functions
>       */
>      if (pciRead(dev, 0, config_space, PCI_CONF_LEN) < 0) {
> -        VIR_WARN("Failed to save PCI config space for %s", dev->name);
> +        pciReportError(conn, VIR_ERR_NO_SUPPORT,
> +                       _("Failed to save PCI config space for %s"),
> +                       dev->name);
>          goto out;
>      }
>  
> @@ -492,9 +497,12 @@ pciTrySecondaryBusReset(virConnectPtr conn, pciDevice *dev)
>  
>      usleep(200 * 1000); /* sleep 200ms */
>  
> -    if (pciWrite(dev, 0, config_space, PCI_CONF_LEN) < 0)
> -        VIR_WARN("Failed to restore PCI config space for %s", dev->name);
> -
> +    if (pciWrite(dev, 0, config_space, PCI_CONF_LEN) < 0) {
> +        pciReportError(conn, VIR_ERR_NO_SUPPORT,
> +                       _("Failed to restore PCI config space for %s"),
> +                       dev->name);
> +        goto out;
> +    }
>      ret = 0;
>  out:
>      pciFreeDevice(conn, parent);
> @@ -516,7 +524,9 @@ pciTryPowerManagementReset(virConnectPtr conn ATTRIBUTE_UNUSED, pciDevice *dev)
>  
>      /* Save and restore the device's config space. */
>      if (pciRead(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) {
> -        VIR_WARN("Failed to save PCI config space for %s", dev->name);
> +        pciReportError(conn, VIR_ERR_NO_SUPPORT,
> +                       _("Failed to save PCI config space for %s"),
> +                       dev->name);
>          return -1;
>      }
>  
> @@ -533,8 +543,12 @@ pciTryPowerManagementReset(virConnectPtr conn ATTRIBUTE_UNUSED, pciDevice *dev)
>  
>      usleep(10 * 1000); /* sleep 10ms */
>  
> -    if (pciWrite(dev, 0, &config_space[0], PCI_CONF_LEN) < 0)
> -        VIR_WARN("Failed to restore PCI config space for %s", dev->name);
> +    if (pciWrite(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) {
> +        pciReportError(conn, VIR_ERR_NO_SUPPORT,
> +                       _("Failed to restore PCI config space for %s"),
> +                       dev->name);
> +        return -1;
> +    }
>  
>      return 0;
>  }
> @@ -582,10 +596,14 @@ pciResetDevice(virConnectPtr conn, pciDevice *dev)
>      if (ret < 0 && dev->bus != 0)
>          ret = pciTrySecondaryBusReset(conn, dev);
>  
> -    if (ret < 0)
> +    if (ret < 0) {
> +        virErrorPtr err = virGetLastError();
>          pciReportError(conn, VIR_ERR_NO_SUPPORT,
> -                       _("No PCI reset capability available for %s"),
> -                       dev->name);
> +                       _("Unable to reset PCI device %s: %s"),
> +                       dev->name,
> +                       err ? err->message : _("no FLR, PM reset or bus reset available"));
> +    }
> +
>      return ret;
>  }
>  
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index ed2f3c4..6d1ec06 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -1465,9 +1465,9 @@ qemuDomainReAttachHostDevices(virConnectPtr conn, virDomainDefPtr def)
>              continue;
>          }
>  
> -        if (pciDettachDevice(conn, dev) < 0) {
> +        if (pciReAttachDevice(conn, dev) < 0) {
>              virErrorPtr err = virGetLastError();
> -            VIR_ERROR(_("Failed to reset PCI device: %s\n"),
> +            VIR_ERROR(_("Failed to re-attach PCI device: %s\n"),
>                        err ? err->message : "");
>              virResetError(err);
>          }

ACK


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|