On 12/28/19 7:12 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
> ---
> Feel free to improve the patch!
>
Nice work! This looks pretty good. Some comments
* It's unfortunate that there's no way for the spice client to
communicate that necessary webdav support is even available in the VM.
Seems like a support nightmare of questions like "why isn't folder share
working". Not much we can do about that though
* Calling this 'Share folder' is too generic IMO and ungooglable, also
conflicting with passing through filesystem devices as well. So IMO the
UI label should be 'SPICE WebDAV Folder' or something along those lines
* The files should be named spicewebdav.ui and spicewebdav.py to match,
along with the object naming. Please move the python file to
virtManager/details
There's a few style warnings from './setup.py pylint'
virtManager/foldershare.py:54: [W503] line break before binary operator
virtManager/foldershare.py:60: [W503] line break before binary operator
virtManager/details/viewers.py:801: [W503] line break before binary operator
virtManager/details/viewers.py:805: [W503] line break before binary operator
And some comments below
> ui/foldershare.ui | 129 +
> ui/vmwindow.ui | 9 +++
> virtManager/details/console.py | 8 ++
> virtManager/details/viewers.py | 63
> virtManager/foldershare.py | 90 +++
> virtManager/vmwindow.py| 8 ++
> 6 files changed, 307 insertions(+)
> create mode 100644 ui/foldershare.ui
> create mode 100644 virtManager/foldershare.py
>
> diff --git a/virtManager/details/console.py b/virtManager/details/console.py
> index 193e79eb..7c0b7495 100644
> --- a/virtManager/details/console.py
> +++ b/virtManager/details/console.py
> @@ -666,6 +666,9 @@ class vmmConsolePages(vmmGObjectUI):
> bool(is_viewer and self._viewer and
> self._viewer.console_has_usb_redirection() and
> self.vm.has_spicevmc_type_redirdev()))
> +self.widget("details-menu-folder-sharing").set_sensitive(
> +bool(is_viewer and self._viewer and
> +self._viewer.console_has_folder_sharing()))
>
> can_sendkey = (is_viewer and not paused)
> for c in self._keycombo_menu.get_children():
> @@ -1012,6 +1015,11 @@ class vmmConsolePages(vmmGObjectUI):
> self._viewer.console_has_usb_redirection())
> def details_viewer_get_usb_widget(self):
> return self._viewer.console_get_usb_widget()
> +def details_viewer_has_folder_sharing(self):
> +return bool(self._viewer and
> +self._viewer.console_has_folder_sharing())
> +def details_viewer_get_folder_share_dialog(self):
> +return self._viewer.console_get_folder_share_dialog()
> def details_viewer_get_pixbuf(self):
> return self._viewer.console_get_pixbuf()
>
> diff --git a/virtManager/details/viewers.py b/virtManager/details/viewers.py
> index 1614ba6d..188ecee1 100644
> --- a/virtManager/details/viewers.py
> +++ b/virtManager/details/viewers.py
> @@ -25,6 +25,7 @@ from virtinst import log
>
> from .sshtunnels import SSHTunnels
> from ..baseclass import vmmGObject
> +from ..foldershare import vmmFolderShare
>
>
> ##
> @@ -207,6 +208,11 @@ class Viewer(vmmGObject):
> def _has_agent(self):
> raise NotImplementedError()
>
> +def _has_folder_sharing(self):
> +raise NotImplementedError()
> +def _get_folder_share_dialog(self):
> +raise NotImplementedError()
> +
>
>
> # APIs accessed by vmmConsolePages #
> @@ -270,6 +276,11 @@ class Viewer(vmmGObject):
> def console_has_agent(self):
> return self._has_agent()
>
> +def console_has_folder_sharing(self):
> +return self._has_folder_sharing()
> +def console_get_folder_share_dialog(self):
> +return self._get_folder_share_dialog()
> +
> def console_remove_display_from_widget(self, widget):
> if self._display and self._display in widget.get_children():
> widget.remove(self._display)
> @@ -437,6 +448,11 @@ class VNCViewer(Viewer):
> def _has_agent(self):
> return False
>
> +def _has_folder_sharing(self):
> +return False
> +def _get_folder_share_dialog(self):
> +return None
> +
>
> ###
> # Connection routines #
> @@ -489,6 +505,8 @@ class SpiceViewer(Viewer):
> self._main_channel_hids = []
> self._display_channel = None
> self._usbdev_manager = None
> +self._webdav_channel = None
> +self._folder_share_dialog = None
>
These need to be unset() in self.close too
>
> ###
> @@
