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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list