[libvirt] [PATCH 1/7] qemu: hotplug: Properly handle errors in qemuDomainWaitForDeviceRemoval

[Cole Robinson]

On 04/05/2016 11:09 AM, Peter Krempa wrote:
> Callers ignore if this function returns -1 and continue as though the
> DEVICE_DELETED event was not received. Since we can't be sure that the
> event was not received we should behave as if the event was not
> supported and remove the device definition right away. The error
> fortunately won't really happen here.
> ---
>  src/qemu/qemu_hotplug.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 48bea6a..6c619e9 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3351,8 +3351,8 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
>  }
> 
>  /* Returns:
> - *  -1 on error
> - *   0 when DEVICE_DELETED event is unsupported
> + *   0 when DEVICE_DELETED event is unsupported, or we failed to reliably wait
> + *   for the event
>   *   1 when DEVICE_DELETED arrived before the timeout and the caller is
>   *     responsible for finishing the removal
>   *   2 device removal did not finish in qemuDomainRemoveDeviceWaitTime
> @@ -3367,7 +3367,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
>          return 0;
> 
>      if (virTimeMillisNow(&until) < 0)
> -        return -1;
> +        return 0;
>      until += qemuDomainRemoveDeviceWaitTime;
> 
>      while (priv->unpluggingDevice) {
> @@ -3376,9 +3376,9 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
>              if (errno == ETIMEDOUT) {
>                  return 2;
>              } else {
> -                virReportSystemError(errno, "%s",
> -                                     _("Unable to wait on unplug condition"));
> -                return -1;
> +                VIR_WARN("Failed to wait on unplug condition for domain '%s' "
> +                         "device '%s'", vm->def->name, priv->unpluggingDevice);
> +                return 0;
>              }
>          }
>      }
> 

Makes sense, and I verified every caller is just checking the return value ==
1. ACK

- Cole