[Spice-devel] [phodav PATCH 1/2 v2] spice-webdavd: Automount shared folder on Windows

[Marc-André Lureau]

----- Original Message -----
> Hi
> 
> On Thu, Feb 25, 2016 at 4:21 PM, Marc-André Lureau <
> marcandre.lureau at gmail.com > wrote:
> 
> 
> Hi
> 
> On Thu, Feb 25, 2016 at 3:19 PM, Lukas Venhoda < lvenhoda at redhat.com > wrote:
> > +static void
> > +map_drive(GTask *task,
> > + gpointer source_object,
> > + gpointer task_data,
> > + GCancellable *cancellable)
> > +{
> > + static int loop_time = G_USEC_PER_SEC/10;
> > + GCancellable * cancel_map = task_data;
> > + gchar local_name[3];
> > + gchar remote_name[] = " http://localhost:9843/ ";
> > + NETRESOURCE net_resource;
> > + guint32 retval;
> > + gint i;
> > +
> > + for (i = 0; i < 5; ++i)
> > + {
> > + if (g_cancellable_is_cancelled (cancel_map))
> > + return;
> > + else
> > + g_usleep (loop_time);
> > + }
> > +
> 
> I am not sure I see the point of waiting 0.5s here. Add a comment? (a
> better way would be to use g_poll with the pollfd cancellable can
> creats: no need to loop)
> 
> 
> I tryed this using g_io_channel_win32_make_pollfd(), but the way the FD
> (well actually file handle) is created on windows, it doesn't support
> polling.
> It might require a more in depth rewrite of the windows part of webdavd to
> make it work.

I mean the cancellable fd (g_cancellable_make_pollfd + g_poll), I would be surprised if this combination didn't work on windows and file a bug.

Also please explain the point of 0.5s delay.

> > +#ifdef G_OS_WIN32
> > + GCancellable * cancel_map = g_cancellable_new ();
> > + GTask * map_drive_task = g_task_new (NULL, NULL, NULL, NULL);
> 
> no unref? you probably should g_task_return() somewhere too.
> 
> If I don't set the GTask to return-on-cancel, I don't need to call
> g_task_return(),
> because the callback is empty.
> 
> From the GTask doc:
> Other than in that case, task will be completed when the GTaskThreadFunc
> returns,
> not when it calls a g_task_return_ function.
> 
> 
> 
> > + g_task_set_task_data (map_drive_task, cancel_map, NULL);
> > + g_task_run_in_thread (map_drive_task, map_drive);
> 
> I think you could unref after this call, since run_in_thread() keeps a ref.
> 
> Before adding and calling map_drive, I think you should implement the
> function to check if there is already a mapping.
> 
> Mapping should be always removed before starting the service.
> It is removed when stopping the service, and automatically removed when
> restarting Windows.
> 

"always" is in the best of the worlds without any crash or force shutdown or manual intervention etc.. Please add a check before adding more drives.

thanks