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
> 鍦? 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@xxxxxxxxx>
> > > ---
> > > 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.
Cheers,
Victor
Attachment:
signature.asc
Description: PGP signature
