Created attachment 329389 [details] [review]
afc: Add more debug when initialisation fails

Created attachment 329390 [details] [review]
afc: Fix volume init with a locked device

The first time we connect the device, we won't be able to create a
lockdown client with a handshake, so the volume won't be created at all
and will not show up in nautilus.

Furthermore, we were checking the OS version with a paired lockdown
client (which is not needed, an unpaired one will do just as well)
and checking whether HouseArrest was supported even if the service was
different.

So, we never needed to have a paired lockdown client in the first place,
and we only need to check for the OS version if HouseArrest is what was
requested.

Created attachment 329391 [details] [review]
afc: Fix indentation

Created attachment 329392 [details] [review]
afc: Better lockdown client creation debug

Created attachment 329393 [details] [review]
afc: Fix mount failing after trusting device

After having trusted the device, if we are too quick at clicking the
"Try Again" button, the creation will fail with an unknown error. Try
again silently up to the maximum number of retries after a short sleep.

Review of attachment 329389 [details] [review]:

Why not...

::: monitor/afc/afcvolume.c
@@ +142,3 @@
   if (err != IDEVICE_E_SUCCESS)
+    {
+      g_debug ("Failed to initialise an idevice_t (%d)", err);

Just you have to add trailing "\n" (I know that this is not usual, but see g_log_set_handler), this applies on other recent afc patches also, sorry I haven't realized this earlier...

Review of attachment 329390 [details] [review]:

Hmm, makes sense, however as I told before, please test it properly...

::: monitor/afc/afcvolume.c
@@ +150,2 @@
     {
+      g_debug ("Failed to create a client with handshake (%d)", lerr);

\n

@@ +159,2 @@
         {
+          g_debug ("Failed to check HouseArrest version");

\n

Review of attachment 329391 [details] [review]:

I personally don't like patches to fix indentation at all, however if you really want to fix the indentation, so please fix it also in other places, e.g.:
g_vfs_backend_afc_check
g_vfs_backend_idevice_check
...

Review of attachment 329392 [details] [review]:

Looks good otherwise!

::: daemon/gvfsbackendafc.c
@@ +275,3 @@
     return 0;
 
+  g_debug ("Got lockdown error '%d' while doing '%s'",

\n

@@ +601,3 @@
     char *message;
 
+    g_debug ("Lockdown client try #%d", retries);

\n

Review of attachment 329393 [details] [review]:

Looks good!

::: daemon/gvfsbackendafc.c
@@ +620,3 @@
+    if (lerr == LOCKDOWN_E_UNKNOWN_ERROR)
+      {
+        g_usleep (G_USEC_PER_SEC);

It may be useful to add g_debug also in this place...

(In reply to Ondrej Holy from comment #6)
> Review of attachment 329389 [details] [review] [review]:
> 
> Why not...
> 
> ::: monitor/afc/afcvolume.c
> @@ +142,3 @@
>    if (err != IDEVICE_E_SUCCESS)
> +    {
> +      g_debug ("Failed to initialise an idevice_t (%d)", err);
> 
> Just you have to add trailing "\n" (I know that this is not usual, but see
> g_log_set_handler), this applies on other recent afc patches also, sorry I
> haven't realized this earlier...

There's no calls to g_log_set_handler in the monitor.

Attachment 329389 [details] pushed as 54658d0 - afc: Add more debug when initialisation fails
Attachment 329390 [details] pushed as 98fb7cf - afc: Fix volume init with a locked device
Attachment 329392 [details] pushed as 478b069 - afc: Better lockdown client creation debug
Attachment 329393 [details] pushed as 8c8aa6a - afc: Fix mount failing after trusting device

(In reply to Bastien Nocera from comment #11)
> (In reply to Ondrej Holy from comment #6)
> > Review of attachment 329389 [details] [review] [review] [review]:
> > 
> > Why not...
> > 
> > ::: monitor/afc/afcvolume.c
> > @@ +142,3 @@
> >    if (err != IDEVICE_E_SUCCESS)
> > +    {
> > +      g_debug ("Failed to initialise an idevice_t (%d)", err);
> > 
> > Just you have to add trailing "\n" (I know that this is not usual, but see
> > g_log_set_handler), this applies on other recent afc patches also, sorry I
> > haven't realized this earlier...
> 
> There's no calls to g_log_set_handler in the monitor.

Ok, my fault, but applies on the rest...

And big thanks for all those AFC related patches!