[Spice-devel] [PATCH 8/9] auto-connect shared CD devices added using command line

[Yuri Benditovich]

On Fri, Dec 6, 2019 at 12:30 PM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> >
> > From: Alexander Nezhinsky <anezhins at redhat.com>
> >
> > Turn shared CD devices added using command line into hot-plugged devices
> > which are redirected according to 'auto-connect' filter rules.
> > By default command line devices are added using 'redirect-on-connect' filter,
> > which do not fit the shard CD connecting requirements.
> >
>
> Can you describe the requirements here?

My 2 cents:
I think the goal here is to make something that the user _expects_.
When he creates shared CD via command-line it does not expect it to
behave exactly as already present disk-on-key - because for
disk-on-key there are 2 possibilities: use it locally or redirect it
immediately on connect to VM.
With shared CD there is no option to use it locally so we try to
redirect it automatically if this does not violate existing rules.

>
> This patch looks like an workaround to me.

Emulated device created via command line is indeed a special kind of thing.
>From one point of view it is a device that "exists" before connection.
>From another point of view it is a device that was created especially
to redirect it.
Which solution you'd suggest?

>
> > Signed-off-by: Alexander Nezhinsky <anezhins at redhat.com>
> > ---
> >  src/usb-device-manager.c | 25 ++++++++++++++++++-------
> >  1 file changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> > index 0961ef9..a69a346 100644
> > --- a/src/usb-device-manager.c
> > +++ b/src/usb-device-manager.c
> > @@ -889,11 +889,9 @@ static void
> > spice_usb_device_manager_check_redir_on_connect(
> >      GTask *task;
> >      SpiceUsbDevice *device;
> >      SpiceUsbBackendDevice *bdev;
> > +    gboolean is_cd, shall_redirect;
> >      guint i;
> >
> > -    if (priv->redirect_on_connect == NULL)
> > -        return;
> > -
> >      for (i = 0; i < priv->devices->len; i++) {
> >          device = g_ptr_array_index(priv->devices, i);
> >
> > @@ -901,10 +899,23 @@ static void
> > spice_usb_device_manager_check_redir_on_connect(
> >              continue;
> >
> >          bdev = spice_usb_device_manager_device_to_bdev(self, device);
> > -        if (spice_usb_backend_device_check_filter(
> > -                            bdev,
> > -                            priv->redirect_on_connect_rules,
> > -                            priv->redirect_on_connect_rules_count) == 0) {
> > +        is_cd = spice_usb_backend_device_get_libdev(bdev) == NULL;
>
> This is checking if emulated, not if is a CD.
>
> > +
> > +        if (priv->redirect_on_connect) {
> > +            shall_redirect = !spice_usb_backend_device_check_filter(
> > +                                bdev,
> > +                                priv->redirect_on_connect_rules,
> > +                                priv->redirect_on_connect_rules_count);
>
> Why not setting some "rules" and "rules_count" variable instead
> and call spice_usb_backend_device_check_filter once?
>
> > +        } else if (is_cd) {
> > +            shall_redirect = !spice_usb_backend_device_check_filter(
> > +                                bdev,
> > +                                priv->auto_conn_filter_rules,
> > ++                               priv->auto_conn_filter_rules_count);
> > +        } else {
> > +            shall_redirect = FALSE;
> > +        }
> > +
> > +        if (shall_redirect) {
> >              /* Note: re-uses spice_usb_device_manager_connect_device_async's
> >                 completion handling code! */
> >              task = g_task_new(self,
>
> Frediano
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel