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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Fri, Dec 6, 2019 at 12:30 PM Frediano Ziglio <fziglio@xxxxxxxxxx> 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@xxxxxxxxxx> 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?
 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]