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

[Alexander Nezhinsky]

On Fri, Dec 6, 2019 at 12:30 PM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> > > 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?
>


> On Fri, Dec 6, 2019 at 1:57 PM Yuri Benditovich <
> yuri.benditovich at daynix.com> wrote:
> 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 posi sibilities: 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.
>

Sure, I will add a few lines on requirements to the commit message.
In general, I agree with Yuri's comments above,  with some minor additions.


> > 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?
>

I don't see any simple and elegant solution which suits the scope of this
patch set.
Please propose, I will be happy to implement it, in place of this patch, or
afterward as an improvement.


> > > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>


> > > @@ -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.
>

I am changing this to call spice_usb_device_manager_is_device_shared_cd(),
which logically correct
while its internal implementation may be subject to change, as discussed in
other mail.


> >
> > > +
> > > +        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?
> >
>
Which problem this is supposed to solve?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20191207/1f6e1559/attachment.html>