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

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

 



On Thu, Feb 25, 2016 at 4:59 PM, Marc-André Lureau <mlureau@xxxxxxxxxx> 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
_______________________________________________
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]