Xen devel - Jan 2006 - Xenstore remove problem - causes bug #473

Hey folks,

Xenstore remove is removing more than it is supposed to. Like:

xenstore-rm /local/domain/0/backend/vif/1

removes /local/domain/0/backend/vif/1*

That causes bug #473

I tried to find the problem in xenstore_client.c, but it seems to be ok. 
Probably the problem in xenstored, but I could not find where exactly.

Any clue?

-- 
Murillo Fernandes Bernardes
IBM Linux Technology Center

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

On Tue, Jan 10, 2006 at 11:01:23AM -0200, Murillo Bernardes wrote:
> Hey folks,
> 
> Xenstore remove is removing more than it is supposed to. Like:
> 
> xenstore-rm /local/domain/0/backend/vif/1
> 
> removes /local/domain/0/backend/vif/1*
> 
> That causes bug #473
I get

~ # xenstore-rm /tools/ewan
~ # xenstore-write /tools/ewan/12/1 v
~ # xenstore-write /tools/ewan/1/1 v
~ # xenstore-write /tools/ewan/13/1 v
~ # xenstore-rm /tools/ewan/1
~ # xenstore-list /tools/ewan
12
13

Which is as is expected, and I get the same when I use the -t flag to
xenstore-rm.  What is your test case exactly?

If you export XENSTORED_TRACE=1 and restart xenstored then you will get a
/var/log/xenstored-trace.log which may be of some use.  Beware that restarting
Xenstored breaks your connections to your backend drivers, so a full reboot is
required if you want to create domains afterwards.

Cheers,

Ewan.

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

On 10 Jan 2006, at 13:01, Murillo Bernardes wrote:
> Xenstore remove is removing more than it is supposed to. Like:
>
> xenstore-rm /local/domain/0/backend/vif/1
>
> removes /local/domain/0/backend/vif/1*
>
> That causes bug #473
>
> I tried to find the problem in xenstore_client.c, but it seems to be 
> ok.
> Probably the problem in xenstored, but I could not find where exactly.
>
> Any clue?
I''m not sure it''s that simple. Try:

xenstore-write /foo/1/bar 0
xenstore-write /foo/11/bar 0
xenstore-rm /foo/1

I find that /foo/11 is not deleted.

Perhaps something is watching vif/1 and, when the watch fires, 
erroneously causing all vif/1* to get deleted?

The xenstored code of interest, by the way, is in 
xenstored_core.c:do_rm()

  -- Keir


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

On Tuesday 10 January 2006 12:19, Keir Fraser wrote:
> I''m not sure it''s that simple. Try:
>
> xenstore-write /foo/1/bar 0
> xenstore-write /foo/11/bar 0
> xenstore-rm /foo/1
>
> I find that /foo/11 is not deleted.
>
You''re right
> Perhaps something is watching vif/1 and, when the watch fires,
> erroneously causing all vif/1* to get deleted?
>
makes sense. I''m trying to figure out...

-- 
Murillo Fernandes Bernardes
IBM Linux Technology Center

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

On Tuesday 10 January 2006 12:19, Keir Fraser wrote:
> Perhaps something is watching vif/1 and, when the watch fires,
> erroneously causing all vif/1* to get deleted?
>
I found it!

When a change occur on one backend device we call cleanup_devices() for each 
device in bus. 

The problem is: the test to see if that device was changed is being done with 
strncmp(), using the length of the changed node as maximum length to be 
compared. 

I think the idea is that any sub-node should be matched also, like: if 
"backend/1/status" changed we have to call backend_changed to
"backend/1"
device, and that is ok. But with that code If we delete device
"backend/1"
all devices matching "backend/1*" are being deleted also.

Suggestions on the best/prettier way to handle that?

-- 
Murillo Fernandes Bernardes
IBM Linux Technology Center

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

On 13 Jan 2006, at 12:28, Murillo Bernardes wrote:
> I think the idea is that any sub-node should be matched also, like: if
> "backend/1/status" changed we have to call backend_changed to 
> "backend/1"
> device, and that is ok. But with that code If we delete device 
> "backend/1"
> all devices matching "backend/1*" are being deleted also.
That can''t be the logic. If ''/backend/1/status''
changed then we would
not strncmp match on ''/backend/1''.

The opposite way round would match though. Perhaps that was the intent.

Anyway, I can implement a fix that maintains the current semantics 
w.r.t. matching on subdirectories.

  -- Keir


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