[Spice-devel] [spice-gtk v4 00/13] CD sharing feature

Tue Sep 25 08:57:11 UTC 2018

On Mon, Sep 24, 2018 at 06:56:59PM +0300, Yuri Benditovich wrote:
> On Mon, Sep 24, 2018 at 4:32 PM Christophe Fergeau <cfergeau at redhat.com>
> wrote:
> 
> > On Sat, Sep 22, 2018 at 12:21:04AM +0300, Yuri Benditovich wrote:
> > > On Sat, Sep 22, 2018 at 12:14 AM, Yuri Benditovich <
> > > yuri.benditovich at daynix.com> wrote:
> > >
> > > > Hi
> > > >
> > > > On Fri, Sep 21, 2018 at 9:10 PM, Christophe Fergeau <
> > cfergeau at redhat.com>
> > > > wrote:
> > > >
> > > >> On Mon, Sep 17, 2018 at 04:22:50PM +0300, Yuri Benditovich wrote:
> > > >> > Changes from v3:
> > > >> > Single commit with all new files split to 12 patches
> > > >> > to make patch review easier
> > > >>
> > > >> What you did is not splitting at all, adding one new file per commit,
> > > >> and then all the modifications to the existing code in a final commit
> > > >> is not what we meant when we asked for split patches.
> > > >>
> > > >> You asked why this is needed, and actually, it turns out your patches
> > > >> introduce a regression in virt-viewer even when not exercising at all
> > > >> any of the new code. The series as it is is not bisectable in any way,
> > > >> so I basically have to look at these 9000 lines of code to try to
> > figure
> > > >> out what broke...
> > > >>
> > > >
> > > BTW I'm sorry to write it, but for the record the patch is not 9000
> > lines,
> > > if is less than 2500 lines including all.
> >
> > cd-scsi.c itself which I assume to be new code is 2700 lines, the new
> > widget is 2000 lines, usb-backend-common.c is 2000 lines. I'm taking the
> > 9000 lines figure from your cover letter by the way..
> >
> > https://lists.freedesktop.org/archives/spice-devel/2018-September/045576.html
> >  26 files changed, 9321 insertions(+), 520 deletions(-)
> >
> >
> > > I believe if you describe the kind of regression I am able to find root
> > > cause of it quickly.
> >
> > I'm sure you can, but if you are the only one who can do that, that's a
> > maintainance concern. The issue is most likely caused by a leak of the
> > usb channel (or a leak from it). Start latest virt-viewer to connect to
> > a VM (probably with USB redir channels configured), exit it, and see it
> > hangs.
> >
> >
> Exit viewer or exit VM?
> Usually I the test with built-in remote-viewer, running it from .libs
> directory
> with LD_LIBRARY_PATH=.

exit virt-viewer (or remote-viewer). You need to have 65ef66e42 'Spice:
listen for new 'SpiceSession::disconnected' signal'.
Probably related to

==10758== 96 bytes in 2 blocks are definitely lost in loss record 13,528 of 16,001
==10758==    at 0x483A766: calloc (vg_replace_malloc.c:711)
==10758==    by 0x76E7FFD: g_malloc0 (gmem.c:129)
==10758==    by 0x5FF2F89: hotplug_callback (usb-backend-common.c:309)
==10758==    by 0x6F86804: libusb_hotplug_register_callback (hotplug.c:307)
==10758==    by 0x5FF35D0: spice_usb_backend_handle_hotplug (usb-backend-common.c:345)
==10758==    by 0x5FF164C: spice_usb_device_manager_initable_init (usb-device-manager.c:319)
==10758==    by 0x74FC3FE: g_initable_new_valist (ginitable.c:248)
==10758==    by 0x74FC4AC: g_initable_new (ginitable.c:162)
==10758==    by 0x5FCF374: spice_usb_device_manager_get (spice-session.c:2735)
==10758==    by 0x424686: create_spice_session (virt-viewer-session-spice.c:407)
==10758==    by 0x4249F8: virt_viewer_session_spice_constructed (virt-viewer-session-spice.c:224)
==10758==    by 0x7655B0A: g_object_new_internal (gobject.c:1845)
==10758==    by 0x765785D: g_object_new_valist (gobject.c:2128)
==10758==    by 0x7657BBC: g_object_new (gobject.c:1648)
==10758==    by 0x424CDB: virt_viewer_session_spice_new (virt-viewer-session-spice.c:1157)
==10758==    by 0x4186E2: virt_viewer_app_create_session (virt-viewer-app.c:1029)
==10758==    by 0x412BC1: virt_viewer_extract_connect_info (virt-viewer.c:559)
==10758==    by 0x412BC1: virt_viewer_update_display (virt-viewer.c:675)
==10758==    by 0x4140D4: virt_viewer_initial_connect (virt-viewer.c:931)
==10758==    by 0x418A59: virt_viewer_app_initial_connect (virt-viewer-app.c:1316)
==10758==    by 0x413987: virt_viewer_connect (virt-viewer.c:1107)
==10758==    by 0x413C41: virt_viewer_start (virt-viewer.c:1149)
==10758==    by 0x418B07: virt_viewer_app_start (virt-viewer-app.c:1680)
==10758==    by 0x41A504: virt_viewer_app_on_application_startup (virt-viewer-app.c:1818)
==10758==    by 0x764FC9C: g_closure_invoke (gclosure.c:810)
==10758==    by 0x76633A5: signal_emit_unlocked_R (gsignal.c:3565)
==10758==    by 0x766C329: g_signal_emit_valist (gsignal.c:3391)
==10758==    by 0x766C922: g_signal_emit (gsignal.c:3447)
==10758==    by 0x753C7C1: g_application_register (gapplication.c:2137)
==10758==    by 0x753D39A: g_application_run (gapplication.c:2444)
==10758==    by 0x414679: main (virt-viewer-main.c:41)

> 
> Nothing hangs.
> 
> Let's take it to separate mail thread, may be? Or all this process must be
> piblic and here?

Sure, feel free to reach out privately or on IRC.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180925/9ea4d13a/attachment.sig>