RE: [Xen-devel] Why xs_domain_open() in fs_backend

["Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>]

>-----Original Message-----
>From: Tim Deegan [mailto:Tim.Deegan@xxxxxxxxxx]
>Sent: Wednesday, October 13, 2010 4:48 PM
>To: Jiang, Yunhong
>Cc: samuel.thibault@xxxxxxxxxxxxx; gm281@xxxxxxxxx;
>xen-devel@xxxxxxxxxxxxxxxxxxx
>Subject: Re: [Xen-devel] Why xs_domain_open() in fs_backend
>
>At 08:19 +0100 on 13 Oct (1286957982), Jiang, Yunhong wrote:
>> Later I noticed that the fs-backend utilize xs_domain_open(), instead
>> of xs_daemon_open(), to communicate with xenstore. grep
>> "xs_domain_open", I noticed it is in fact only used in
>> xenstore_client.c, (xl use it when xs_daemon_open failed, but I
>> suspect if it is really tested), and the xenstore_client.c does not
>> use watch feature at all.
>
>xs_domain_open() is required for xenstore clients to work when they're
>not in the same domain as xenstored (either because xenstore is in a
>stub domain or because the client is not in dom0).  Removing it is
>definitely wrong.

When I firstly checking the code, I also think the xs_domain_open() should be 
more approprate. But it is a bit surprise that it is not used at all, except 
one utility , xentore-client.c , in xenstore directory, which definitely not 
need this (I assume xenstore-ls should always be in same domain as xenstore 
daemon). 
I suspect if xs_domain_open() and the xenfs is really widely tested. Does 
Citrix has any test suite to cover it?

>
>(IMHO xs_daemon_open() should be killed entirely, but there are some
>dom0 kernels where the xs_domain_open() connection isn't allowed to send
>XS_INTRODUCE commands.  That shouldn't make a difference here, though).

But we should at least add xs_domain_close() firstly, matching xs_domain_open() 
with xs_deamon_close() is really something strange :-)

>
>> Following simple patch make it work, but I'm not sure if it is the
>> right method, will fs-backend run in other domain?
>
>What does the change to ROOT_NODE do in this patch?  Any chance that it
>fixes the problem by itself? :)

No, it does not work.

Thanks
--jyh

>
>Cheers,
>
>Tim.
>
>> Thanks
>> --jyh
>>
>> diff -r a33886146b45 tools/fs-back/fs-backend.c
>> --- a/tools/fs-back/fs-backend.c    Fri Oct 08 11:41:57 2010 +0100
>> +++ b/tools/fs-back/fs-backend.c    Wed Oct 13 15:10:22 2010 +0800
>> @@ -462,7 +462,7 @@ int main(void)
>>      sigaction(SIGUSR2, &act, NULL);
>>
>>      /* Open the connection to XenStore first */
>> -    xsh = xs_domain_open();
>> +    xsh = xs_daemon_open();
>>      assert(xsh != NULL);
>>      xs_rm(xsh, XBT_NULL, ROOT_NODE);
>>      /* Create watch node */
>> diff -r a33886146b45 tools/fs-back/fs-backend.h
>> --- a/tools/fs-back/fs-backend.h    Fri Oct 08 11:41:57 2010 +0100
>> +++ b/tools/fs-back/fs-backend.h    Wed Oct 13 15:10:33 2010 +0800
>> @@ -9,7 +9,7 @@
>>  #include <xen/io/fsif.h>
>>  #include "sys-queue.h"
>>
>> -#define ROOT_NODE           "backend/vfs"
>> +#define ROOT_NODE           "/local/domain/0/backend/vfs"
>>  #define EXPORTS_SUBNODE     "exports"
>>  #define EXPORTS_NODE        ROOT_NODE"/"EXPORTS_SUBNODE
>>  #define WATCH_NODE          EXPORTS_NODE"/requests"
>> ~
>> ~
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
>
>--
>Tim Deegan <Tim.Deegan@xxxxxxxxxx>
>Principal Software Engineer, XenServer Engineering
>Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel