[Spice-devel] [PATCH spice] smartcard: set char device state

[Marc-André Lureau]

Hi
On Tue, Aug 21, 2018 at 7:32 PM Christophe Fergeau <cfergeau at redhat.com> wrote:
>
> Hey,
>
> I've been trying to cut a spice-server release for a long time now, it's
> one of the patches I'd prefer to punt to after the release (which I was
> supposed to be doing today, but while testing it I hit these unrelated
> libcacard issues... :-/ )

What about taking it now?
thanks

>
> Christophe
>
> On Mon, Aug 20, 2018 at 03:51:41PM +0200, Marc-André Lureau wrote:
> > Hi
> > On Mon, Aug 20, 2018 at 2:06 PM Frediano Ziglio <fziglio at redhat.com> wrote:
> > >
> > > Without proper testing I would say Nack.
> > > I though we agreed on this.
> >
> > I have done manual testing. We don't have any smartcard channel test
> > in spice, afaik.
> >
> > What you asked me about disabling the device from the guest is
> > unrelated to this change imho.
> >
> > Please reconsider merging this, it is not changing any current logic,
> > just informing qemu about channel state to avoid useless work.
> >
> > >
> > > Frediano
> > >
> > > >
> > > > ping
> > > > On Tue, Aug 7, 2018 at 5:27 PM <marcandre.lureau at redhat.com> wrote:
> > > > >
> > > > > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > > >
> > > > > Follow all other char devices implementation (spicevmc, agent,
> > > > > stream-device) and set the char device state when
> > > > > connected/disconnected. This allows qemu to discard writes, optimize a
> > > > > bit the source polling, and will trigger HUP events.
> > > > >
> > > > > See related qemu "char/spice: discard write() if backend is
> > > > > disconnected".
> > > > >
> > > > > Note: sif->state() should probably be handled at the char-device
> > > > > level. I am not sure what the smartcard channel really brings over
> > > > > plain spicevmc...
> > > > >
> > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > > > ---
> > > > >  server/smartcard.c | 15 +++++++++++++++
> > > > >  1 file changed, 15 insertions(+)
> > > > >
> > > > > diff --git a/server/smartcard.c b/server/smartcard.c
> > > > > index 2cb68e06..403805a8 100644
> > > > > --- a/server/smartcard.c
> > > > > +++ b/server/smartcard.c
> > > > > @@ -343,6 +343,11 @@ void
> > > > > smartcard_char_device_attach_client(SpiceCharDeviceInstance *char_device,
> > > > >          dev->priv->scc = NULL;
> > > > >          smartcard_channel_client_set_char_device(scc, NULL);
> > > > >          red_channel_client_disconnect(RED_CHANNEL_CLIENT(scc));
> > > > > +    } else {
> > > > > +        SpiceCharDeviceInterface *sif =
> > > > > spice_char_device_get_interface(char_device);
> > > > > +        if (sif->state) {
> > > > > +            sif->state(char_device, 1);
> > > > > +        }
> > > > >      }
> > > > >  }
> > > > >
> > > > > @@ -373,11 +378,21 @@ gboolean
> > > > > smartcard_char_device_notify_reader_remove(RedCharDeviceSmartcard *dev)
> > > > >  void smartcard_char_device_detach_client(RedCharDeviceSmartcard
> > > > >  *smartcard,
> > > > >                                           SmartCardChannelClient *scc)
> > > > >  {
> > > > > +    SpiceCharDeviceInterface *sif;
> > > > > +    SpiceCharDeviceInstance *sin;
> > > > > +
> > > > > +    g_object_get(smartcard, "sin", &sin, NULL);
> > > > > +    sif = spice_char_device_get_interface(sin);
> > > > > +
> > > > >      spice_assert(smartcard->priv->scc == scc);
> > > > >      red_char_device_client_remove(RED_CHAR_DEVICE(smartcard),
> > > > >                                    red_channel_client_get_client(RED_CHANNEL_CLIENT(scc)));
> > > > >      smartcard_channel_client_set_char_device(scc, NULL);
> > > > >      smartcard->priv->scc = NULL;
> > > > > +
> > > > > +    if (sif->state) {
> > > > > +        sif->state(sin, 0);
> > > > > +    }
> > > > >  }
> > > > >
> > > > >  SmartCardChannelClient*
> > > > >  smartcard_char_device_get_client(RedCharDeviceSmartcard *smartcard)
> > > > > --
> > > > > 2.18.0.547.g1d89318c48
> > > > >
> > > > > _______________________________________________
> > > > > Spice-devel mailing list
> > > > > Spice-devel at lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > >
> > > >
> > > >
> > > > --
> > > > Marc-André Lureau
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > >
> >
> >
> >
> > --
> > Marc-André Lureau
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel