Xenstore remove problem - causes bug #473

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Xenstore remove problem - causes bug #473

Murillo Bernardes-2
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
[hidden email]
http://lists.xensource.com/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: Xenstore remove problem - causes bug #473

Ewan Mellor
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
[hidden email]
http://lists.xensource.com/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: Xenstore remove problem - causes bug #473

Keir Fraser
In reply to this post by Murillo Bernardes-2

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
[hidden email]
http://lists.xensource.com/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: Xenstore remove problem - causes bug #473

Murillo Bernardes-2
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
[hidden email]
http://lists.xensource.com/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: Xenstore remove problem - causes bug #473

Murillo Bernardes-2
In reply to this post by Keir Fraser
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
[hidden email]
http://lists.xensource.com/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: Xenstore remove problem - causes bug #473

Keir Fraser

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
[hidden email]
http://lists.xensource.com/xen-devel