[Spice-devel] [PATCH spice-gtk v4 26/29] test-cd-emu: Test attach/detach emulated device
Frediano Ziglio
fziglio at redhat.com
Tue Aug 27 14:40:39 UTC 2019
>
> Hi,
>
> On Tue, Aug 27, 2019 at 10:22:43AM +0100, Frediano Ziglio wrote:
> > Mock some usb-backend functions to be able to simulate device
> > attachment and detachment.
> > Create session and channel to pass some valid pointer anyway.
> > Emulate channel state correctly.
> > Make sure HELLO packets are sent correctly at the beginning and
> > no more afterwards.
> > Test auto-connect enabled or disabled.
> >
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> > tests/cd-emu.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 151 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/cd-emu.c b/tests/cd-emu.c
> > index 7bf1fa3c..f0966662 100644
> > --- a/tests/cd-emu.c
> > +++ b/tests/cd-emu.c
> > @@ -14,10 +14,13 @@
> > You should have received a copy of the GNU Lesser General Public
> > License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > */
> > -#include <gio/gio.h>
> > +
> > +// Mock some code in usb-backend.c
> > +#define spice_usbredir_write_callback mock_spice_usbredir_write_callback
> > +#define spice_channel_get_state mock_spice_channel_get_state
>
> I wonder if isn't better to create a -private header for this
> instead?
>
That would solve calling static functions but not allow mocking functions
which is the main purpose here.
Yes, it seems hacky but from the solutions I've found it's one of the less
hacky, really.
Also it does not change the tested code at all and it does not require to
compile the source with multiple options which is nice.
> > +#include "../src/usb-backend.c"
> >
> > #include "usb-device-cd.h"
> > -#include "usb-emulation.h"
> >
> > static SpiceUsbBackendDevice *device = NULL;
> >
> > @@ -67,6 +70,150 @@ static void multiple(const void *param)
> > spice_usb_backend_delete(be);
> > }
> >
> > +static unsigned int messages_sent = 0;
> > +static unsigned int hellos_sent = 0;
> > +static SpiceUsbBackendChannel *usb_ch;
> > +
> > +int
> > +mock_spice_usbredir_write_callback(SpiceUsbredirChannel *channel, uint8_t
> > *data, int count)
> > +{
> > + ++messages_sent;
> > + g_assert_cmpint(count, >=, 4);
> > + const uint32_t type = data[0] + data[1] * 0x100u + data[2] * 0x10000u
> > + data[3] * 0x1000000u;
> > + if (type == usb_redir_hello) {
> > + ++hellos_sent;
> > + }
> > +
> > + // return we handled the data
> > + spice_usb_backend_return_write_data(usb_ch, data);
> > + return count;
> > +}
> > +
> > +// channel state to return from Mock function
> > +static enum spice_channel_state ch_state =
> > SPICE_CHANNEL_STATE_UNCONNECTED;
> > +
> > +enum spice_channel_state
> > +mock_spice_channel_get_state(SpiceChannel *channel)
> > +{
> > + return ch_state;
> > +}
> > +
> > +// number of GObjects allocated we expect will be freed
> > +static unsigned gobjects_allocated = 0;
> > +static void decrement_allocated(gpointer data G_GNUC_UNUSED, GObject
> > *old_gobject G_GNUC_UNUSED)
> > +{
> > + g_assert_cmpint(gobjects_allocated, !=, 0);
> > + --gobjects_allocated;
> > +}
> > +
> > +#define DATA_START \
> > + do { static const uint8_t data[] = {
> > +#define DATA_SEND \
> > + }; \
> > + spice_usb_backend_read_guest_data(usb_ch, (uint8_t*)data,
> > G_N_ELEMENTS(data)); \
> > + } while(0)
> > +
> > +static void attach(const void *param)
> > +{
> > + const bool attach_on_connect = !!GPOINTER_TO_UINT(param);
> > +
> > + hellos_sent = 0;
> > + messages_sent = 0;
> > + ch_state = SPICE_CHANNEL_STATE_UNCONNECTED;
> > +
> > + SpiceSession *session = spice_session_new();
> > + g_assert_nonnull(session);
> > + g_object_weak_ref(G_OBJECT(session), decrement_allocated, NULL);
> > + SpiceChannel *ch = spice_channel_new(session, SPICE_CHANNEL_USBREDIR,
> > 0);
> > + g_assert_nonnull(ch);
> > + g_object_weak_ref(G_OBJECT(ch), decrement_allocated, NULL);
> > + gobjects_allocated = 2;
> > +
> > + /*
> > + * real test, allocate a channel usbredir, emulate device
> > + * initialization.
> > + * Filter some call.
> > + * Start sequence:
> > + * - spice_usb_backend_new
> > + * - spice_usb_backend_register_hotplug
> > + * - spice_usb_backend_create_emulated_device
> > + * - spice_usb_backend_channel_new
> > + * - spice_usb_backend_channel_attach (if redir on connect)
> > + * - spice_usb_backend_channel_flush_writes
> > + * - spice_usbredir_write_callback
> > + * - spice_usb_backend_return_write_data
> > + * - spice_usb_backend_read_guest_data
> > + * - spice_usb_backend_channel_attach (if not redir on connect)
> > + */
> > + GError *err = NULL;
> > + SpiceUsbBackend * be = spice_usb_backend_new(&err);
> > + g_assert_nonnull(be);
> > + g_assert_null(err);
> > + spice_usb_backend_register_hotplug(be, NULL, test_hotplug_callback,
> > &err);
> > + g_assert_null(err);
> > +
> > + CdEmulationParams params = { "test-cd-emu.iso", 1 };
> > + g_assert_true(create_emulated_cd(be, ¶ms, &err));
> > + g_assert_null(err);
> > + g_assert_nonnull(device);
> > +
> > + usb_ch = spice_usb_backend_channel_new(be,
> > SPICE_USBREDIR_CHANNEL(ch));
> > + g_assert_nonnull(usb_ch);
> > +
> > + // attach on connect
> > + ch_state = SPICE_CHANNEL_STATE_CONNECTING;
> > + if (attach_on_connect) {
> > + g_assert_true(spice_usb_backend_channel_attach(usb_ch, device,
> > &err));
> > + g_assert_null(err);
> > + }
> > + g_assert_cmpint(hellos_sent, ==, 0);
> > + g_assert_cmpint(messages_sent, ==, 0);
> > +
> > + // try to get initial data
> > + ch_state = SPICE_CHANNEL_STATE_READY;
> > + spice_usb_backend_channel_flush_writes(usb_ch);
> > +
> > + // we should get an hello (only one!)
> > + g_assert_cmpint(hellos_sent, ==, 1);
> > + g_assert_cmpint(messages_sent, ==, 1);
> > +
> > + // send hello reply
> > + DATA_START
> > + 0x00,0x00,0x00,0x00,0x44,0x00,0x00,0x00,0x00,0x00,0x00,0x00, //000
> > ....D.......
> > + 0x71,0x65,0x6d,0x75,0x20,0x75,0x73,0x62,0x2d,0x72,0x65,0x64, //00c
> > qemu usb-red
> > + 0x69,0x72,0x20,0x67,0x75,0x65,0x73,0x74,0x20,0x33,0x2e,0x30, //018
> > ir guest 3.0
> > + 0x2e,0x31,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, //024
> > .1..........
> > + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, //030
> > ............
> > + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, //03c
> > ............
> > + 0x00,0x00,0x00,0x00,0xff,0x00,0x00,0x00, //048
> > ........
> > + DATA_SEND;
>
> Should we have different 'hellos' in the future? Let's say we
> support 3.0 but not 3.1, etc.
>
That "3.0" is not much important, it's just a string you can write "Pizza Margherita"
and code will be still happy. What's worth are the last 4 bytes, but that we can change
if we need in the future.
> > +
> > + if (!attach_on_connect) {
> > + g_assert_true(spice_usb_backend_channel_attach(usb_ch, device,
> > &err));
> > + g_assert_null(err);
> > + }
> > + g_assert_cmpint(hellos_sent, ==, 1);
> > + g_assert_cmpint(messages_sent, >, 1);
> > +
> > + // cleanup
> > + spice_usb_backend_device_unref(device);
> > + device = NULL;
> > + spice_usb_backend_channel_delete(usb_ch);
> > + usb_ch = NULL;
> > + spice_usb_backend_deregister_hotplug(be);
> > + spice_usb_backend_delete(be);
> > +
> > + // this it's the correct sequence to free session!
> > + // g_object_unref is not enough, causing wrong reference countings
> > + spice_session_disconnect(session);
> > + g_object_unref(session);
> > + while (g_main_context_iteration(NULL, FALSE)) {
> > + continue;
> > + }
> > +
> > + g_assert_cmpint(gobjects_allocated, ==, 0);
> > +}
> > +
> > static void
> > write_test_iso(void)
> > {
> > @@ -87,6 +234,8 @@ int main(int argc, char* argv[])
> >
> > g_test_add_data_func("/cd-emu/simple", GUINT_TO_POINTER(1), multiple);
> > g_test_add_data_func("/cd-emu/multiple", GUINT_TO_POINTER(128),
> > multiple);
> > + g_test_add_data_func("/cd-emu/attach_no_auto", GUINT_TO_POINTER(0),
> > attach);
> > + g_test_add_data_func("/cd-emu/attach_auto", GUINT_TO_POINTER(1),
> > attach);
>
> Not much to complain actually, happy again that we have this
> tested. Lots of pre increment/decrement. I don't care much but
> that's not that common in the client, I think.
>
Removed (at least in this patch, tell me if you see more, not strong
on it)
> >
> > return g_test_run();
> > }
Frediano
More information about the Spice-devel
mailing list