[Spice-devel] [phodav PATCH 3/3 v7] spice-webdavd-windows: Dismount shared folder on service stop
Victor Toso
lists at victortoso.com
Mon Jun 27 16:45:27 UTC 2016
Hi,
On Mon, Jun 27, 2016 at 05:46:57PM +0200, Victor Toso wrote:
> Hi,
>
> On Fri, May 27, 2016 at 05:16:34PM +0200, Lukáš Venhoda wrote:
> > From: Lukas Venhoda <lvenhoda at redhat.com>
> >
> > 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 at 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.
Pushed with the changes, many thanks!
https://git.gnome.org/browse/phodav/commit/?id=606ebd7ee34db9ac1f826d2d47f21fc32db40c2f
>
> Many thanks,
> toso
>
> > g_clear_object (&socket_service);
> >
> > return 0;
> > --
> > 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