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

[Lukas Venhoda]

On Thu, Feb 25, 2016 at 4:59 PM, Marc-André Lureau <mlureau at redhat.com>
wrote:

>
>
> ----- Original Message -----
> > > + 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.
>
>
Aaah you mean g_poll for the cancel_map. Yeah that would work.


> Also please explain the point of 0.5s delay.
>

If sharing is disabled, read_thread will immediately return and cause the
main_loop to stop.
This in turn causes the cancellable object to be canceled and the loop
stops.

If nothing happens in that 0.5 second (no cancel is called), it means, that
sharing is enabled,
because read_thread (and main loop) is blocked, and we can call map_drive.

This could probably be even less. I started with 5s, and downed it to 0.5s,
but since the actual mapping takes about 5 seconds, it wouldn't make much
difference.

Anyway will change to g_cancellable_make_pollfd + g_poll.


>
> > > +#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
>

Ok will add the check.

-- 
Lukas Venhoda
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160225/6d25c812/attachment-0001.html>