[Spice-devel] [PATCH spice-gtk v2 0/6] Add basic tests for usb-acl-helper

[Fabiano Fidêncio]

Jonathon,

On Thu, Mar 10, 2016 at 6:17 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
> This does some minor cleanup to the SpiceUsbAclHelper class and adds some basic
> tests so that we can easily excercise more of its behavior.
>
> Differences to the previous series
>  - use an environment variable to specify the mock acl helper binary instead of
>    a GObject property
>  - do a little API cleanup after getting a bit more familiar with the code
>  - fixed a regression from the first patch where cancelling the GCancellable
>    didn't actually result in the task being completed
>
> Jonathon Jongsma (6):
>   Introduce spice_usb_acl_helper_cancel()
>   usb-acl-helper: Avoid deadlock when cancelled
>   Remove spice_usb_acl_helper_close_acl()
>   Rename spice_usb_acl_helper_open_acl() to _open_acl_async()
>   usb-acl-helper: add env var for specifying acl helper binary
>   Add tests for usb-acl-helper
>
>  src/channel-usbredir.c  |  15 ++--
>  src/usb-acl-helper.c    |  34 ++++----
>  src/usb-acl-helper.h    |  12 +--
>  tests/Makefile.am       |  15 ++--
>  tests/mock-acl-helper.c |  94 ++++++++++++++++++++
>  tests/usb-acl-helper.c  | 226 ++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 360 insertions(+), 36 deletions(-)
>  create mode 100644 tests/mock-acl-helper.c
>  create mode 100644 tests/usb-acl-helper.c
>
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

I've done a series of manual tests in your patches and they seem okay
to be pushed, so it's an ACK from my side. Not sure if someone's else
wants to review this series, so I'd wait till Monday before pushing
the patches.

I found a (possible) usbredir issue when redirecting/unredirecting a
camera to the guest. When you enable/disable the device a few times,
you end up with a "There was an error playing video from the webcam"
message on Cheese. It occurs even without your patches and I'll be
filling a bug in our bugzilla any time soon so we can treat this issue
later on.

Best Regards,
--
Fabiano Fidêncio