[Spice-devel] [phodav PATCH 1/3 v5] spice-webdavd-windows: Automount shared folder

Victor Toso lists at victortoso.com
Fri Apr 8 06:49:19 UTC 2016


Hi,

On Thu, Apr 07, 2016 at 05:44:51PM +0200, Lukas Venhoda wrote:
> Try to connect to shared folder automatically on Windows.
>
> On each loop of run_service(), run a GTask, that waits for half a second.
> If read_thread() returns, it means, that sharing is not yet connected,
> and the map_drive task is cancelled.
>
> If the map_drive task is NOT cancelled, it enumerates free drive letters,
> and maps Spice Folder to highest possible (from Z: to A:).
>
> If all drive letters are already assigned, show a critical error, but
> don't stop the service. The folder can still be accessed trough other
> means.

Patch looks good. Minor comments below.

> ---
> Changes since v4:
>  - Changed cancel_map to MapDriveData structure
>  - Added error checking for GetLogicalDrives()
>  - Moved defines and variables around
>  - Renamed some functions and variables
>  - Better debug messages
> 
> Changes since v3:
>  - Added init_netresource and clear_netresource functions
>     - Should clean up the map_drive function
>     - Better handeling of adress to map/unmap
>        - Not hardcoded port
>  - Syntax cleanup
>  - Changed criticals to warnings
>  - Better TODO
> 
> Changes since v2:
>  - Changed for loop to gpoll
>  - split map_drive into 2 functions
>  - added enum for return values of map_drive
>  - added TODO for renaming the drive
>  - added explanation for the 0.5 delay into commit log
> 
> Changes since v1:
>  - Changed GThread to a GTask
>  - Only wait half a second, instead of 5
> 
> Changes since RFC:
>  - Calling WNetAddConnection2() blindly was slow and caause many threads to spawn.
>     - Now only call it once, when it is sure, that it will connect.
>  - Now connects to a * drive, instead of always Z:
>  - Thread is now spawned and joined every time run_service() is called.
> ---
>  Makefile.am           |   4 ++
>  spice/spice-webdavd.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 137 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 6127f93..d8e2d53 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -87,6 +87,10 @@ spice_webdavd_LDADD =		\
>  	$(PIE_LDFLAGS)		\
>  	$(NULL)
> 
> +if OS_WIN32
> +spice_webdavd_LDADD += -lnetapi32 -lmpr
> +endif
> +
>  deps.txt:
>  	$(AM_V_GEN)rpm -qa | grep $(host_os) | sort | unix2dos > $@
> 
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index f9c5cf1..6c2e53f 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -737,11 +737,139 @@ open_mux_path (const char *path)
>    mux_queue = output_queue_new (G_OUTPUT_STREAM (mux_ostream));
>  }
>
> +#ifdef G_OS_WIN32
> +#define MAX_SHARED_FOLDER_NAME_SIZE 64
> +#define MAX_DRIVE_LETTER_SIZE 3
> +typedef enum _MapDriveEnum
> +{
> +  MAP_DRIVE_OK,
> +  MAP_DRIVE_TRY_AGAIN,
> +  MAP_DRIVE_ERROR
> +} MapDriveEnum;
> +
> +typedef struct _MapDriveData
> +{
> +  GCancellable * cancel_map;

The other structures in the code have the '*' with the variable.

> +} MapDriveData;

> +
> +static gchar
> +get_free_drive_letter(void)
> +{
> +  const guint32 max_mask = 1 << 25;
> +  guint32 drives;
> +  gint i;
> +
> +  drives = GetLogicalDrives ();
> +  if (drives == 0) {
> +    g_warning ("%s", g_win32_error_message (GetLastError ()));
> +    return 0;
> +  }
> +
> +  for (i = 0; i < 26; i++)
> +  {
> +    guint32 mask = max_mask >> i;
> +    if ((drives & mask) == 0)
> +      return 'z' - i;
> +  }

The indentation of the braces are different for the other part of the code.

> +
> +  return 0;
> +}
> +
> +/* User is required to call netresource_free, when no longer needed. */
> +static void
> +netresource_init(NETRESOURCE * net_resource, const gchar drive_letter)
> +{
> +  net_resource->dwType = RESOURCETYPE_DISK;
> +  net_resource->lpLocalName = g_strdup_printf("%c:", drive_letter);
> +  net_resource->lpRemoteName = g_strdup_printf("http://localhost:%d/", port);
> +  net_resource->lpProvider = NULL;
> +}
> +
> +static void
> +netresource_free(NETRESOURCE * net_resource)
> +{
> +  g_free(net_resource->lpLocalName);
> +  g_free(net_resource->lpRemoteName);
> +}
> +
> +static MapDriveEnum
> +map_drive(const gchar drive_letter)
> +{
> +  NETRESOURCE net_resource;
> +  guint32 errn;
> +
> +  netresource_init(&net_resource, drive_letter);
> +  errn = WNetAddConnection2 (&net_resource, NULL, NULL, CONNECT_TEMPORARY);
> +  netresource_free(&net_resource);
> +
> +  if (errn == NO_ERROR) {
> +    g_debug ("Shared folder mapped to %c succesfully", drive_letter);
> +    return MAP_DRIVE_OK;
> +  } else if (errn == ERROR_ALREADY_ASSIGNED) {
> +    g_debug ("Drive letter %c is already assigned", drive_letter);
> +    return MAP_DRIVE_TRY_AGAIN;
> +  } else {
> +    g_warning ("map_drive error %d", errn);
> +    return MAP_DRIVE_ERROR;

You can move out the logic in the else as all the conditions before are
returning

> +  }
> +}
> +
> +static void
> +map_drive_cb(GTask *task,
> +             gpointer source_object,
> +             gpointer task_data,
> +             GCancellable *cancellable)
> +{
> +  const guint32 delay = 500; //half a second
> +  MapDriveData * map_drive_data = task_data;
> +  gchar drive_letter;
> +  GPollFD cancel_pollfd;
> +  guint32 ret = 0;
> +
> +  if (!g_cancellable_make_pollfd (map_drive_data->cancel_map, &cancel_pollfd)) {
> +    g_critical ("GPollFD failed to create.");
> +    return;
> +  }
> +
> +  ret = g_poll (&cancel_pollfd, 1, delay);
> +  g_cancellable_release_fd (map_drive_data->cancel_map);
> +
> +  if (ret != 0) {
> +    return;
> +  }
> +
> +  while (TRUE)
> +  {
> +    drive_letter = get_free_drive_letter ();
> +    if (drive_letter == 0) {
> +      g_warning ("all drive letters already assigned.");
> +      break;
> +    }
> +
> +    if (map_drive (drive_letter) != MAP_DRIVE_TRY_AGAIN) {
> +      break;
> +  }
> +  //TODO: After mapping, rename network drive from \\localhost at PORT\DavWWWRoot
> +  //      to something like SPICE Shared Folder
> +  }
> +  return;

The function is void so you don't really to return here

> +}
> +#endif
> +
>  static void
>  run_service (void)
>  {
>    g_debug ("Run service");
> 
> +#ifdef G_OS_WIN32
> +  MapDriveData map_drive_data;
> +  map_drive_data.cancel_map = g_cancellable_new ();
> +  GTask * map_drive_task = g_task_new (NULL, NULL, NULL, NULL);
> +  g_task_set_task_data (map_drive_task, &map_drive_data, NULL);
> +  g_task_run_in_thread (map_drive_task, map_drive_cb);
> +  g_object_unref (map_drive_task);
> +#endif
> +
>    g_socket_service_start (socket_service);
> 
>    cancel = g_cancellable_new ();
> @@ -775,6 +903,11 @@ run_service (void)
>    g_main_loop_run (loop);
>    g_main_loop_unref (loop);
> 
> +#ifdef G_OS_WIN32
> +  g_cancellable_cancel (map_drive_data.cancel_map);
> +  g_object_unref (map_drive_data.cancel_map);
> +#endif
> +
>    g_cancellable_cancel (cancel);
> 
>    g_clear_object (&mux_istream);
> --
> 2.5.5
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


More information about the Spice-devel mailing list