Hi On Thu, Aug 13, 2020 at 8:24 PM Cole Robinson <crobinso@xxxxxxxxxx> 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@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. > > > > Interesting, thanks for the comment. > > - Cole >
