Xen devel - Nov 2007 - [PATCH] Recover from corrupt tdb on reboot

This patch was created for 3.1.0.  Based on inspection, I believe the
current version has the same problem, but I did not actually try it.
I hope you''ll find the patch useful anyway.

Xen cannot work when xenstored''s tdb is corrupt.  When that happens
somehow (and we''ve seen it happen), even reboot doesn''t
recover from
it.  It could: there is no state in tdb that needs to be persisted
across reboots.

The appended patch arranges that tdb is removed before xenstored is
started, provided it doesn''t already run.  This is safe, because:

* xenstored cannot be restarted.  If it dies, Xen''s screwed until
  reboot.

* /usr/sbin/xend always starts xenstored anyway.

* xenstored locks its pid-file (see write_pidfile() in
  tools/xenstore/xenstored_core.c), and refuses to start when it
  can''t.

* My patch makes /usr/sbin/xend remove tdb iff it can lock the
  pid-file.  In other words, it removes tdb only when xenstored is not
  running, and locks it out until it is done.

  Bonus fix: it also removes stale copies of the tdb xenstored tends
  to leave behind when it exits uncleanly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>



diff -r 3191627e5ad6 tools/misc/xend
--- a/tools/misc/xend	Wed Oct 31 16:21:18 2007 +0000
+++ b/tools/misc/xend	Mon Nov 05 18:23:11 2007 +0100
@@ -23,6 +23,8 @@
    On Solaris, the daemons are SMF managed, and you should not attempt
    to start xend by hand.
 """
+import fcntl
+import glob
 import os
 import os.path
 import sys
@@ -76,6 +78,23 @@ def check_user():
         raise CheckError("invalid user")
 
 def start_xenstored():
+    pidfname = "/var/run/xenstore.pid"
+    try:
+        f = open(pidfname, "a")
+        try:
+            fcntl.lockf(f, fcntl.LOCK_EX | fcntl.LOCK_NB)
+            rootdir = os.getenv("XENSTORED_ROOTDIR") or
"/var/lib/xenstored"
+            for i in glob.glob(rootdir + "/tdb*"):
+                try:
+                    os.unlink(i)
+                except:
+                    pass
+            os.unlink(pidfname)
+        except:
+            pass
+        f.close()
+    except:
+        pass
     XENSTORED_TRACE = os.getenv("XENSTORED_TRACE")
     cmd = "xenstored --pid-file /var/run/xenstore.pid"
     if XENSTORED_TRACE:

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

On Mon, Nov 05, 2007 at 06:36:19PM +0100, Markus Armbruster wrote:
> Xen cannot work when xenstored''s tdb is corrupt.  When that
happens
> somehow (and we''ve seen it happen), even reboot doesn''t
recover from
> it.  It could: there is no state in tdb that needs to be persisted
> across reboots.
We''re putting the tdb in a tmpfs (in the short-term), is there a reason
you can''t do that too? Especially as it''s way faster.
> The appended patch arranges that tdb is removed before xenstored is
> started, provided it doesn''t already run.  This is safe, because:
> 
> * xenstored cannot be restarted.  If it dies, Xen''s screwed until
>   reboot.
That will hopefully not always be the case. At that point we''ll need a
different solution (tmpfs or not).

regards
john

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

On Mon, Nov 05, 2007 at 07:43:19PM +0000, John Levon
wrote:
> On Mon, Nov 05, 2007 at 06:36:19PM +0100, Markus Armbruster wrote:
> 
> > Xen cannot work when xenstored''s tdb is corrupt.  When that
happens
> > somehow (and we''ve seen it happen), even reboot
doesn''t recover from
> > it.  It could: there is no state in tdb that needs to be persisted
> > across reboots.
> 
> We''re putting the tdb in a tmpfs (in the short-term), is there a
reason
> you can''t do that too? Especially as it''s way faster.
That''s exactly what we''re doing in Fedora 9, but this patch
was really
for the benefit of any existing deployment who might not want to change
their setup for TDB storage. If we want to switch xen-unstable to use
tmpfs by default, then this patch would likely be unnecessary....

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

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

On Mon, Nov 05, 2007 at 07:56:59PM +0000, Daniel P. Berrange wrote:
> > > Xen cannot work when xenstored''s tdb is corrupt.  When
that happens
> > > somehow (and we''ve seen it happen), even reboot
doesn''t recover from
> > > it.  It could: there is no state in tdb that needs to be
persisted
> > > across reboots.
> > 
> > We''re putting the tdb in a tmpfs (in the short-term), is
there a reason
> > you can''t do that too? Especially as it''s way
faster.
> 
> That''s exactly what we''re doing in Fedora 9, but this
patch was really
> for the benefit of any existing deployment who might not want to change
> their setup for TDB storage. If we want to switch xen-unstable to use
> tmpfs by default, then this patch would likely be unnecessary....
Keir, others? Given that xenstored restartability is quite some way off?

regards
john

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

On 5/11/07 20:03, "John Levon" <levon@movementarian.org> wrote:
>> That''s exactly what we''re doing in Fedora 9, but this
patch was really
>> for the benefit of any existing deployment who might not want to change
>> their setup for TDB storage. If we want to switch xen-unstable to use
>> tmpfs by default, then this patch would likely be unnecessary....
> 
> Keir, others? Given that xenstored restartability is quite some way off?
I think this patch is sensible given where we are with xenstored right now.
Not everyone might want to run with xenstored''s tdb on tmpfs.

 -- Keir



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

On Tue, Nov 06, 2007 at 07:08:59AM +0000, Keir Fraser wrote:
> >> That''s exactly what we''re doing in Fedora 9, but
this patch was really
> >> for the benefit of any existing deployment who might not want to
change
> >> their setup for TDB storage. If we want to switch xen-unstable to
use
> >> tmpfs by default, then this patch would likely be unnecessary....
> > 
> > Keir, others? Given that xenstored restartability is quite some way
off?
> 
> I think this patch is sensible given where we are with xenstored right now.
> Not everyone might want to run with xenstored''s tdb on tmpfs.
Can you think of someone who might not, and what reason they would have?

regards
john

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

On 6/11/07 13:54, "John Levon" <levon@movementarian.org> wrote:
>>> Keir, others? Given that xenstored restartability is quite some way
off?
>> 
>> I think this patch is sensible given where we are with xenstored right
now.
>> Not everyone might want to run with xenstored''s tdb on tmpfs.
> 
> Can you think of someone who might not, and what reason they would have?
The main argument for taking this patch is that it fixes a real user problem
that I have personally had emails about, and it doesn''t look like
xenstored
is actually going to be changed to use tmpfs before 3.2.0. So this is a fine
patch for 3.2.0.

 -- Keir



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

On Tue, Nov 06, 2007 at 04:01:56PM +0000, Keir Fraser wrote:
> >> I think this patch is sensible given where we are with xenstored
right now.
> >> Not everyone might want to run with xenstored''s tdb on
tmpfs.
> > 
> > Can you think of someone who might not, and what reason they would
have?
> 
> The main argument for taking this patch is that it fixes a real user
problem
> that I have personally had emails about, and it doesn''t look like
xenstored
> is actually going to be changed to use tmpfs before 3.2.0. So this is a
fine
> patch for 3.2.0.
That''s fine, I suppose, even though the tmpfs change is much simpler,
and improves things above and beyond.

regards
john

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