# HG changeset patch
# User Jim Fehlig <jfehlig@novell.com>
# Date 1289324668 25200
# Node ID fb0498c0cbdd6be20bbcb5560779ad6f380fe9e8
# Parent 985f5fa8fc59f84c8577c482df6246258c785991
xend: drbd improvements
1) drbdadm state is debrecated, and the script tries to detect the
primary/secondary information from the deprecated message
2) os.popen2 does not work with the array as argument
3) popen2 is deprecated since python2.6
Re: http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1685
From: Berthold Gunreben <bg@suse.de>
Signed-off-by: Jim Fehlig <jfehlig@novell.com>
diff -r 985f5fa8fc59 -r fb0498c0cbdd tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py Mon Nov 08 17:25:54 2010 +0000
+++ b/tools/python/xen/xend/XendDomainInfo.py Tue Nov 09 10:44:28 2010 -0700
@@ -35,6 +35,7 @@
import shutil
import traceback
from types import StringTypes
+from subprocess import *
import xen.lowlevel.xc
from xen.util import asserts, auxbin, mkdir
@@ -3254,7 +3255,9 @@
if disk.find(":") != -1:
(disktype, diskname) = disk.split('':'', 1)
if disktype == ''drbd'':
- (drbdadmstdin, drbdadmstdout) =
os.popen2(["/sbin/drbdadm", "state", diskname])
+ p = Popen(["/sbin/drbdadm", "role",
diskname],
+ stdin=PIPE, stdout=PIPE, close_fds=True )
+ (drbdadmstdin, drbdadmstdout) = (p.stdin, p.stdout)
(state, junk) =
drbdadmstdout.readline().split(''/'', 1)
if state == ''Secondary'':
os.system(''/sbin/drbdadm primary '' +
diskname)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Jim Fehlig writes ("[Xen-devel] [PATCH] xend: drbd
improvements"):> xend: drbd improvements
>
> 1) drbdadm state is debrecated, and the script tries to detect the
> primary/secondary information from the deprecated message
> 2) os.popen2 does not work with the array as argument
> 3) popen2 is deprecated since python2.6
Forgive my ignorance, but: why does xend need to mess with drbd at
all ? Why can''t it just open the block device ?
xl and libxl don''t have any drbd functionality so if this is necessary
it''s a missing feature.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson wrote:> Jim Fehlig writes ("[Xen-devel] [PATCH] xend: drbd improvements"): > >> xend: drbd improvements >> >> 1) drbdadm state is debrecated, and the script tries to detect the >> primary/secondary information from the deprecated message >> 2) os.popen2 does not work with the array as argument >> 3) popen2 is deprecated since python2.6 >> > > Forgive my ignorance, but: why does xend need to mess with drbd at > all ? Why can''t it just open the block device ? >Good question. I too was surprised to see special casing of drbd within xend. Seems c/s 20158 added this code to allow bootloaders (pygrub, domUloader, et. al.) to work with drbd devices using the ''drbd:resource'' syntax. Perhaps the author of that patch (cc''d) can provide more details. This patch simply makes improvements to the existing code. BTW, c/s 20158 also uses popen2() in tools/python/xen/util/blkif.py, so IMO a V2 will be needed if this patch is considered.> xl and libxl don''t have any drbd functionality so if this is necessary > it''s a missing feature. >Micheal, does xl/libxl need this functionality? Regards, Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jim Fehlig wrote:> Ian Jackson wrote: > >> Jim Fehlig writes ("[Xen-devel] [PATCH] xend: drbd improvements"): >> >> >>> xend: drbd improvements >>> >>> 1) drbdadm state is debrecated, and the script tries to detect the >>> primary/secondary information from the deprecated message >>> 2) os.popen2 does not work with the array as argument >>> 3) popen2 is deprecated since python2.6 >>> >>> >> Forgive my ignorance, but: why does xend need to mess with drbd at >> all ? Why can''t it just open the block device ? >> >> > > Good question. I too was surprised to see special casing of drbd within > xend. Seems c/s 20158 added this code to allow bootloaders (pygrub, > domUloader, et. al.) to work with drbd devices using the ''drbd:resource'' > syntax. Perhaps the author of that patch (cc''d) can provide more > details.The patch author''s mail address is no longer valid, so we won''t be receiving any feedback from him :-(. Regards, Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jim Fehlig wrote:> Jim Fehlig wrote: > >> Ian Jackson wrote: >> >> >>> Jim Fehlig writes ("[Xen-devel] [PATCH] xend: drbd improvements"): >>> >>> >>> >>>> xend: drbd improvements >>>> >>>> 1) drbdadm state is debrecated, and the script tries to detect the >>>> primary/secondary information from the deprecated message >>>> 2) os.popen2 does not work with the array as argument >>>> 3) popen2 is deprecated since python2.6 >>>> >>>> >>>> >>> Forgive my ignorance, but: why does xend need to mess with drbd at >>> all ? Why can''t it just open the block device ? >>> >>> >>> >> Good question. I too was surprised to see special casing of drbd within >> xend. Seems c/s 20158 added this code to allow bootloaders (pygrub, >> domUloader, et. al.) to work with drbd devices using the ''drbd:resource'' >> syntax. Perhaps the author of that patch (cc''d) can provide more >> details. >> > > The patch author''s mail address is no longer valid, so we won''t be > receiving any feedback from him :-(. >Sorry for the delay here, but I finally got around to investigating this further. As it turns out, drbd''s block-drbd script handles all of the details that c/s 20158 introduces within xend :-(. IMO, this c/s should be reverted as it causes a regression. I''ve tested drbd without this changeset and it works fine. Regards, Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
According to the block-drbd script''s comments, live migration with DRBD based disks require dual primary setup and the block-drbd script automatically handles "changing the target host''s vm disk to primary" when live migration is initiated. Removing drbd support from xend would necessitate an external agent to actually promote the guest disk in target host to primary mode. shriram On Thu, Dec 16, 2010 at 4:45 PM, Jim Fehlig <jfehlig@novell.com> wrote:> Jim Fehlig wrote: >> Jim Fehlig wrote: >> >>> Ian Jackson wrote: >>> >>> >>>> Jim Fehlig writes ("[Xen-devel] [PATCH] xend: drbd improvements"): >>>> >>>> >>>> >>>>> xend: drbd improvements >>>>> >>>>> 1) drbdadm state is debrecated, and the script tries to detect the >>>>> primary/secondary information from the deprecated message >>>>> 2) os.popen2 does not work with the array as argument >>>>> 3) popen2 is deprecated since python2.6 >>>>> >>>>> >>>>> >>>> Forgive my ignorance, but: why does xend need to mess with drbd at >>>> all ? Why can''t it just open the block device ? >>>> >>>> >>>> >>> Good question. I too was surprised to see special casing of drbd within >>> xend. Seems c/s 20158 added this code to allow bootloaders (pygrub, >>> domUloader, et. al.) to work with drbd devices using the ''drbd:resource'' >>> syntax. Perhaps the author of that patch (cc''d) can provide more >>> details. >>> >> >> The patch author''s mail address is no longer valid, so we won''t be >> receiving any feedback from him :-(. >> > > Sorry for the delay here, but I finally got around to investigating this > further. As it turns out, drbd''s block-drbd script handles all of the > details that c/s 20158 introduces within xend :-(. IMO, this c/s should > be reverted as it causes a regression. I''ve tested drbd without this > changeset and it works fine. > > Regards, > Jim > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >-- perception is but an offspring of its own self _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan wrote:> According to the block-drbd script''s comments, live migration with > DRBD based disks > require dual primary setup and the block-drbd script automatically handles > "changing the target host''s vm disk to primary" when live migration > is initiated. > > Removing drbd support from xend would necessitate an external agent to > actually promote the guest disk in target host to primary mode. >Reverting c/s 20158 does not remove drbd support in xen. On the contrary, it fixes the use of drbd as documented here http://www.drbd.org/users-guide/s-xen-configure-domu.html The drbd project provides block-drbd. I''m not talking about removing it. I''m talking about removing a hack in xend that prevents even using drbd resource type. Regards, Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jim Fehlig writes ("Re: [Xen-devel] [PATCH] xend: drbd
improvements"):> Sorry for the delay here, but I finally got around to investigating this
> further. As it turns out, drbd''s block-drbd script handles all of
the
> details that c/s 20158 introduces within xend :-(. IMO, this c/s should
> be reverted as it causes a regression. I''ve tested drbd without
this
> changeset and it works fine.
Thanks for looking into this and testing things. I have reverted
20158 from xen-unstable.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
I was just looking over the patch on blkif.py and I believe there is a bug.
--- a/tools/python/xen/util/blkif.py Fri Dec 10 18:08:19 2010 +0000
+++ b/tools/python/xen/util/blkif.py Wed Jan 05 23:31:24 2011 +0000
@@ -71,15 +71,8 @@ def _parse_uname(uname):
if uname.find(":") != -1:
(typ, fn) = uname.split(":", 1)
- if typ == "phy" and not fn.startswith("/"):
+ if typ in ("phy", "drbd") and not
fn.startswith("/"):
fn = "/dev/%s" %(fn,)
-
- if typ == "drbd":
- if not fn.startswith("drbd"):
- (drbdadmstdin, drbdadmstdout)
os.popen2(["/sbin/drbdadm", "sh-dev", fn])
- fn = drbdadmstdout.readline().strip()
- else:
- fn = "/dev/%s" %(fn,)
if typ in ("tap", "tap2"):
(taptype, fn) = fn.split(":", 1
When you specify a drbd disk for a domU, its format is
drbd:<resourceName>
wherein drbd expects a resource file in /etc/drbd.d/resourceName.res, that
contains the actual drbd device name (/dev/drbd1 or something).
drbd does not export resources as devices in the /dev/ path (as is
the assumption
in the above patch). It actually exports them under /dev/drbd/by-res/.
Atleast thats
the case in my Debian box, with drbd 8.3.9
I can send out a patch if you folks agree with this.
shriram
On Wed, Jan 5, 2011 at 3:32 PM, Ian Jackson <Ian.Jackson@eu.citrix.com>
wrote:>
> Jim Fehlig writes ("Re: [Xen-devel] [PATCH] xend: drbd
improvements"):
> > Sorry for the delay here, but I finally got around to investigating
this
> > further. As it turns out, drbd''s block-drbd script handles
all of the
> > details that c/s 20158 introduces within xend :-(. IMO, this c/s
should
> > be reverted as it causes a regression. I''ve tested drbd
without this
> > changeset and it works fine.
>
> Thanks for looking into this and testing things. I have reverted
> 20158 from xen-unstable.
>
> Ian.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
--
perception is but an offspring of its own self
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Shriram Rajagopalan wrote:> I was just looking over the patch on blkif.py and I believe there is a bug. > --- a/tools/python/xen/util/blkif.py Fri Dec 10 18:08:19 2010 +0000 > +++ b/tools/python/xen/util/blkif.py Wed Jan 05 23:31:24 2011 +0000 > @@ -71,15 +71,8 @@ def _parse_uname(uname): > if uname.find(":") != -1: > (typ, fn) = uname.split(":", 1) > > - if typ == "phy" and not fn.startswith("/"): > + if typ in ("phy", "drbd") and not fn.startswith("/"): > fn = "/dev/%s" %(fn,) > - > - if typ == "drbd": > - if not fn.startswith("drbd"): > - (drbdadmstdin, drbdadmstdout) > os.popen2(["/sbin/drbdadm", "sh-dev", fn]) > - fn = drbdadmstdout.readline().strip() > - else: > - fn = "/dev/%s" %(fn,) > > if typ in ("tap", "tap2"): > (taptype, fn) = fn.split(":", 1 > > > When you specify a drbd disk for a domU, its format is > drbd:<resourceName> >Correct. Sadly, I forgot I was testing on SLES, which contains local xend patches to fix problems wrt external block scripts. One of the patches has existed for ages, before many of us were working on xen :-). I would really like to get these changes upstream, even though xend is dying. The first patch is a revert of http://xenbits.xensource.com/xen-unstable.hg?rev/152257350930. The second patch, which I''ve rebased against -unstable, is attached. Can you test it? Thanks, Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jim Fehlig wrote:> Shriram Rajagopalan wrote: > >> I was just looking over the patch on blkif.py and I believe there is a bug. >> --- a/tools/python/xen/util/blkif.py Fri Dec 10 18:08:19 2010 +0000 >> +++ b/tools/python/xen/util/blkif.py Wed Jan 05 23:31:24 2011 +0000 >> @@ -71,15 +71,8 @@ def _parse_uname(uname): >> if uname.find(":") != -1: >> (typ, fn) = uname.split(":", 1) >> >> - if typ == "phy" and not fn.startswith("/"): >> + if typ in ("phy", "drbd") and not fn.startswith("/"): >> fn = "/dev/%s" %(fn,) >> - >> - if typ == "drbd": >> - if not fn.startswith("drbd"): >> - (drbdadmstdin, drbdadmstdout) >> os.popen2(["/sbin/drbdadm", "sh-dev", fn]) >> - fn = drbdadmstdout.readline().strip() >> - else: >> - fn = "/dev/%s" %(fn,) >> >> if typ in ("tap", "tap2"): >> (taptype, fn) = fn.split(":", 1 >> >> >> When you specify a drbd disk for a domU, its format is >> drbd:<resourceName> >> >> > > Correct. Sadly, I forgot I was testing on SLES, which contains local > xend patches to fix problems wrt external block scripts. One of the > patches has existed for ages, before many of us were working on xen > :-). I would really like to get these changes upstream, even though > xend is dying. The first patch is a revert of > http://xenbits.xensource.com/xen-unstable.hg?rev/152257350930. The > second patch, which I''ve rebased against -unstable, is attached. Can > you test it? >Shriram, Have you had a chance to test my suggestion above (reverting c/s 19444 and using the xend patch we have in SuSE)? It has been working well for us, but another "Tested-by:" may convince upstream to take these changes as well. Perhaps it is too late for 4.1 as we''re at RC2 already. Thanks! Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
darn!.. I missed your earlier email totally, in the flood of emails in my inbox :(. sorry about the delayed response. I dont have pygrub based vm disk images/volumes on my Debian devel boxes. But I have tested your patches (revert 20158, 19444 & applying your patch) with pv domUs, using the "block-drbd" script and the following work fine as expected. --Promoting drbd resource to primary before booting VM --Automatic secondary->primary resource promotion (at target host) during live migration shriram On Wed, Jan 12, 2011 at 7:38 PM, Jim Fehlig <jfehlig@novell.com> wrote:> Jim Fehlig wrote: >> Shriram Rajagopalan wrote: >> >>> I was just looking over the patch on blkif.py and I believe there is a bug. >>> --- a/tools/python/xen/util/blkif.py Fri Dec 10 18:08:19 2010 +0000 >>> +++ b/tools/python/xen/util/blkif.py Wed Jan 05 23:31:24 2011 +0000 >>> @@ -71,15 +71,8 @@ def _parse_uname(uname): >>> if uname.find(":") != -1: >>> (typ, fn) = uname.split(":", 1) >>> >>> - if typ == "phy" and not fn.startswith("/"): >>> + if typ in ("phy", "drbd") and not fn.startswith("/"): >>> fn = "/dev/%s" %(fn,) >>> - >>> - if typ == "drbd": >>> - if not fn.startswith("drbd"): >>> - (drbdadmstdin, drbdadmstdout) >>> os.popen2(["/sbin/drbdadm", "sh-dev", fn]) >>> - fn = drbdadmstdout.readline().strip() >>> - else: >>> - fn = "/dev/%s" %(fn,) >>> >>> if typ in ("tap", "tap2"): >>> (taptype, fn) = fn.split(":", 1 >>> >>> >>> When you specify a drbd disk for a domU, its format is >>> drbd:<resourceName> >>> >>> >> >> Correct. Sadly, I forgot I was testing on SLES, which contains local >> xend patches to fix problems wrt external block scripts. One of the >> patches has existed for ages, before many of us were working on xen >> :-). I would really like to get these changes upstream, even though >> xend is dying. The first patch is a revert of >> http://xenbits.xensource.com/xen-unstable.hg?rev/152257350930. The >> second patch, which I''ve rebased against -unstable, is attached. Can >> you test it? >> > > Shriram, > > Have you had a chance to test my suggestion above (reverting c/s 19444 > and using the xend patch we have in SuSE)? It has been working well for > us, but another "Tested-by:" may convince upstream to take these changes > as well. Perhaps it is too late for 4.1 as we''re at RC2 already. > > Thanks! > Jim > >-- perception is but an offspring of its own self _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jim Fehlig writes ("Re: [Xen-devel] [PATCH] xend: drbd
improvements"):> Have you had a chance to test my suggestion above (reverting c/s 19444
> and using the xend patch we have in SuSE)? It has been working well for
> us, but another "Tested-by:" may convince upstream to take these
changes
> as well. Perhaps it is too late for 4.1 as we''re at RC2 already.
I''d love to have another test report. I didn''t apply these
patches
because it wasn''t clear to me exactly what the proposed changes were
and, as you say, that they had been sufficiently tested.
I think this change will be fine for 4.1, especially if the patch
doesn''t change non-drbd codepaths.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson wrote:> Jim Fehlig writes ("Re: [Xen-devel] [PATCH] xend: drbd improvements"): > >> Have you had a chance to test my suggestion above (reverting c/s 19444 >> and using the xend patch we have in SuSE)? It has been working well for >> us, but another "Tested-by:" may convince upstream to take these changes >> as well. Perhaps it is too late for 4.1 as we''re at RC2 already. >> > > I''d love to have another test report.Shriram reported that he successfully tested these changes.> I didn''t apply these patches > because it wasn''t clear to me exactly what the proposed changes were > and, as you say, that they had been sufficiently tested. >The revert of c/s 19444 simply removes more ''drbd'' cruft from xend internals. As for my xend patch, is the commit message that vague? ------------------------- xend: improve psudeo-bootloader support for external block scripts Userspace tools support external block scripts (e.g. block-drbd provided by drbd project). The psuedo-bootloader setup code in xend has a few limitations wrt external block scripts, which this patch addresses. blkif.py: parse_uname() utility function should be able to parse a disk specifier understood by the rest of the tools. XendDomainInfo.py: Block devices using external block scripts must be attached to dom0 before running the psuedo-bootloader. ------------------------- I can improve it if need be. Regards, Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jim Fehlig writes ("Re: [Xen-devel] [PATCH] xend: drbd
improvements"):> Correct. Sadly, I forgot I was testing on SLES, which contains local
> xend patches to fix problems wrt external block scripts. One of the
> patches has existed for ages, before many of us were working on xen
> :-). I would really like to get these changes upstream, even though
> xend is dying. The first patch is a revert of
> http://xenbits.xensource.com/xen-unstable.hg?rev/152257350930. The
> second patch, which I''ve rebased against -unstable, is attached.
Can
> you test it?
Thanks, Jim, for this patch, which I have now applied along with the
revert of 19444:152257350930.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel