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

[Lukas Venhoda]

Hi
On Thu, Sep 17, 2015 at 2:47 PM, Marc-André Lureau <mlureau at redhat.com>
wrote:

> Hi
>
> ----- Original Message -----

> How do you prevent from having multiple drive pointing to the spice
> folder?
> >
> >
> > Currently, I don't.
> >
> > Only way to connect multiple drives, is if you restart the service,
> > because the drives will disconnect, after restarting guest.
> >
> > I will add a disconnect function to the quit function in the service,
> > so that if uou disable, or restart teh service, the drive will be
> unmapped
> >
>
> Can't you enumerate existing mappings and only add one if it's necessary?
>
>
Yes there seems to be API for that, so I can try to implement it.

Still I'll make a disconnect function anyway, because it doesn't make sense
to have a drive mapped, when the webdavd service is not even running


> > > ---
> > > Changes since RFC:
> > > - Calling WNetAddConnection2() blindly was slow and caause many
> threads to
> > > spawn.
> > > - Now only call it once, when it is sure, that it will connect.
> > > - Now connects to a * drive, instead of always Z:
> > > - Thread is now spawned and joined every time run_service() is called.
> > > ---
> > > Makefile.am | 4 +++
> > > spice/spice-webdavd.c | 76
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 80 insertions(+)
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 485417b..df9a73e 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -87,6 +87,10 @@ spice_webdavd_LDADD = \
> > > $(PIE_LDFLAGS) \
> > > $(NULL)
> > >
> > > +if OS_WIN32
> > > +spice_webdavd_LDADD += -lnetapi32 -lmpr
> > > +endif
> > > +
> > > deps.txt:
> > > $(AM_V_GEN)rpm -qa | grep $(host_os) | sort | unix2dos > $@
> > >
> > > diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> > > index f9c5cf1..3940f1c 100644
> > > --- a/spice/spice-webdavd.c
> > > +++ b/spice/spice-webdavd.c
> > > @@ -737,11 +737,81 @@ open_mux_path (const char *path)
> > > mux_queue = output_queue_new (G_OUTPUT_STREAM (mux_ostream));
> > > }
> > >
> > > +#ifdef G_OS_WIN32
> > > +static gchar
> > > +get_free_drive_letter(void)
> > > +{
> > > + const guint32 max_mask = 1 << 25;
> > > + guint32 drives;
> > > + guint32 mask;
> > > + gint i;
> > > +
> > > + drives = GetLogicalDrives ();
> > > +
> > > + for (i = 0; i < 26; i++)
> > > + {
> > > + mask = max_mask >> i;
> > > + if ((drives & mask) == 0)
> > > + return 'Z' - i;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static gpointer
> > > +map_drive(gpointer user_data)
> > > +{
> > > + GCancellable * cancel_map = user_data;
> > > + gchar drive_letter;
> > > + 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 NULL;
> > > + else
> > > + g_usleep (G_USEC_PER_SEC);
> > > + }
> >
> > It looks like this would fail if the guest is started without client
> > connected (or not sharing folder).
> >
> > I'm not sure I follow.
> > This is supposed to return NULL precisely, if the client is not
> connected,
> > or not sharing folder. Windows webdavd will loop and call run_service(),
> > until sharing is enabled, which will then NOT cancel this function,
> > and we map the drive.
>
> ah so the service keeps running and next time the client connects, then
> the thread will be started again?
>

No, the thread starts only after the service is started for the first time.
When user disconnects, the service
currently doesn't notice on Windows (read_thread is still blocked), so when
a different user connects,
the drive is still mapped, but when the new user enables sharing, the
original drive will work, as it connects
to the same remote name (http://localhost:9843/).

This is still an issue, because if the new user doesn't enable sharing, the
drive will still be there, but won't
be accessible. But that is issue for another patch (and bug).

...

> Sorry I didn't mean what the thread was doing, but rather why we need a
thread for this task.

Well I could use a g_task or g_idle, or something like that, but g_idle
isn't cancellable, and g_task
seemed too complicated for this. When I tried to do this in the main_thread
(as it just has to be a
different thread then read_thread), I ran into race conditions between the
read_thread and map_drive,
especially, when using sleep between polling for cancellable.
Spawning a thread for a few seconds just seemed to be less complicated.

As for why this has to be in a different thread, then read_thread, is
because if I call map_drive before
read actually blocks, it returns error, as if it's not connected. I think
this is because when we try to map
the drive, client sends a message to the guest, and the read has to be
ready to receive it, which in turn
lets the map_drive to actually map the drive.

I tryed using every possible way I could think of, even windows specific
polling, but I couldn't make
anything work, except just plain spawning another thread.

Lukas Venhoda
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150917/3e210011/attachment-0001.html>