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. Thanks, Cole
