Re: [phodav PATCH 3/3 v7] spice-webdavd-windows: Dismount shared folder on service stop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@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.

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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]