<div dir="ltr">Hi<br><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 25, 2016 at 4:21 PM, Marc-André Lureau <span dir="ltr"><<a href="mailto:marcandre.lureau@gmail.com" target="_blank">marcandre.lureau@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi<br>
<div><div class="h5"><br>
On Thu, Feb 25, 2016 at 3:19 PM, Lukas Venhoda <<a href="mailto:lvenhoda@redhat.com">lvenhoda@redhat.com</a>> wrote:<br>
> +static void<br>
> +map_drive(GTask *task,<br>
> +          gpointer source_object,<br>
> +          gpointer task_data,<br>
> +          GCancellable *cancellable)<br>
> +{<br>
> +  static int loop_time = G_USEC_PER_SEC/10;<br>
> +  GCancellable * cancel_map = task_data;<br>
> +  gchar local_name[3];<br>
> +  gchar remote_name[] = "<a href="http://localhost:9843/" rel="noreferrer" target="_blank">http://localhost:9843/</a>";<br>
> +  NETRESOURCE net_resource;<br>
> +  guint32 retval;<br>
> +  gint i;<br>
> +<br>
> +  for (i = 0; i < 5; ++i)<br>
> +    {<br>
> +      if (g_cancellable_is_cancelled (cancel_map))<br>
> +        return;<br>
> +      else<br>
> +        g_usleep (loop_time);<br>
> +    }<br>
> +<br>
<br>
</div></div>I am not sure I see the point of waiting 0.5s here. Add a comment? (a<br>
better way would be to use g_poll with the pollfd cancellable can<br>
creats: no need to loop)<br>
<span class=""><br></span></blockquote><div><br></div><div>I tryed this using <code class="">g_io_channel_win32_make_pollfd(), but the way the FD<br>(well actually file handle) is created on windows, it doesn't support polling.<br></code></div><div><code class="">It might require a more in depth rewrite of the windows part of webdavd to make it work.<br></code></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
> +  if ((drive_letter = get_free_drive_letter ()) == 0)<br>
> +      g_error ("all drive letters already assigned.");<br>
<br>
</span>This will abort(), I think it's better to have simply a critical. The<br>
webdav folder can be accessed through other means (browser etc)<br></blockquote><div><br></div><div>Will change to critical<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> +  local_name[0] = drive_letter;<br>
> +  local_name[1] = ':';<br>
> +  local_name[2] = 0;<br>
<br>
</span>This is not strictly equivalent to "net use *", it will be racy. It<br>
probably needs to retry if the localname is already used.<br></blockquote><div><br></div><div>That's why there's <span class="">get_free_drive_letter function().<br>The localname will either be usable, or the driver cannot be mapped.<br></span></div><div><span class="">Do you know </span>equivalent function to net use? I could find one.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> +  net_resource.dwType = RESOURCETYPE_DISK;<br>
> +  net_resource.lpLocalName = local_name;<br>
> +  net_resource.lpRemoteName = remote_name;<br>
> +  net_resource.lpProvider = NULL;<br>
> +<br>
> +  retval = WNetAddConnection2 (&net_resource, NULL, NULL, CONNECT_TEMPORARY);<br>
> +<br>
> +  if (retval == NO_ERROR)<br>
> +    g_debug ("map_drive ok");<br>
> +  else if (retval == ERROR_ALREADY_ASSIGNED)<br>
> +    g_debug ("map_drive already asigned");<br>
<br>
</span>here, goto getfreedriver?<br></blockquote><div><br></div><div>If someone maps the drive before I can?<br></div><div>Well a simple do while loop can't hurt here.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> +  else<br>
> +    g_error ("map_drive error %d", retval);<br>
<br>
</span>Please avoid g_error for non-fatal issues.<br></blockquote><div><br></div><div>Will change to critical<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
The map-driver.bat file uses a nifty hack to name the folder "Spice<br>
client" (Spice folder would make more sense). It would be really nice<br>
to name the folder here too.<br>
<span class=""><br></span></blockquote><div><br></div><div>Yes, I forgot about that, because I already have the reg hack in my registry.<br></div><div>I can try to reproduce it programatically.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
> +<br>
> +  return;<br>
> +}<br>
> +#endif<br>
> +<br>
>  static void<br>
>  run_service (void)<br>
>  {<br>
>    g_debug ("Run service");<br>
><br>
> +#ifdef G_OS_WIN32<br>
> +  GCancellable * cancel_map = g_cancellable_new ();<br>
> +  GTask * map_drive_task = g_task_new (NULL, NULL, NULL, NULL);<br>
<br>
</span>no unref? you probably should g_task_return() somewhere too.<br></blockquote><div><br></div><div>If I don't set the GTask to return-on-cancel, I don't need to call g_task_return(),<br>because the callback is empty. </div><div><br>From the GTask doc:<br>Other than in that case, <em class=""><code>task</code></em>
 will be completed when the
<a class="" href="https://developer.gnome.org/gio/stable/GTask.html#GTaskThreadFunc" title="GTaskThreadFunc ()"><span class="">GTaskThreadFunc</span></a> returns,<br>not when it calls a
<code class="">g_task_return_</code> function.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> +  g_task_set_task_data (map_drive_task, cancel_map, NULL);<br>
> +  g_task_run_in_thread (map_drive_task, map_drive);<br>
<br>
</span>I think you could unref after this call, since run_in_thread() keeps a ref.<br>
<br>
Before adding and calling map_drive, I think you should implement the<br>
function to check if there is already a mapping.<br></blockquote><div><br></div><div>Mapping should be always removed before starting the service.<br></div><div>It is removed when stopping the service, and automatically removed when restarting Windows.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> +#endif<br>
> +<br>
>    g_socket_service_start (socket_service);<br>
><br>
>    cancel = g_cancellable_new ();<br>
> @@ -775,6 +852,10 @@ run_service (void)<br>
>    g_main_loop_run (loop);<br>
>    g_main_loop_unref (loop);<br>
><br>
> +#ifdef G_OS_WIN32<br>
> +  g_cancellable_cancel (cancel_map);<br>
<br>
</span>no unref either?<br></blockquote><div><br></div><div>Will add the necessary unrefs.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> +#endif<br>
> +<br>
>    g_cancellable_cancel (cancel);<br>
><br>
>    g_clear_object (&mux_istream);<br>
> --<br>
> 2.5.0<br>
><br>
</span>> _______________________________________________<br>
> Spice-devel mailing list<br>
> <a href="mailto:Spice-devel@lists.freedesktop.org">Spice-devel@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/spice-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/spice-devel</a><br>
<span class=""><font color="#888888"><br>
<br>
<br>
--<br>
Marc-André Lureau<br>
</font></span></blockquote></div><br>-- <br><div class="gmail_signature">Lukas Venhoda</div>
</div></div></div>