Re: [Xen-devel] [PATCH 2/7] libvchan: create xenstore entries in one transaction

[Marek Marczykowski <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On 26.04.2013 20:04, David Vrabel wrote:
> On 21/04/13 02:46, Marek Marczykowski wrote:
>> This will prevent race when client waits for server with xs_watch - all
>> entries should appear at once.
> 
> Aren't xenstore writes strictly ordered?  The client just needs to read
> the keys in the inverse order to avoid races, yes?

IMO this isn't the best idea - application gives only xenstore directory to
libxenvchan_*_init, I see no reason to assume anything in application:
a) what entries will be created/expected there,
b) in which order,
c) and it will not be changed in the future.

Adding it in one transaction seems to be better idea. Also this will fix
hypothetical race between two simultaneously started servers - when entries
are added without transaction it can happen to end up with event-channel from
one server and ring-ref from another.


> David
> 
>>
>> Signed-off-by: Marek Marczykowski <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
>> ---
>>  tools/libvchan/init.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
>> index f0d2505..dbdcc6c 100644
>> --- a/tools/libvchan/init.c
>> +++ b/tools/libvchan/init.c
>> @@ -234,6 +234,7 @@ static int init_xs_srv(struct libxenvchan *ctrl, int 
>> domain, const char* xs_base
>>      char buf[64];
>>      char ref[16];
>>      char* domid_str = NULL;
>> +    xs_transaction_t xs_trans = NULL;
>>      xs = xs_domain_open();
>>      if (!xs)
>>              goto fail;
>> @@ -249,20 +250,29 @@ static int init_xs_srv(struct libxenvchan *ctrl, int 
>> domain, const char* xs_base
>>      perms[1].id = domain;
>>      perms[1].perms = XS_PERM_READ;
>>  
>> +retry_transaction:
>> +    xs_trans = xs_transaction_start(xs);
>> +    if (!xs_trans)
>> +            goto fail_xs_open;
>> +
>>      snprintf(ref, sizeof ref, "%d", ring_ref);
>>      snprintf(buf, sizeof buf, "%s/ring-ref", xs_base);
>> -    if (!xs_write(xs, 0, buf, ref, strlen(ref)))
>> +    if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
>>              goto fail_xs_open;
>> -    if (!xs_set_permissions(xs, 0, buf, perms, 2))
>> +    if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
>>              goto fail_xs_open;
>>  
>>      snprintf(ref, sizeof ref, "%d", ctrl->event_port);
>>      snprintf(buf, sizeof buf, "%s/event-channel", xs_base);
>> -    if (!xs_write(xs, 0, buf, ref, strlen(ref)))
>> +    if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
>>              goto fail_xs_open;
>> -    if (!xs_set_permissions(xs, 0, buf, perms, 2))
>> +    if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
>>              goto fail_xs_open;
>>  
>> +    if (!xs_transaction_end(xs, xs_trans, 0))
>> +            if (errno == EAGAIN)
>> +                    goto retry_transaction;
>> +
>>      ret = 0;
>>   fail_xs_open:
>>      free(domid_str);
> 


-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel