Xen devel - Jun 2013 - [PATCH] [RFC] Fix RegEx Issues with xendomains for both SXP and JSON outputs of lx list -l

(Resent, as Yahoo seems to silently drop sentemail with the subject starting
with a square open bracket. Space inserted - apologies if received more than
once)

Modifications to xendomains so that it correctly identifies a new domain
entry when parsing the output of xl list -l. For both SXP and JSON
outputs, it was failing to spot a new domain entry and was saving the
second domain under a filename generated from the first domain''s name.

I have listed this as RFC because although it is a patch against 4.3RC5,
I have not had a chance to test against RC5, only 4.2.2. I won''t get an
opportunity until the weekend, so I invite comment and anybody to give
it a test against RC5. Ian C has already commented elsewhere that the xl
list -l JSON output has been altered between 4.2.x and 4.3RC5, so this
might affect the indentation (the 4 spaces in the middle of the
LIST_GREP variable) - the trigger for new domain data is ''   
{'' in JSON
version (in at least 4.2.2) and ''(domain'' in SXP version. Not
tested
against xm.

It seems there is a problem in 4.2.2''s implementation of SXP output of
xl list -l in that all domain ID''s are outputted as -1. Ian C suggested
applying a change that originally went into 4.3 to resolve this issue.
This seemed to do the trick (see thread [Xen-users] DomU
suspension/hibernation )

Thanks to David Sutton for tracking through the issues with me and Ian C
for guidance and assistance.


Signed-off-by: Ian Murray <murrayie@yahoo.co.uk>


diff --git a/tools/hotplug/Linux/init.d/xendomains
b/tools/hotplug/Linux/init.d/xendomains
index 730541e..38371af 100644
--- a/tools/hotplug/Linux/init.d/xendomains
+++ b/tools/hotplug/Linux/init.d/xendomains
@@ -206,7 +206,7 @@ rdnames()
      done
  }

-LIST_GREP=''((domain\|(domid\|(name\|^{$\|"name":\|"domid":''
+LIST_GREP=''(domain\|(domid\|(name\|^   
{$\|"name":\|"domid":''
  parseln()
  {
      if [[ "$1" =~ ''(domain'' ]] || [[
"$1" = "{" ]]; then
@@ -237,7 +237,7 @@ is_running()
                RC=0
                ;;
        esac
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | grep "$LIST_GREP")
      return $RC
  }

@@ -319,7 +319,7 @@ all_zombies()
        if test "$state" != "-b---d" -a "$state"
!= "-----d"; then
            return 1;
        fi
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | grep "$LIST_GREP")
      return 0
  }

@@ -450,7 +450,7 @@ stop()
            fi
            kill $WDOG_PID >/dev/null 2>&1
        fi
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | grep "$LIST_GREP")

      # NB. this shuts down ALL Xen domains (politely), not just the ones in
      # AUTODIR/*
@@ -487,7 +487,7 @@ check_domain_up()
                return 0
                ;;
        esac
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | grep "$LIST_GREP")
      return 1
  }

(Resent several times, as Yahoo seems to be silently dropping some emails with
square brackets in the subject for rme - apologies if received more than once)

Modifications to xendomains so that it correctly identifies a new domain
entry when parsing the output of xl list -l. For both SXP and JSON
outputs, it was failing to spot a new domain entry and was saving the
second domain under a filename generated from the first domain''s name.

I have listed this as RFC because although it is a patch against 4.3RC5,
I have not had a chance to test against RC5, only 4.2.2. I won''t get an
opportunity until the weekend, so I invite comment and anybody to give
it a test against RC5. Ian C has already commented elsewhere that the xl
list -l JSON output has been altered between 4.2.x and 4.3RC5, so this
might affect the indentation (the 4 spaces in the middle of the
LIST_GREP variable) - the trigger for new domain data is ''   
{'' in JSON
version (in at least 4.2.2) and ''(domain'' in SXP version. Not
tested
against xm.

It seems there is a problem in 4.2.2''s implementation of SXP output of
xl list -l in that all domain ID''s are outputted as -1. Ian C suggested
applying a change that originally went into 4.3 to resolve this issue.
This seemed to do the trick (see thread [Xen-users] DomU
suspension/hibernation )

Thanks to David Sutton for tracking through the issues with me and Ian C
for guidance and assistance.


Signed-off-by: Ian Murray <murrayie@yahoo.co.uk>


diff --git a/tools/hotplug/Linux/init.d/xendomains
b/tools/hotplug/Linux/init.d/xendomains
index 730541e..38371af 100644
--- a/tools/hotplug/Linux/init.d/xendomains
+++ b/tools/hotplug/Linux/init.d/xendomains
@@ -206,7 +206,7 @@ rdnames()
      done
  }

-LIST_GREP=''((domain\|(domid\|(name\|^{$\|"name":\|"domid":''
+LIST_GREP=''(domain\|(domid\|(name\|^   
{$\|"name":\|"domid":''
  parseln()
  {
      if [[ "$1" =~ ''(domain'' ]] || [[
"$1" = "{" ]]; then
@@ -237,7 +237,7 @@ is_running()
                RC=0
                ;;
        esac
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | grep "$LIST_GREP")
      return $RC
  }

@@ -319,7 +319,7 @@ all_zombies()
        if test "$state" != "-b---d" -a "$state"
!= "-----d"; then
            return 1;
        fi
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | grep "$LIST_GREP")
      return 0
  }

@@ -450,7 +450,7 @@ stop()
            fi
            kill $WDOG_PID >/dev/null 2>&1
        fi
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | grep "$LIST_GREP")

      # NB. this shuts down ALL Xen domains (politely), not just the ones in
      # AUTODIR/*
@@ -487,7 +487,7 @@ check_domain_up()
                return 0
                ;;
        esac
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | grep "$LIST_GREP")
      return 1
  }

On Thu, 2013-06-20 at 13:00 +0100, Ian Murray wrote:
> (Resent, as Yahoo seems to silently drop sentemail with the subject
> starting with a square open bracket. Space inserted - apologies if
> received more than once)
How very unhelpful of them! FWIW This seems to be the first one which
made it into the list archives as well as my INBOX.

I''m working through my post-vacation backlog this morning but I hope to
have a chance to look at this vs. 4.3-rc soon since I think it is a
worthwhile bugfix for 4.3. George, what does your release manager''s hat
think?
> Modifications to xendomains so that it correctly identifies a new domain
> entry when parsing the output of xl list -l. For both SXP and JSON
> outputs, it was failing to spot a new domain entry and was saving the
> second domain under a filename generated from the first domain''s
name.
> 
> I have listed this as RFC because although it is a patch against 4.3RC5,
> I have not had a chance to test against RC5, only 4.2.2. I won''t
get an
> opportunity until the weekend, so I invite comment and anybody to give
> it a test against RC5. Ian C has already commented elsewhere that the xl
> list -l JSON output has been altered between 4.2.x and 4.3RC5, so this
> might affect the indentation (the 4 spaces in the middle of the
> LIST_GREP variable) - the trigger for new domain data is ''   
{'' in JSON
> version (in at least 4.2.2) and ''(domain'' in SXP version.
Not tested
> against xm.
> 
> It seems there is a problem in 4.2.2''s implementation of SXP
output of
> xl list -l in that all domain ID''s are outputted as -1. Ian C
suggested
> applying a change that originally went into 4.3 to resolve this issue.
> This seemed to do the trick (see thread [Xen-users] DomU
> suspension/hibernation )
Would you be able to submit that backport as a patch against the 4.2
staging branch and CC myself and Ian Jackson
<ian.jackson@eu.citrix.com> ?

This isn''t a straight forward backport, since the majority of the
commit
is not a suitable candidate, but if you reference in the commit message
the original commit id/subject and point out that only the one hunk is a
useful bug fix which is why it''s been split out then I think it would
be
fine to have in the next 4.2 release.

Thanks,
Ian.
> 
> Thanks to David Sutton for tracking through the issues with me and Ian C
> for guidance and assistance.
> 
> 
> Signed-off-by: Ian Murray <murrayie@yahoo.co.uk>
> 
> 
> diff --git a/tools/hotplug/Linux/init.d/xendomains
> b/tools/hotplug/Linux/init.d/xendomains
> index 730541e..38371af 100644
> --- a/tools/hotplug/Linux/init.d/xendomains
> +++ b/tools/hotplug/Linux/init.d/xendomains
> @@ -206,7 +206,7 @@ rdnames()
>       done
>   }
> 
>
-LIST_GREP=''((domain\|(domid\|(name\|^{$\|"name":\|"domid":''
> +LIST_GREP=''(domain\|(domid\|(name\|^   
{$\|"name":\|"domid":''
>   parseln()
>   {
>       if [[ "$1" =~ ''(domain'' ]] || [[
"$1" = "{" ]]; then
> @@ -237,7 +237,7 @@ is_running()
>                 RC=0
>                 ;;
>         esac
> -    done < <($CMD list -l | grep $LIST_GREP)
> +    done < <($CMD list -l | grep "$LIST_GREP")
>       return $RC
>   }
> 
> @@ -319,7 +319,7 @@ all_zombies()
>         if test "$state" != "-b---d" -a
"$state" != "-----d"; then
>             return 1;
>         fi
> -    done < <($CMD list -l | grep $LIST_GREP)
> +    done < <($CMD list -l | grep "$LIST_GREP")
>       return 0
>   }
> 
> @@ -450,7 +450,7 @@ stop()
>             fi
>             kill $WDOG_PID >/dev/null 2>&1
>         fi
> -    done < <($CMD list -l | grep $LIST_GREP)
> +    done < <($CMD list -l | grep "$LIST_GREP")
> 
>       # NB. this shuts down ALL Xen domains (politely), not just the ones
in
>       # AUTODIR/*
> @@ -487,7 +487,7 @@ check_domain_up()
>                 return 0
>                 ;;
>         esac
> -    done < <($CMD list -l | grep $LIST_GREP)
> +    done < <($CMD list -l | grep "$LIST_GREP")
>       return 1
>   }

On 06/25/2013 10:40 AM, Ian Campbell wrote:
> On Thu, 2013-06-20 at 13:00 +0100, Ian Murray wrote:
>> (Resent, as Yahoo seems to silently drop sentemail with the subject
>> starting with a square open bracket. Space inserted - apologies if
>> received more than once)
>
> How very unhelpful of them! FWIW This seems to be the first one which
> made it into the list archives as well as my INBOX.
>
> I''m working through my post-vacation backlog this morning but I
hope to
> have a chance to look at this vs. 4.3-rc soon since I think it is a
> worthwhile bugfix for 4.3. George, what does your release
manager''s hat
> think?
Well it sounded from the description like xendomains for xl is just 
broken, right?  You can''t really make it any more broken, and this
patch
doesn''t cover any other codepaths, so there''s basically zero
risk of
impact on other, working functionality.

So yes, I think it''s good for 4.3:

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

----- Original Message -----
> From: Ian Campbell <Ian.Campbell@citrix.com>
> To: Ian Murray <murrayie@yahoo.co.uk>
> Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>;
David Sutton <kantras@gmail.com>; Joshua Tuttle <jtuttle@i-a-i.com>;
George Dunlap <george.dunlap@eu.citrix.com>; Ian Jackson
<Ian.Jackson@eu.citrix.com>
> Sent: Tuesday, 25 June 2013, 10:40
> Subject: Re: [PATCH] [RFC] Fix RegEx Issues with xendomains for both SXP
and
 JSON outputs of lx list -l
> 
> On Thu, 2013-06-20 at 13:00 +0100, Ian Murray wrote:
>>  (Resent, as Yahoo seems to silently drop sentemail with the subject
>>  starting with a square open bracket. Space inserted - apologies if
>>  received more than once)
> 
> How very unhelpful of them! FWIW This seems to be the first one which
> made it into the list archives as well as my INBOX.
> 
> I''m working through my post-vacation backlog this morning but I
hope to
> have a chance to look at this vs. 4.3-rc soon since I think it is a
> worthwhile bugfix for 4.3. George, what does your release
manager''s hat
> think?
There are white space issues with this patch on this thread as it is, so I
re-issued it more recently directly from git, as is the preferred method I
think.

As was suggested by David Sutton, there are probably more issues with this
script than has been covered so far, but this change makes suspension and
restoration work for me, at least. He suggests there are problems with zombies,
etc.

> 
>>  Modifications to xendomains so that it correctly identifies a new
domain
>>  entry when parsing the output of xl list -l. For both SXP and JSON
>>  outputs, it was failing to spot a new domain entry and was saving the
>>  second domain under a filename generated from the first
domain''s name.
>> 
>>  I have listed this as RFC because although it is a patch against
4.3RC5,
>>  I have not had a chance to test against RC5, only 4.2.2. I
won''t get an
>>  opportunity until the weekend, so I invite comment and anybody to give
>>  it a test against RC5. Ian C has already commented elsewhere that the
xl
>>  list -l JSON output has been altered between 4.2.x and 4.3RC5, so this
>>  might affect the indentation (the 4 spaces in the middle of the
>>  LIST_GREP variable) - the trigger for new domain data is ''   
{'' in
> JSON
>>  version (in at least 4.2.2) and ''(domain'' in SXP
version. Not
> tested
>>  against xm.
>> 
>>  It seems there is a problem in 4.2.2''s implementation of SXP
output of
>>  xl list -l in that all domain ID''s are outputted as -1. Ian C
suggested
>>  applying a change that originally went into 4.3 to resolve this issue.
>>  This seemed to do the trick (see thread [Xen-users] DomU
>>  suspension/hibernation )
> 
> Would you be able to submit that backport as a patch against the 4.2
> staging branch and CC myself and Ian Jackson
> <ian.jackson@eu.citrix.com> ?
> 
> This isn''t a straight forward backport, since the majority of the
commit
> is not a suitable candidate, but if you reference in the commit message
> the original commit id/subject and point out that only the one hunk is a
> useful bug fix which is why it''s been split out then I think it
would be
> fine to have in the next 4.2 release.
I''ll give this a go, but it will be a learning experience. I
don''t recall seeing any backporting info in the patch submission notes
but I will take another look. Who signs a (partial) backport off? Can''t
be me, because I didn''t write it. Or can it? or is it copied from the
original submission?
> 
> Thanks,
> Ian.
> 
>> 
>>  Thanks to David Sutton for tracking through the issues with me and Ian
C
>>  for guidance and assistance.
>> 
>> 
>>  Signed-off-by: Ian Murray <murrayie@yahoo.co.uk>
>> 
>> 
>>  diff --git a/tools/hotplug/Linux/init.d/xendomains
>>  b/tools/hotplug/Linux/init.d/xendomains
>>  index 730541e..38371af 100644
>>  --- a/tools/hotplug/Linux/init.d/xendomains
>>  +++ b/tools/hotplug/Linux/init.d/xendomains
>>  @@ -206,7 +206,7 @@ rdnames()
>>        done
>>    }
>> 
>> 
>
-LIST_GREP=''((domain\|(domid\|(name\|^{$\|"name":\|"domid":''
>>  +LIST_GREP=''(domain\|(domid\|(name\|^    
> {$\|"name":\|"domid":''
>>    parseln()
>>    {
>>        if [[ "$1" =~ ''(domain'' ]] || [[
"$1" =
> "{" ]]; then
>>  @@ -237,7 +237,7 @@ is_running()
>>                  RC=0
>>                  ;;
>>          esac
>>  -    done < <($CMD list -l | grep $LIST_GREP)
>>  +    done < <($CMD list -l | grep "$LIST_GREP")
>>        return $RC
>>    }
>> 
>>  @@ -319,7 +319,7 @@ all_zombies()
>>          if test "$state" != "-b---d" -a 
> "$state" != "-----d"; then
>>              return 1;
>>          fi
>>  -    done < <($CMD list -l | grep $LIST_GREP)
>>  +    done < <($CMD list -l | grep "$LIST_GREP")
>>        return 0
>>    }
>> 
>>  @@ -450,7 +450,7 @@ stop()
>>              fi
>>              kill $WDOG_PID >/dev/null 2>&1
>>          fi
>>  -    done < <($CMD list -l | grep $LIST_GREP)
>>  +    done < <($CMD list -l | grep "$LIST_GREP")
>> 
>>        # NB. this shuts down ALL Xen domains (politely), not just the
ones
> in
>>        # AUTODIR/*
>>  @@ -487,7 +487,7 @@ check_domain_up()
>>                  return 0
>>                  ;;
>>          esac
>>  -    done < <($CMD list -l | grep $LIST_GREP)
>>  +    done < <($CMD list -l | grep "$LIST_GREP")
>>        return 1
>>    }
>

On Tue, 2013-06-25 at 11:56 +0100, Ian Murray wrote:
> 
> 
> 
> ----- Original Message -----
> > From: Ian Campbell <Ian.Campbell@citrix.com>
> > To: Ian Murray <murrayie@yahoo.co.uk>
> > Cc: "xen-devel@lists.xen.org"
<xen-devel@lists.xen.org>; David Sutton <kantras@gmail.com>; Joshua
Tuttle <jtuttle@i-a-i.com>; George Dunlap
<george.dunlap@eu.citrix.com>; Ian Jackson
<Ian.Jackson@eu.citrix.com>
> > Sent: Tuesday, 25 June 2013, 10:40
> > Subject: Re: [PATCH] [RFC] Fix RegEx Issues with xendomains for both
SXP and
>  JSON outputs of lx list -l
> > 
> > On Thu, 2013-06-20 at 13:00 +0100, Ian Murray wrote:
> >>  (Resent, as Yahoo seems to silently drop sentemail with the
subject
> >>  starting with a square open bracket. Space inserted - apologies
if
> >>  received more than once)
> > 
> > How very unhelpful of them! FWIW This seems to be the first one which
> > made it into the list archives as well as my INBOX.
> > 
> > I''m working through my post-vacation backlog this morning but
I hope to
> > have a chance to look at this vs. 4.3-rc soon since I think it is a
> > worthwhile bugfix for 4.3. George, what does your release
manager''s hat
> > think?
> 
> There are white space issues with this patch on this thread as it is,
> so I re-issued it more recently directly from git, as is the preferred
> method I think.
It''s certainly less error prone. Do you have the message-id handy so I
can find the right version?
> As was suggested by David Sutton, there are probably more issues with
> this script than has been covered so far, but this change makes
> suspension and restoration work for me, at least. He suggests there
> are problems with zombies, etc.
You mean the handling of zombies by this script, rather than it somehow
causing zombies?

Zombie domains are already themselves a bug in and of themselves so if
that can be reproduced we''d like to hear about it, as for the
script''s
handling of those zombies if/when they occur, I think it''s probably
something to deal with in 4.4.
> >>  Modifications to xendomains so that it correctly identifies a new
domain
> >>  entry when parsing the output of xl list -l. For both SXP and
JSON
> >>  outputs, it was failing to spot a new domain entry and was saving
the
> >>  second domain under a filename generated from the first
domain''s name.
> >> 
> >>  I have listed this as RFC because although it is a patch against
4.3RC5,
> >>  I have not had a chance to test against RC5, only 4.2.2. I
won''t get an
> >>  opportunity until the weekend, so I invite comment and anybody to
give
> >>  it a test against RC5. Ian C has already commented elsewhere that
the xl
> >>  list -l JSON output has been altered between 4.2.x and 4.3RC5, so
this
> >>  might affect the indentation (the 4 spaces in the middle of the
> >>  LIST_GREP variable) - the trigger for new domain data is
''    {'' in
> > JSON
> >>  version (in at least 4.2.2) and ''(domain'' in
SXP version. Not
> > tested
> >>  against xm.
> >> 
> >>  It seems there is a problem in 4.2.2''s implementation of
SXP output of
> >>  xl list -l in that all domain ID''s are outputted as -1.
Ian C suggested
> >>  applying a change that originally went into 4.3 to resolve this
issue.
> >>  This seemed to do the trick (see thread [Xen-users] DomU
> >>  suspension/hibernation )
> > 
> > Would you be able to submit that backport as a patch against the 4.2
> > staging branch and CC myself and Ian Jackson
> > <ian.jackson@eu.citrix.com> ?
> > 
> > This isn''t a straight forward backport, since the majority of
the commit
> > is not a suitable candidate, but if you reference in the commit
message
> > the original commit id/subject and point out that only the one hunk is
a
> > useful bug fix which is why it''s been split out then I think
it would be
> > fine to have in the next 4.2 release.
> 
> I''ll give this a go, but it will be a learning experience. I
don''t
> recall seeing any backporting info in the patch submission notes but I
> will take another look.
Thanks. It''s not that common to have to do things this way (mostly we
take the entire patch) so there isn''t much in the way of docs. If you
just treat it for the most part like a regular submission and explain
what is going on in the commit message with references to the original
commit ID and title then you won''t go far wrong.
>  Who signs a (partial) backport off? Can''t be me, because I
didn''t
> write it. Or can it? or is it copied from the original submission?
I think it would be appropriate to retain the original signed-off-by(s)
and acks etc and append your own. Some people like to do:

  Commit message

  Signed-off-by: Original Author <original.author@example.com>
  Acked-by: 

  Backported to X.Y

  Signed-off-by: Back Porter <bporter@example.com>

Or something like that, which is fine IMHO. Or if your commit message
makes it clear enough what is going on then just tacking your S-o-b onto
the original blocks of S-o-b''s would be fine.

Ian J, does that sound about right? The issue is that one hunk of
a73a7a0c647a "xl: Remove global domid and enable -Wshadow" is actually
a
useful fix but the commit as a whole is large and unsuitable for
backporting IMHO.

Ian.

----- Original Message -----
> From: Ian Campbell <Ian.Campbell@citrix.com>
> To: Ian Murray <murrayie@yahoo.co.uk>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>; Joshua Tuttle
<jtuttle@i-a-i.com>; Ian Jackson <Ian.Jackson@eu.citrix.com>; David
Sutton <kantras@gmail.com>; "xen-devel@lists.xen.org"
<xen-devel@lists.xen.org>
> Sent: Tuesday, 25 June 2013, 14:44
> Subject: Re: [Xen-devel] [PATCH] [RFC] Fix RegEx Issues with xendomains for
 both SXP and JSON outputs of lx list -l
> 
> On Tue, 2013-06-25 at 11:56 +0100, Ian Murray wrote:
>> 
>> 
>> 
>>  ----- Original Message -----
>>  > From: Ian Campbell <Ian.Campbell@citrix.com>
>>  > To: Ian Murray <murrayie@yahoo.co.uk>
>>  > Cc: "xen-devel@lists.xen.org" 
> <xen-devel@lists.xen.org>; David Sutton <kantras@gmail.com>;
Joshua
> Tuttle <jtuttle@i-a-i.com>; George Dunlap 
> <george.dunlap@eu.citrix.com>; Ian Jackson 
> <Ian.Jackson@eu.citrix.com>
>>  > Sent: Tuesday, 25 June 2013, 10:40
>>  > Subject: Re: [PATCH] [RFC] Fix RegEx Issues with xendomains for
both
> SXP and
>>   JSON outputs of lx list -l
>>  > 
>>  > On Thu, 2013-06-20 at 13:00 +0100, Ian Murray wrote:
>>  >>  (Resent, as Yahoo seems to silently drop sentemail with the 
> subject
>>  >>  starting with a square open bracket. Space inserted -
apologies
> if
>>  >>  received more than once)
>>  > 
>>  > How very unhelpful of them! FWIW This seems to be the first one
which
>>  > made it into the list archives as well as my INBOX.
>>  > 
>>  > I''m working through my post-vacation backlog this
morning but I
> hope to
>>  > have a chance to look at this vs. 4.3-rc soon since I think it is
a
>>  > worthwhile bugfix for 4.3. George, what does your release 
> manager''s hat
>>  > think?
>> 
>>  There are white space issues with this patch on this thread as it is,
>>  so I re-issued it more recently directly from git, as is the preferred
>>  method I think.
> 
> It''s certainly less error prone. Do you have the message-id handy
so I
> can find the right version?
> 
Date: Sat, 22 Jun 2013 13:38:11 +0100
Message-Id: <1371904691-9842-1-git-send-email-murrayie@yahoo.co.uk>

>>  As was suggested by David Sutton, there are probably more issues with
>>  this script than has been covered so far, but this change makes
>>  suspension and restoration work for me, at least. He suggests there
>>  are problems with zombies, etc.
> 
> You mean the handling of zombies by this script, rather than it somehow
> causing zombies?
Yes, handling of zombies.
> 
> Zombie domains are already themselves a bug in and of themselves so if
> that can be reproduced we''d like to hear about it, as for the
script''s
> handling of those zombies if/when they occur, I think it''s
probably
> something to deal with in 4.4.
This is something that David Sutton made comment on the original
"users" thread. Without looking back, I seem to recall he''d
spotted a bug in the xendomains script''s handling rather than
experiencing it. I have no experience of the issue. The reason for mentioning it
was I just wanted to comment that my patch and testing wasn''t a
designed to fix all issues with xendomains, just the ones I had experience of.
> 
>>  >>  Modifications to xendomains so that it correctly identifies
a new
> domain
>>  >>  entry when parsing the output of xl list -l. For both SXP
and
> JSON
>>  >>  outputs, it was failing to spot a new domain entry and was
saving
> the
>>  >>  second domain under a filename generated from the first 
> domain''s name.
>>  >> 
>>  >>  I have listed this as RFC because although it is a patch
against
> 4.3RC5,
>>  >>  I have not had a chance to test against RC5, only 4.2.2. I 
> won''t get an
>>  >>  opportunity until the weekend, so I invite comment and
anybody to
> give
>>  >>  it a test against RC5. Ian C has already commented elsewhere
that
> the xl
>>  >>  list -l JSON output has been altered between 4.2.x and
4.3RC5, so
> this
>>  >>  might affect the indentation (the 4 spaces in the middle of
the
>>  >>  LIST_GREP variable) - the trigger for new domain data is
''   
> {'' in 
>>  > JSON
>>  >>  version (in at least 4.2.2) and ''(domain''
in SXP version.
> Not 
>>  > tested
>>  >>  against xm.
>>  >> 
>>  >>  It seems there is a problem in 4.2.2''s
implementation of SXP
> output of
>>  >>  xl list -l in that all domain ID''s are outputted as
-1. Ian C
> suggested
>>  >>  applying a change that originally went into 4.3 to resolve
this
> issue.
>>  >>  This seemed to do the trick (see thread [Xen-users] DomU
>>  >>  suspension/hibernation )
>>  > 
>>  > Would you be able to submit that backport as a patch against the
4.2
>>  > staging branch and CC myself and Ian Jackson
>>  > <ian.jackson@eu.citrix.com> ?
>>  > 
>>  > This isn''t a straight forward backport, since the
majority of the
> commit
>>  > is not a suitable candidate, but if you reference in the commit 
> message
>>  > the original commit id/subject and point out that only the one
hunk is
> a
>>  > useful bug fix which is why it''s been split out then I
think it
> would be
>>  > fine to have in the next 4.2 release.
>> 
>>  I''ll give this a go, but it will be a learning experience. I
don''t
>>  recall seeing any backporting info in the patch submission notes but I
>>  will take another look.
> 
> Thanks. It''s not that common to have to do things this way (mostly
we
> take the entire patch) so there isn''t much in the way of docs. If
you
> just treat it for the most part like a regular submission and explain
> what is going on in the commit message with references to the original
> commit ID and title then you won''t go far wrong.
> 
>>   Who signs a (partial) backport off? Can''t be me, because I
didn''t
>>  write it. Or can it? or is it copied from the original submission?
> 
> I think it would be appropriate to retain the original signed-off-by(s)
> and acks etc and append your own. Some people like to do:
> 
>   Commit message
> 
>   Signed-off-by: Original Author <original.author@example.com>
>   Acked-by: 
> 
>   Backported to X.Y
> 
>   Signed-off-by: Back Porter <bporter@example.com>
> 
> Or something like that, which is fine IMHO. Or if your commit message
> makes it clear enough what is going on then just tacking your S-o-b onto
> the original blocks of S-o-b''s would be fine.
> 
> Ian J, does that sound about right? The issue is that one hunk of
> a73a7a0c647a "xl: Remove global domid and enable -Wshadow" is
actually
> a
> useful fix but the commit as a whole is large and unsuitable for
> backporting IMHO.
> 
Okay, will look into it.

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

Reply below,

On Tue, Jun 25, 2013 at 9:55 AM, Ian Murray <murrayie@yahoo.co.uk> wrote:
>
> Date: Sat, 22 Jun 2013 13:38:11 +0100
> Message-Id: <1371904691-9842-1-git-send-email-murrayie@yahoo.co.uk>
>
>
> >>  As was suggested by David Sutton, there are probably more issues
with
> >>  this script than has been covered so far, but this change makes
> >>  suspension and restoration work for me, at least. He suggests
there
> >>  are problems with zombies, etc.
> >
> > You mean the handling of zombies by this script, rather than it
somehow
> > causing zombies?
>
> Yes, handling of zombies.
>
> Yes, it was handling of zombies - in particular the xendomains script was
making use of the state information of the various domains, which isn''t
part of the information exported  by xl list -l in either format


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

>________________________________
> From: David Sutton <kantras@gmail.com>
>To: Ian Murray <murrayie@yahoo.co.uk> 
>Cc: Ian Campbell <Ian.Campbell@citrix.com>; George Dunlap
<george.dunlap@eu.citrix.com>; Joshua Tuttle <jtuttle@i-a-i.com>;
Ian Jackson <Ian.Jackson@eu.citrix.com>;
"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
>Sent: Tuesday, 25 June 2013, 16:14
>Subject: Re: [Xen-devel] [PATCH] [RFC] Fix RegEx Issues with xendomains for
both SXP and JSON outputs of lx list -l
> 
>
>
>Reply below,
>
>
>On Tue, Jun 25, 2013 at 9:55 AM, Ian Murray <murrayie@yahoo.co.uk>
wrote:
>
>
>>Date: Sat, 22 Jun 2013 13:38:11 +0100
>>Message-Id:
<1371904691-9842-1-git-send-email-murrayie@yahoo.co.uk>
>>
>>
>>
>>>>  As was suggested by David Sutton, there are probably more
issues with
>>>>  this script than has been covered so far, but this change
makes
>>>>  suspension and restoration work for me, at least. He suggests
there
>>>>  are problems with zombies, etc.
>>>
>>> You mean the handling of zombies by this script, rather than it
somehow
>>> causing zombies?
>>
>>Yes, handling of zombies.
>>
>>
>>
>Yes, it was handling of zombies - in particular the xendomains script was
making use of the state information of the various domains, which isn''t
part of the information exported  by xl list -l in either format
>
I am guessing, but I would say the script used to use xl/xm list without the -l
and thiis is where $state was set originally. As you point this isn''t
available from xl list -l (should it be?). If I was approaching this as a
re-write, the first thing I would do would be to devise some test cases for all
the things this script is supposed to deal with. I don''t know how to
create zombies at will, for example. Perhaps the hypervisor gurus do. Until that
can be done, this script is going to be hard to test properly. Just my opinion.





> 
>
>
>
>

On Tue, 2013-06-25 at 16:39 +0100, Ian Murray wrote:
> 
> 
> 
> 
> 
> 
> >________________________________
> > From: David Sutton <kantras@gmail.com>
> >To: Ian Murray <murrayie@yahoo.co.uk> 
> >Cc: Ian Campbell <Ian.Campbell@citrix.com>; George Dunlap
<george.dunlap@eu.citrix.com>; Joshua Tuttle <jtuttle@i-a-i.com>;
Ian Jackson <Ian.Jackson@eu.citrix.com>;
"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
> >Sent: Tuesday, 25 June 2013, 16:14
> >Subject: Re: [Xen-devel] [PATCH] [RFC] Fix RegEx Issues with xendomains
for both SXP and JSON outputs of lx list -l
> > 
> >
> >
> >Reply below,
> >
> >
> >On Tue, Jun 25, 2013 at 9:55 AM, Ian Murray
<murrayie@yahoo.co.uk> wrote:
> >
> >
> >>Date: Sat, 22 Jun 2013 13:38:11 +0100
> >>Message-Id:
<1371904691-9842-1-git-send-email-murrayie@yahoo.co.uk>
> >>
> >>
> >>
> >>>>  As was suggested by David Sutton, there are probably more
issues with
> >>>>  this script than has been covered so far, but this change
makes
> >>>>  suspension and restoration work for me, at least. He
suggests there
> >>>>  are problems with zombies, etc.
> >>>
> >>> You mean the handling of zombies by this script, rather than
it somehow
> >>> causing zombies?
> >>
> >>Yes, handling of zombies.
> >>
> >>
> >>
> >Yes, it was handling of zombies - in particular the xendomains script
> was making use of the state information of the various domains, which
> isn''t part of the information exported  by xl list -l in either
format
> >
> 
> I am guessing, but I would say the script used to use xl/xm list
> without the -l and thiis is where $state was set originally.
That sounds like a good bet.
>  As you point this isn''t available from xl list -l (should it
be?). If
> I was approaching this as a re-write, the first thing I would do would
> be to devise some test cases for all the things this script is
> supposed to deal with. I don''t know how to create zombies at will,
for
> example. Perhaps the hypervisor gurus do. Until that can be done, this
> script is going to be hard to test properly. Just my opinion.
If you can create zombies then that is a bug I think, which we would fix
and break your test case ;-)

If I were going to write test cases for this script I would start by
collecting a library of "xl list -l" outputs (both real world and
handwritten) and devise a test mode which sucked this in instead of
running the command and operated in a dry-run mode.

This is effectively what we''ve done, to a greater or lesser extent
(mostly lesser...), for xl disk stanza parsing
(tools/libxl/check-xl-disk-parse) or pygrub (tools/pygrub/examples/).
It''s nice because when someone reports a bug you can just add their
output to the set of things you test.

Ian.

----- Original Message -----
> From: Ian Campbell <Ian.Campbell@citrix.com>
> To: Ian Murray <murrayie@yahoo.co.uk>
> Cc: David Sutton <kantras@gmail.com>; George Dunlap
<george.dunlap@eu.citrix.com>; Joshua Tuttle <jtuttle@i-a-i.com>;
Ian Jackson <Ian.Jackson@eu.citrix.com>;
"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
> Sent: Wednesday, 26 June 2013, 11:50
> Subject: Re: [Xen-devel] [PATCH] [RFC] Fix RegEx Issues with xendomains for
 both SXP and JSON outputs of lx list -l
> 
> On Tue, 2013-06-25 at 16:39 +0100, Ian Murray wrote:
>> 
>> 
>> 
>> 
>> 
>> 
>>  >________________________________
>>  > From: David Sutton <kantras@gmail.com>
>>  >To: Ian Murray <murrayie@yahoo.co.uk> 
>>  >Cc: Ian Campbell <Ian.Campbell@citrix.com>; George Dunlap 
> <george.dunlap@eu.citrix.com>; Joshua Tuttle
<jtuttle@i-a-i.com>;
> Ian Jackson <Ian.Jackson@eu.citrix.com>; 
> "xen-devel@lists.xen.org" <xen-devel@lists.xen.org> 
>>  >Sent: Tuesday, 25 June 2013, 16:14
>>  >Subject: Re: [Xen-devel] [PATCH] [RFC] Fix RegEx Issues with
xendomains
> for both SXP and JSON outputs of lx list -l
>>  > 
>>  >
>>  >
>>  >Reply below,
>>  >
>>  >
>>  >On Tue, Jun 25, 2013 at 9:55 AM, Ian Murray 
> <murrayie@yahoo.co.uk> wrote:
>>  >
>>  >
>>  >>Date: Sat, 22 Jun 2013 13:38:11 +0100
>>  >>Message-Id: 
> <1371904691-9842-1-git-send-email-murrayie@yahoo.co.uk>
>>  >>
>>  >>
>>  >>
>>  >>>>  As was suggested by David Sutton, there are probably
more
> issues with
>>  >>>>  this script than has been covered so far, but this
change
> makes
>>  >>>>  suspension and restoration work for me, at least. He
> suggests there
>>  >>>>  are problems with zombies, etc.
>>  >>>
>>  >>> You mean the handling of zombies by this script, rather
than
> it somehow
>>  >>> causing zombies?
>>  >>
>>  >>Yes, handling of zombies.
>>  >>
>>  >>
>>  >>
>>  >Yes, it was handling of zombies - in particular the xendomains
script
>>  was making use of the state information of the various domains, which
>>  isn''t part of the information exported  by xl list -l in
either format
>>  >
>> 
>>  I am guessing, but I would say the script used to use xl/xm list
>>  without the -l and thiis is where $state was set originally.
> 
> That sounds like a good bet.
> 
>>   As you point this isn''t available from xl list -l (should it
be?). If
>>  I was approaching this as a re-write, the first thing I would do would
>>  be to devise some test cases for all the things this script is
>>  supposed to deal with. I don''t know how to create zombies at
will, for
>>  example. Perhaps the hypervisor gurus do. Until that can be done, this
>>  script is going to be hard to test properly. Just my opinion.
> 
> If you can create zombies then that is a bug I think, which we would fix
> and break your test case ;-)
I was thinking of...

xl zombify <domainid>     :)
> 
> If I were going to write test cases for this script I would start by
> collecting a library of "xl list -l" outputs (both real world and
> handwritten) and devise a test mode which sucked this in instead of
> running the command and operated in a dry-run mode.
> 
> This is effectively what we''ve done, to a greater or lesser extent
> (mostly lesser...), for xl disk stanza parsing
> (tools/libxl/check-xl-disk-parse) or pygrub (tools/pygrub/examples/).
> It''s nice because when someone reports a bug you can just add
their
> output to the set of things you test.
I think that is a good idea except it doesn''t capture an issue if the
output of the real xl list -l changes. Besides, I liked the idea of David S. to
get away from parsing xl list -l. Assuming this is what happened, I am not sure
of the design logic of changing the behaviour of xl list -l to work with JSON.
IMHO, I would say better to add another option for that output, so as not to
cause compatibility issues.

Anyway, one of the for future, I think.

> 
> Ian.
>