[PATCH] virtManager: Folder sharing implementation for SPICE session

[Marc-André Lureau]

Hi

On Thu, Aug 13, 2020 at 8:24 PM Cole Robinson <crobinso at redhat.com> wrote:
>
> On 8/13/20 11:44 AM, Victor Toso wrote:
> > On Thu, Aug 13, 2020 at 10:34:52AM +0800, dianlujitao wrote:
> >> Thanks for reviewing.
> >>
> >> I agree that it's a big mess to maintain its availability, do
> >> you have connections with the SPICE team so there's chance to
> >> work together to improve the protocol? Essentially virt-viewer
> >> suffers from the same problem and can benefit from the
> >> enhancement if it happens.
> >>
> >> For the planned virt-viewer integration, since virt-manager's
> >> built-in viewer has implemented most of virt-viewer's feature,
> >> I don't quite understand the intuition behind, would that bring
> >> much advantages over the built-in viewer?
> >
> > So, the problem is the client (virt-manager here) not being able
> > to tell that "a sharing is happening now"? I think you can watch
> > over "port-opened" [0] for that.
> >
> >     [0] https://www.spice-space.org/api/spice-gtk/SpicePortChannel.html#SpicePortChannel--port-opened
> >
>
> No, what we mean is some notification from spice that there is actually
> a webdav agent listening/ready in the VM. Something akin to
> agent-connected signal but for webdav or similar. Does that exist?

If I remember correctly, you can check and listen on channel
"port-opened" property. It means there is something that opened the
channel in the guest.

>
> >> 鍦? 2020/8/13 涓婂崍4:29, Cole Robinson 鍐欓亾:
> >>> On 7/25/20 9:30 AM, Jitao Lu wrote:
> >>>>   * This implements folder sharing for the built-in Spice client, tested
> >>>>     working with Win10 guest.
> >>>>   * The basic idea is taken from virt-viewer.
> >>>>
> >>>> Signed-off-by: Jitao Lu <dianlujitao at gmail.com>
> >>>> ---
> >>>>   ui/spicewebdav.ui                  | 129 +++++++++++++++++++++++++++++
> >>>>   ui/vmwindow.ui                     |  17 +++-
> >>>>   virtManager/details/console.py     |   8 ++
> >>>>   virtManager/details/spicewebdav.py |  60 ++++++++++++++
> >>>>   virtManager/details/viewers.py     |  68 +++++++++++++++
> >>>>   virtManager/vmwindow.py            |   8 ++
> >>>>   6 files changed, 286 insertions(+), 4 deletions(-)
> >>>>   create mode 100644 ui/spicewebdav.ui
> >>>>   create mode 100644 virtManager/details/spicewebdav.py
> >>>>
> >>> This looks pretty good, thanks. The only bits I would like to add are:
> >>>
> >>> * Tooltip when the Menu item is disabled, indicating the reason it's not
> >>> enabled: not using SPICE, or no webdav channel enabled
> >>> * Probably a warning label in the dialog that webdav requires an agent
> >>> running the guest OS.
> >>>
> >>> But honestly I'm trying to decide if this is worth it for
> >>> virt-manager.  Generally these types of features that require
> >>> external config just to get working are a big pain for
> >>> support. spice doesn't have any mechanism that I can tell to
> >>> inform us if anything is listening on the other side of the
> >>> webdav channel so we can't give good feedback in the UI if
> >>> this even has a chance of working. And specifically for spice
> >>> features, per the design[1] document, anything advanced I
> >>> would prefer to leave for virt-viewer to do (which obviously
> >>> already handles this as you mention).  Because it's not
> >>> something that works out of the box and requires external
> >>> config, it's not a stretch to ask users to also use
> >>> virt-viewer when they need it.
> >>>
> >>> [1]: https://github.com/virt-manager/virt-manager/blob/master/DESIGN.md
> >>>
> >>> So I'm unsure what to do. I was planning for the next release
> >>> to investigate some ways to make it easier to use virt-viewer
> >>> side by side with virt-manager, maybe an option to have
> >>> virt-manager not autoconnect to the console, and possibly
> >>> even a button or menu option to launch virt-viewer directly,
> >>> but I need to play with it.
> >
> > IMHO, this is the way to go. I actually suggested the same thing
> > to GNOME Boxes in the past.
> >
>
> Interesting, thanks for the comment.
>
> - Cole
>