Hi,
On Fri, May 27, 2016 at 05:16:34PM +0200, Lukáš Venhoda wrote:
> From: Lukas Venhoda <lvenhoda@xxxxxxxxxx>
>
> When stopping the service, automatically disconnect shared folder on
> windows. Not dismounting could lead to multiple shared folders.
> ---
> Fixup:
> - Added GMutex init and clear
> - Inititalize service_data
> - Only unmap when drive_letter != 0
>
> Changes since v6:
> - Changed gchar* to ServiceData*
> - Added mutex for ServiceData multithread handeling
> - Moved ServiceData from windows service only to all OS
> - ServiceData is currently empty on Linux
> - Can be used in the future
> - Added correct unmapping for --no-service
>
> Changes since v5:
> - fixed indentation
> - fixed * indentaion
> - removed return from void functions
> - changed ServiceData * to gpointer, so that it compiles on linux
>
> Changes since v4:
> - Dont lookup drive letter when unmapping
> - Uses a service_data structure and map_drive_data structure to store
> the drive_letter for later use while unmapping
> - Better debug messages
>
> Changes since v3:
> - Better handeling of string names
> - Syntax cleanup
> - Remove global variable drive_letter
> - Now scans for mapped drive and unmaps it
>
> Changes since v2:
> - None
>
> Changes since v1:
> - New patch
> ---
> spice/spice-webdavd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 77 insertions(+), 4 deletions(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index 216d895..077a1b3 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -57,6 +57,14 @@ typedef struct _OutputQueueElem
> gpointer user_data;
> } OutputQueueElem;
>
> +typedef struct _ServiceData
> +{
> +#ifdef G_OS_WIN32
> + gchar drive_letter;
> + GMutex mutex;
> +#endif
> +} ServiceData;
> +
> static GCancellable *cancel;
>
> static OutputQueue*
> @@ -749,6 +757,7 @@ typedef enum _MapDriveEnum
>
> typedef struct _MapDriveData
> {
> + ServiceData *service_data;
> GCancellable *cancel_map;
> } MapDriveData;
>
> @@ -855,6 +864,31 @@ map_drive(const gchar drive_letter)
> }
>
> static void
> +unmap_drive(const gchar drive_letter)
Might be better to pass the service_data instead
> +{
> + gchar local_name[MAX_DRIVE_LETTER_SIZE];
> + guint32 errn;
> +
And do the mutex lock/unlock here
> + g_snprintf(local_name, MAX_DRIVE_LETTER_SIZE, "%c:", drive_letter);
> + errn = WNetCancelConnection2(local_name, CONNECT_UPDATE_PROFILE, TRUE);
And also set service_data->drive_letter to 0 in case of success (just
for completeness)
> +
> + if (errn == NO_ERROR)
> + {
> + g_debug ("Shared folder unmapped succesfully");
> + }
> + else if (errn == ERROR_NOT_CONNECTED)
> + {
> + g_debug ("Drive %c is not connected", drive_letter);
> + }
> + else
> + {
> + g_warning ("map_drive error %d", errn);
> + }
> +
> + return;
> +}
> +
> +static void
> map_drive_cb(GTask *task,
> gpointer source_object,
> gpointer task_data,
> @@ -896,11 +930,16 @@ map_drive_cb(GTask *task,
> //TODO: After mapping, rename network drive from \\localhost@PORT\DavWWWRoot
> // to something like SPICE Shared Folder
> }
> +
> + g_mutex_lock(&map_drive_data->service_data->mutex);
> + map_drive_data->service_data->drive_letter = drive_letter;
> + g_mutex_unlock(&map_drive_data->service_data->mutex);
> }
> +
> #endif
>
> static void
> -run_service (void)
> +run_service (ServiceData *service_data)
> {
> g_debug ("Run service");
>
> @@ -909,6 +948,11 @@ run_service (void)
> map_drive_data.cancel_map = g_cancellable_new ();
> gchar drive_letter = get_spice_folder_letter ();
>
> + g_mutex_lock(&service_data->mutex);
> + service_data->drive_letter = drive_letter;
> + map_drive_data.service_data = service_data;
> + g_mutex_unlock(&service_data->mutex);
> +
> if (drive_letter == 0)
> {
> GTask *map_drive_task = g_task_new (NULL, NULL, NULL, NULL);
> @@ -990,10 +1034,19 @@ service_ctrl_handler (DWORD ctrl, DWORD type, LPVOID data, LPVOID ctx)
> {
> DWORD ret = NO_ERROR;
>
> + ServiceData *service_data = ctx;
> +
> switch (ctrl)
> {
> case SERVICE_CONTROL_STOP:
> case SERVICE_CONTROL_SHUTDOWN:
> + if (service_data->drive_letter != 0)
> + {
> + g_mutex_lock(&service_data->mutex);
> + unmap_drive (service_data->drive_letter);
> + g_mutex_unlock(&service_data->mutex);
> + g_mutex_clear(&service_data->mutex);
> + }
> quit (SIGTERM);
> service_status.dwCurrentState = SERVICE_STOP_PENDING;
> SetServiceStatus (service_status_handle, &service_status);
> @@ -1009,8 +1062,13 @@ service_ctrl_handler (DWORD ctrl, DWORD type, LPVOID data, LPVOID ctx)
> VOID WINAPI
> service_main (DWORD argc, TCHAR *argv[])
> {
> + ServiceData service_data;
> +
> + service_data.drive_letter = 0;
> + g_mutex_init(&service_data.mutex);
> +
> service_status_handle =
> - RegisterServiceCtrlHandlerEx ("spice-webdavd", service_ctrl_handler, NULL);
> + RegisterServiceCtrlHandlerEx ("spice-webdavd", service_ctrl_handler, &service_data);
>
> g_return_if_fail (service_status_handle != 0);
>
> @@ -1024,7 +1082,7 @@ service_main (DWORD argc, TCHAR *argv[])
> SetServiceStatus (service_status_handle, &service_status);
>
> while (!quit_service) {
> - run_service ();
> + run_service (&service_data);
> g_usleep (G_USEC_PER_SEC);
> }
>
> @@ -1048,6 +1106,7 @@ static GOptionEntry entries[] = {
> int
> main (int argc, char *argv[])
> {
> + ServiceData service_data;
> GOptionContext *opts;
> GError *error = NULL;
>
> @@ -1095,6 +1154,10 @@ main (int argc, char *argv[])
> "incoming", G_CALLBACK (incoming_callback),
> NULL);
>
> +
> + service_data.drive_letter = 0;
> + g_mutex_init(&service_data.mutex);
> +
> #ifdef G_OS_WIN32
> SERVICE_TABLE_ENTRY service_table[] =
> {
> @@ -1110,10 +1173,20 @@ main (int argc, char *argv[])
> } else
> #endif
> while (!quit_service) {
> - run_service ();
> + run_service (&service_data);
> g_usleep (G_USEC_PER_SEC);
> }
>
> +#ifdef G_OS_WIN32
> + if (service_data.drive_letter != 0)
> + {
> + g_mutex_lock(&service_data.mutex);
> + unmap_drive (service_data.drive_letter);
> + g_mutex_unlock(&service_data.mutex);
> + g_mutex_clear(&service_data.mutex);
> + }
> +#endif
> +
Besides that, looks good to me, I can do the changes before pushing the
patch, just let me know if you agree with it.
Many thanks,
toso
> g_clear_object (&socket_service);
>
> return 0;
> --
> 2.5.5
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel