[PATCH][XEN]xm dump command add on

[PATCH][XEN]xm dump command add on

[Ken Hironaka]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

This adds the xm dump command to xend.
The command outputs the memory contents of the specified domU to a
coredump file.
This will fail if tried on dom0.
The interface is as follows:

xm dump [-L][-C] <domID> [output path]

-L Live dump:By default, xm dump does an xm pause, unpause before and
after taking the dump, respectively. This option disables the
pause/unpause and simply takes the dump.

-C crash dump: This executes an xm destroy after the dump file is
complete.
The output path is optional, and if it is not specified, the path will
be
/var/xen/dump/<domU name>.<domU ID>.core

This command uses the existant dumpCore(), which has been used for
coredump when a domU crashed. 
In this patch, the xc_domain_dumpcore_via_callback() in xc_core.c of
libxc is also modified. Previously, the
xc_domain_dumpcore_via_callback() did not respond to error when
copy_from_domain_page() failed. In other words, the dump core remained
silent even if mapping the domain memory failed and its page could not
be copied. When this happened, erroneous data had been dumped to the
file without the user realizing it. Now, it has been modified so that if
copy_from_domain_page fails, this fact is recorded in the logfile.
However even in such cases, the dumping will continue as before.

Signed-off-by: Ken Hironaka <hironaka.ken@soft.fujitsu.com>

Reference
http://lists.xensource.com/archives/html/xen-devel/2006-08/msg00181.html
http://lists.xensource.com/archives/html/xen-devel/2006-08/msg00259.html
http://lists.xensource.com/archives/html/xen-devel/2006-08/msg00483.html

[-- Attachment #2: xmdump_patch.diff --]
[-- Type: text/x-patch, Size: 5761 bytes --]

diff -r 0e32095a7b46 tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c	Wed Aug 09 21:34:27 2006 +0100
+++ b/tools/libxc/xc_core.c	Fri Aug 11 17:03:39 2006 +0900
@@ -36,7 +36,7 @@ xc_domain_dumpcore_via_callback(int xc_h
     vcpu_guest_context_t  ctxt[MAX_VIRT_CPUS];
     char dummy[PAGE_SIZE];
     int dummy_len;
-    int sts;
+    int sts,cpy_sts;
 
     if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
     {
@@ -101,9 +101,11 @@ xc_domain_dumpcore_via_callback(int xc_h
     if ( sts != 0 )
         goto error_out;
 
-    for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
+    for ( dump_mem = dump_mem_start, i = 0,cpy_sts=0; i < nr_pages; i++ )
     {
-        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+        if((sts=copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem))<0)
+            cpy_sts=sts;
+
         dump_mem += PAGE_SIZE;
         if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
         {
@@ -113,6 +115,8 @@ xc_domain_dumpcore_via_callback(int xc_h
             dump_mem = dump_mem_start;
         }
     }
+    if(cpy_sts!=0)
+        goto error_out;
 
     free(dump_mem_start);
     free(page_array);
diff -r 0e32095a7b46 tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py	Wed Aug 09 21:34:27 2006 +0100
+++ b/tools/python/xen/xend/XendDomain.py	Fri Aug 11 16:29:41 2006 +0900
@@ -390,6 +390,22 @@ class XendDomain:
         except Exception, ex:
             raise XendError(str(ex))
 
+    def domain_dump(self, domid, filename):
+        """Dump domain core."""
+
+        dominfo = self.domain_lookup_by_name_or_id_nr(domid)
+        if not dominfo:
+            raise XendInvalidDomain(str(domid))
+
+        if dominfo.getDomid() == PRIV_DOMAIN:
+            raise XendError("Cannot dump core for privileged domain %s" % domid)
+
+        try:
+            log.info("Dumping Core for Domain %s (%d).", dominfo.getName(),
+                     dominfo.getDomid())
+            return dominfo.dumpCore(filename)
+        except Exception, ex:
+            raise XendError(str(ex))
 
     def domain_destroy(self, domid):
         """Terminate domain immediately."""
@@ -402,9 +418,9 @@ class XendDomain:
             val = dominfo.destroy()
         else:
             try:
-                val = xc.domain_destroy(int(domid))
+                val = xc.domain_destroy(domid)
             except Exception, ex:
-                raise XendInvalidDomain(str(domid))
+                raise XendError(str(ex))
         return val       
 
     def domain_migrate(self, domid, dst, live=False, resource=0, port=0):
diff -r 0e32095a7b46 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Wed Aug 09 21:34:27 2006 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py	Fri Aug 11 15:13:51 2006 +0900
@@ -967,11 +967,12 @@ class XendDomainInfo:
         self.restart(True)
 
 
-    def dumpCore(self):
+    def dumpCore(self,corefile=None):
         """Create a core dump for this domain.  Nothrow guarantee."""
         
         try:
-            corefile = "/var/xen/dump/%s.%s.core" % (self.info['name'],
+            if not corefile:
+                corefile = "/var/xen/dump/%s.%s.core" % (self.info['name'],
                                                      self.domid)
             xc.domain_dumpcore(self.domid, corefile)
 
diff -r 0e32095a7b46 tools/python/xen/xm/main.py
--- a/tools/python/xen/xm/main.py	Wed Aug 09 21:34:27 2006 +0100
+++ b/tools/python/xen/xm/main.py	Fri Aug 11 16:37:02 2006 +0900
@@ -56,6 +56,8 @@ create_help =  """create [-c] <ConfigFil
 create_help =  """create [-c] <ConfigFile>
                [Name=Value]..       Create a domain based on Config File"""
 destroy_help = "destroy <DomId>                  Terminate a domain immediately"
+dump_help =   "dump [-LC] <DomId> [FileName]     Dump core of the specified domain"
+
 help_help =    "help                             Display this message"
 list_help =    "list [--long] [DomId, ...]       List information about domains"
 list_label_help = "list [--label] [DomId, ...]      List information about domains including their labels"
@@ -138,6 +140,7 @@ short_command_list = [
     "console",
     "create",
     "destroy",
+    "dump",
     "help",
     "list",
     "mem-set",
@@ -158,6 +161,7 @@ domain_commands = [
     "destroy",
     "domid",
     "domname",
+    "dump",
     "list",
     "list_label",
     "mem-max",
@@ -588,6 +592,43 @@ def xm_unpause(args):
 
     server.xend.domain.unpause(dom)
 
+def xm_dump(args):
+    arg_check(args, "dump",1,3)
+    live=False
+    crash=False
+    import getopt
+    optlist,args=getopt.gnu_getopt(args,"LC")
+    for opt in optlist:
+        if opt[0]=="-L":
+            live=True
+        if opt[0]=="-C":
+            crash=True
+
+    if len(args)==0 or len(args)>2:
+        err("invalid number of parameters")
+        usage("dump")
+
+    dom=args[0]
+    if len(args)==2:
+        filename=args[1]
+    else:
+        filename=None
+
+    if not live:
+        print "pausing domain:%s ..." % str(dom)
+        server.xend.domain.pause(dom)
+
+    print "dumping core of domain:%s ..." % str(dom)
+    server.xend.domain.dump(dom,filename)
+    
+    if not live:
+        print "unpausing domain:%s ..." % str(dom)
+        server.xend.domain.unpause(dom)
+
+    if crash:
+        print "destroying domain:%s ..." % str(dom)
+        server.xend.domain.destroy(dom)
+
 def xm_rename(args):
     arg_check(args, "rename", 2)
 
@@ -1112,6 +1153,7 @@ commands = {
     "destroy": xm_destroy,
     "domid": xm_domid,
     "domname": xm_domname,
+    "dump": xm_dump,
     "rename": xm_rename,
     "restore": xm_restore,
     "save": xm_save,

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH][XEN]xm dump command add on

[Jimi Xenidis]

can we call this "xm core" or "xm crash"?
"dump" has so many meanings, I had plans to use it for "register" and  
"memory" dump, tho I think even those should have better names.
thoughts?
-JX
On Aug 14, 2006, at 4:03 AM, Ken Hironaka wrote:

> This adds the xm dump command to xend.
> The command outputs the memory contents of the specified domU to a
> coredump file.
> This will fail if tried on dom0.
> The interface is as follows:
>
> xm dump [-L][-C] <domID> [output path]
>
> -L Live dump:By default, xm dump does an xm pause, unpause before and
> after taking the dump, respectively. This option disables the
> pause/unpause and simply takes the dump.
>
> -C crash dump: This executes an xm destroy after the dump file is
> complete.
> The output path is optional, and if it is not specified, the path will
> be
> /var/xen/dump/<domU name>.<domU ID>.core
>
> This command uses the existant dumpCore(), which has been used for
> coredump when a domU crashed.
> In this patch, the xc_domain_dumpcore_via_callback() in xc_core.c of
> libxc is also modified. Previously, the
> xc_domain_dumpcore_via_callback() did not respond to error when
> copy_from_domain_page() failed. In other words, the dump core remained
> silent even if mapping the domain memory failed and its page could not
> be copied. When this happened, erroneous data had been dumped to the
> file without the user realizing it. Now, it has been modified so  
> that if
> copy_from_domain_page fails, this fact is recorded in the logfile.
> However even in such cases, the dumping will continue as before.
>
> Signed-off-by: Ken Hironaka <hironaka.ken@soft.fujitsu.com>
>
> Reference
> http://lists.xensource.com/archives/html/xen-devel/2006-08/ 
> msg00181.html
> http://lists.xensource.com/archives/html/xen-devel/2006-08/ 
> msg00259.html
> http://lists.xensource.com/archives/html/xen-devel/2006-08/ 
> msg00483.html
> <xmdump_patch.diff>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH][XEN]xm dump command add on

[Keir Fraser]

On 14/8/06 1:28 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:

> can we call this "xm core" or "xm crash"?
> "dump" has so many meanings, I had plans to use it for "register" and
> "memory" dump, tho I think even those should have better names.
> thoughts?

Xm dump-core would be sensible.

 -- Keir

Re: [PATCH][XEN]xm dump command add on

[Ken Hironaka]

thank you on your input.

I understand how this can be confusing. I will change the name to 
xm dump-core
and resubmit the patch.

Ken

On Mon, 2006-08-14 at 13:31 +0100, Keir Fraser wrote:
> 
> 
> On 14/8/06 1:28 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> 
> > can we call this "xm core" or "xm crash"?
> > "dump" has so many meanings, I had plans to use it for "register" and
> > "memory" dump, tho I think even those should have better names.
> > thoughts?
> 
> Xm dump-core would be sensible.
> 
>  -- Keir
> 
> 

Re: [PATCH][XEN]xm dump command add on

[John Levon]

On Mon, Aug 14, 2006 at 05:03:59PM +0900, Ken Hironaka wrote:

> +        if((sts=copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem))<0)
> +            cpy_sts=sts;
> +
> +    if(cpy_sts!=0)
> +        goto error_out;

Should be:

	if ( ... )

> +        try:
> +            log.info("Dumping Core for Domain %s (%d).", dominfo.getName(),
> +                     dominfo.getDomid())

Better as "Domain core dump requested for domain %s (%d)" ?

> +    if not live:
> +        print "pausing domain:%s ..." % str(dom)
> +        server.xend.domain.pause(dom)

Is it really worth mentioning the pause? It doesn't take any amount of
time, surely.

regards
john

Re: [PATCH][XEN]xm dump command add on

[Ken Hironaka]

John, comment are enclosed.

On Mon, 2006-08-14 at 15:50 +0100, John Levon wrote:
> On Mon, Aug 14, 2006 at 05:03:59PM +0900, Ken Hironaka wrote:
> 
> > +        if((sts=copy_from_domain_page(xc_handle, domid,
page_array[i], dump_mem))<0)
> > +            cpy_sts=sts;
> > +
> > +    if(cpy_sts!=0)
> > +        goto error_out;
> 
> Should be:
> 
>       if ( ... )
> 
> > +        try:
> > +            log.info("Dumping Core for Domain %s (%d).",
dominfo.getName(),
> > +                     dominfo.getDomid())
> 

I must note, this section is in C code and not Python, as it is part of
xc_dumpcore() in libxc. I understand that it is better to log the error
as soon as possible, but logging takes place at the Python layer above.

> Better as "Domain core dump requested for domain %s (%d)" ?
> 
> > +    if not live:
> > +        print "pausing domain:%s ..." % str(dom)
> > +        server.xend.domain.pause(dom)
> 
> Is it really worth mentioning the pause? It doesn't take any amount of
> time, surely.
Yes, I do think that is is necessary to mention a pause. There are times
when the user will send an Interrupt with say, CTRL+C at the middle of
dumping. Of course, xm dump will terminate, but the domU will continue
to be paused and it will have to be unpaused manually. Of course, if you
read the inner workings I wrote you would understand, but I think it
will save a lot of people time wondering why their domain froze.

> regards
> john

best regards,
Ken

Re: [PATCH][XEN]xm dump command add on

[John Levon]

On Tue, Aug 15, 2006 at 08:49:57AM +0900, Ken Hironaka wrote:

> > Better as "Domain core dump requested for domain %s (%d)" ?
> > 
> I must note, this section is in C code and not Python, as it is part of
> xc_dumpcore() in libxc. I understand that it is better to log the error
> as soon as possible, but logging takes place at the Python layer above.

Not sure why that's relevant? I was merely asking for better text for
the message.

> > > +    if not live:
> > > +        print "pausing domain:%s ..." % str(dom)
> > > +        server.xend.domain.pause(dom)
> > 
> > Is it really worth mentioning the pause? It doesn't take any amount of
> > time, surely.
> Yes, I do think that is is necessary to mention a pause. There are times
> when the user will send an Interrupt with say, CTRL+C at the middle of
> dumping. Of course, xm dump will terminate, but the domU will continue

Perhaps that's a bug - it should clean up on a control-C by unpausing
the domain, if appropriate.

regards
john

Re: [PATCH][XEN]xm dump command add on

[Ken Hironaka]

On Tue, 2006-08-15 at 00:58 +0100, John Levon wrote:
> On Tue, Aug 15, 2006 at 08:49:57AM +0900, Ken Hironaka wrote:
> 
> > > Better as "Domain core dump requested for domain %s (%d)" ?
> > > 
> > I must note, this section is in C code and not Python, as it is part
of
> > xc_dumpcore() in libxc. I understand that it is better to log the
error
> > as soon as possible, but logging takes place at the Python layer
above.
> 
> Not sure why that's relevant? I was merely asking for better text for
> the message.

Oh ok, sorry I was just a little confused. thank you.
> 
> > > > +    if not live:
> > > > +        print "pausing domain:%s ..." % str(dom)
> > > > +        server.xend.domain.pause(dom)
> > > 
> > > Is it really worth mentioning the pause? It doesn't take any
amount of
> > > time, surely.
> > Yes, I do think that is is necessary to mention a pause. There are
times
> > when the user will send an Interrupt with say, CTRL+C at the middle
of
> > dumping. Of course, xm dump will terminate, but the domU will
continue
> 
> Perhaps that's a bug - it should clean up on a control-C by unpausing
> the domain, if appropriate.

Ok, I will take your advice into consideration. It's a simple keyboard
Interrupt exception catch so, it won't be a problem fixing it. I will
add these fixes and reemit the patch.

Thank you for your input.

best regards,
Ken

Re: [PATCH][XEN]xm dump command add on

[Akio Takebe]

Hi, Ken

Good work!
But you should check coding-style. ;-)

How about the following patch?
I think cpy_sts is always -1 or 0.
I think checking error counter is more worth than error status.
I don't test the following patch. This is RFC.

diff -r bef360142b62 tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c     Mon Aug 14 14:21:21 2006 -0600
+++ b/tools/libxc/xc_core.c     Wed Aug 16 09:19:08 2006 +0900
@@ -37,6 +37,7 @@ xc_domain_dumpcore_via_callback(int xc_h
     char dummy[PAGE_SIZE];
     int dummy_len;
     int sts;
+    int err_cnt = 0;
 
     if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
     {
@@ -103,7 +104,10 @@ xc_domain_dumpcore_via_callback(int xc_h
 
     for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
     {
-        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+        sts = copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+        if ( sts != 0 )
+            err_cnt++;
+
         dump_mem += PAGE_SIZE;
         if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
         {
@@ -112,6 +116,11 @@ xc_domain_dumpcore_via_callback(int xc_h
                 goto error_out;
             dump_mem = dump_mem_start;
         }
+    }
+
+    if ( err_cnt != 0 ){
+        IPRINTF("Could not copy from domid=%d page\n", domid);
+        goto error_out;
     }
 
     free(dump_mem_start);


Best Regards,

Akio Takebe

Re: [PATCH][XEN]xm dump command add on

[Akio Takebe]

Oops I send wrong patch.


>Hi, Ken
>
>Good work!
>But you should check coding-style. ;-)
>
>How about the following patch?
>I think cpy_sts is always -1 or 0.
>I think checking error counter is more worth than error status.
>I don't test the following patch. This is RFC.
>

How about the following patch? 

diff -r bef360142b62 tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c     Mon Aug 14 14:21:21 2006 -0600
+++ b/tools/libxc/xc_core.c     Wed Aug 16 09:59:21 2006 +0900
@@ -37,6 +37,7 @@ xc_domain_dumpcore_via_callback(int xc_h
     char dummy[PAGE_SIZE];
     int dummy_len;
     int sts;
+    unsigned int err_cnt = 0;
 
     if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
     {
@@ -103,7 +104,10 @@ xc_domain_dumpcore_via_callback(int xc_h
 
     for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
     {
-        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+        sts = copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+        if ( sts != 0 )
+            err_cnt++;
+
         dump_mem += PAGE_SIZE;
         if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
         {
@@ -112,6 +116,11 @@ xc_domain_dumpcore_via_callback(int xc_h
                 goto error_out;
             dump_mem = dump_mem_start;
         }
+    }
+
+    if ( err_cnt != 0 ){
+        IPRINTF("Could not copy from domid=%d (%d)pages\n", domid, err_cnt);
+        goto error_out;
     }
 
     free(dump_mem_start);

Best Regards,

Akio Takebe

Re: [PATCH][XEN]xm dump command add on

[Ken Hironaka]

Akio,

Thank you for your input. Though you are using an error counter, it
seems like you are still using it as an error status flag
if(err_cnt!=0)...
How is this different from simply setting 0 or -1?

On Wed, 2006-08-16 at 09:53 +0900, Akio Takebe wrote:
> Hi, Ken
> 
> Good work!
> But you should check coding-style. ;-)
> 
> How about the following patch?
> I think cpy_sts is always -1 or 0.
> I think checking error counter is more worth than error status.
> I don't test the following patch. This is RFC.
> 
> diff -r bef360142b62 tools/libxc/xc_core.c
> --- a/tools/libxc/xc_core.c     Mon Aug 14 14:21:21 2006 -0600
> +++ b/tools/libxc/xc_core.c     Wed Aug 16 09:19:08 2006 +0900
> @@ -37,6 +37,7 @@ xc_domain_dumpcore_via_callback(int xc_h
>      char dummy[PAGE_SIZE];
>      int dummy_len;
>      int sts;
> +    int err_cnt = 0;
>  
>      if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
>      {
> @@ -103,7 +104,10 @@ xc_domain_dumpcore_via_callback(int xc_h
>  
>      for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
>      {
> -        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
> +        sts = copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
> +        if ( sts != 0 )
> +            err_cnt++;
> +
>          dump_mem += PAGE_SIZE;
>          if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
>          {
> @@ -112,6 +116,11 @@ xc_domain_dumpcore_via_callback(int xc_h
>                  goto error_out;
>              dump_mem = dump_mem_start;
>          }
> +    }
> +
> +    if ( err_cnt != 0 ){
> +        IPRINTF("Could not copy from domid=%d page\n", domid);
> +        goto error_out;
>      }
>  
>      free(dump_mem_start);
> 
> 
> Best Regards,
> 
> Akio Takebe
> 

Re: [PATCH][XEN]xm dump command add on

[Akio Takebe]

Hi, Ken

Yes, This is wrong patch.
Please see another mail.
http://lists.xensource.com/archives/html/xen-devel/2006-08/msg00901.html

Best Regards,

Akio Takebe

>Akio,
>
>Thank you for your input. Though you are using an error counter, it
>seems like you are still using it as an error status flag
>if(err_cnt!=0)...
>How is this different from simply setting 0 or -1?
>
>On Wed, 2006-08-16 at 09:53 +0900, Akio Takebe wrote:
>> Hi, Ken
>> 
>> Good work!
>> But you should check coding-style. ;-)
>> 
>> How about the following patch?
>> I think cpy_sts is always -1 or 0.
>> I think checking error counter is more worth than error status.
>> I don't test the following patch. This is RFC.
>> 
>> diff -r bef360142b62 tools/libxc/xc_core.c
>> --- a/tools/libxc/xc_core.c     Mon Aug 14 14:21:21 2006 -0600
>> +++ b/tools/libxc/xc_core.c     Wed Aug 16 09:19:08 2006 +0900
>> @@ -37,6 +37,7 @@ xc_domain_dumpcore_via_callback(int xc_h
>>      char dummy[PAGE_SIZE];
>>      int dummy_len;
>>      int sts;
>> +    int err_cnt = 0;
>>  
>>      if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
>>      {
>> @@ -103,7 +104,10 @@ xc_domain_dumpcore_via_callback(int xc_h
>>  
>>      for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
>>      {
>> -        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
>> +        sts = copy_from_domain_page(xc_handle, domid, page_array[i], 
>> dump_mem);
>> +        if ( sts != 0 )
>> +            err_cnt++;
>> +
>>          dump_mem += PAGE_SIZE;
>>          if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
>>          {
>> @@ -112,6 +116,11 @@ xc_domain_dumpcore_via_callback(int xc_h
>>                  goto error_out;
>>              dump_mem = dump_mem_start;
>>          }
>> +    }
>> +
>> +    if ( err_cnt != 0 ){
>> +        IPRINTF("Could not copy from domid=%d page\n", domid);
>> +        goto error_out;
>>      }
>>  
>>      free(dump_mem_start);
>> 
>> 
>> Best Regards,
>> 
>> Akio Takebe
>> 

RE: [PATCH][XEN]xm dump command add on

[Graham, Simon]

Sorry to be slow replying - I have a couple of comments on the proposed
patch:

1. in xc_domain_dumpcore_via_callback why not just 'goto error_out;' if
sts<0? (as is done
   if the callback returns an error)

2. No big deal, but as I mentioned before, I don't know that there is
much value in having
   the -L and -C options to the 'xm dump-core' command since you can do
this by issuing 
   other xm commands before/after this one.

3. As others have commented, you really should log this command to
xend.log with log.info()
   (especially if you leave in the -L and -C options!) and the log
should include info on
   whether or not a pause/crash was done - this is very important when
you go back to try
   and work out why a domain paused or stopped unexpectedly!

Simon


> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-
> bounces@lists.xensource.com] On Behalf Of Ken Hironaka
> Sent: Monday, August 14, 2006 4:04 AM
> To: xen-devel@lists.xensource.com
> Subject: [Xen-devel] [PATCH][XEN]xm dump command add on
> 
> This adds the xm dump command to xend.
> The command outputs the memory contents of the specified domU to a
> coredump file.
> This will fail if tried on dom0.
> The interface is as follows:
> 
> xm dump [-L][-C] <domID> [output path]
> 
> -L Live dump:By default, xm dump does an xm pause, unpause before and
> after taking the dump, respectively. This option disables the
> pause/unpause and simply takes the dump.
> 
> -C crash dump: This executes an xm destroy after the dump file is
> complete.
> The output path is optional, and if it is not specified, the path will
> be
> /var/xen/dump/<domU name>.<domU ID>.core
> 
> This command uses the existant dumpCore(), which has been used for
> coredump when a domU crashed.
> In this patch, the xc_domain_dumpcore_via_callback() in xc_core.c of
> libxc is also modified. Previously, the
> xc_domain_dumpcore_via_callback() did not respond to error when
> copy_from_domain_page() failed. In other words, the dump core remained
> silent even if mapping the domain memory failed and its page could not
> be copied. When this happened, erroneous data had been dumped to the
> file without the user realizing it. Now, it has been modified so that
> if
> copy_from_domain_page fails, this fact is recorded in the logfile.
> However even in such cases, the dumping will continue as before.
> 
> Signed-off-by: Ken Hironaka <hironaka.ken@soft.fujitsu.com>
> 
> Reference
> http://lists.xensource.com/archives/html/xen-devel/2006-
> 08/msg00181.html
> http://lists.xensource.com/archives/html/xen-devel/2006-
> 08/msg00259.html
> http://lists.xensource.com/archives/html/xen-devel/2006-
> 08/msg00483.html

RE: [PATCH][XEN]xm dump command add on

[Ken Hironaka]

hey Simon,

comments are enclosed

On Wed, 2006-08-16 at 13:01 -0400, Graham, Simon wrote: 
> Sorry to be slow replying - I have a couple of comments on the proposed
> patch:
> 
> 1. in xc_domain_dumpcore_via_callback why not just 'goto error_out;' if
> sts<0? (as is done
>    if the callback returns an error)
> 
just because dumping failed on one page, it doesn't mean that the whole
dump is useless. in fact, there are many cases where the debugger would
like to see even a little information to help him/her. so i think it is
better to keep on dumping even if an error has occured. especially with
live dump, the page table state can change even while dumping, and if
that is the case, the dump can fail to dump a page even if it is not
caused by a critical error. instead, a message that shows how many pages
failed as been logged.

> 2. No big deal, but as I mentioned before, I don't know that there is
> much value in having
>    the -L and -C options to the 'xm dump-core' command since you can do
> this by issuing 
>    other xm commands before/after this one.
> 
you are right, they can be combined with other commands, but i think
they are options that would be expected an a regular dump command.

> 3. As others have commented, you really should log this command to
> xend.log with log.info()
>    (especially if you leave in the -L and -C options!) and the log
> should include info on
>    whether or not a pause/crash was done - this is very important when
> you go back to try
>    and work out why a domain paused or stopped unexpectedly!

i am not sure if i am misundertanding your comment, but they are logged
into Xend.log. since they are combined with xm pause/unpause and
destory, they are also logged as well by default. 
> 
> Simon

best regards,
Ken 
> 
> 
> > -----Original Message-----
> > From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-
> > bounces@lists.xensource.com] On Behalf Of Ken Hironaka
> > Sent: Monday, August 14, 2006 4:04 AM
> > To: xen-devel@lists.xensource.com
> > Subject: [Xen-devel] [PATCH][XEN]xm dump command add on
> > 
> > This adds the xm dump command to xend.
> > The command outputs the memory contents of the specified domU to a
> > coredump file.
> > This will fail if tried on dom0.
> > The interface is as follows:
> > 
> > xm dump [-L][-C] <domID> [output path]
> > 
> > -L Live dump:By default, xm dump does an xm pause, unpause before and
> > after taking the dump, respectively. This option disables the
> > pause/unpause and simply takes the dump.
> > 
> > -C crash dump: This executes an xm destroy after the dump file is
> > complete.
> > The output path is optional, and if it is not specified, the path will
> > be
> > /var/xen/dump/<domU name>.<domU ID>.core
> > 
> > This command uses the existant dumpCore(), which has been used for
> > coredump when a domU crashed.
> > In this patch, the xc_domain_dumpcore_via_callback() in xc_core.c of
> > libxc is also modified. Previously, the
> > xc_domain_dumpcore_via_callback() did not respond to error when
> > copy_from_domain_page() failed. In other words, the dump core remained
> > silent even if mapping the domain memory failed and its page could not
> > be copied. When this happened, erroneous data had been dumped to the
> > file without the user realizing it. Now, it has been modified so that
> > if
> > copy_from_domain_page fails, this fact is recorded in the logfile.
> > However even in such cases, the dumping will continue as before.
> > 
> > Signed-off-by: Ken Hironaka <hironaka.ken@soft.fujitsu.com>
> > 
> > Reference
> > http://lists.xensource.com/archives/html/xen-devel/2006-
> > 08/msg00181.html
> > http://lists.xensource.com/archives/html/xen-devel/2006-
> > 08/msg00259.html
> > http://lists.xensource.com/archives/html/xen-devel/2006-
> > 08/msg00483.html

RE: [PATCH][XEN]xm dump command add on

[Graham, Simon]

> > 1. in xc_domain_dumpcore_via_callback why not just 'goto error_out;'
> if
> > sts<0? (as is done
> >    if the callback returns an error)
> >
> just because dumping failed on one page, it doesn't mean that the
whole
> dump is useless. in fact, there are many cases where the debugger
would
> like to see even a little information to help him/her. so i think it
is
> better to keep on dumping even if an error has occured. especially
with
> live dump, the page table state can change even while dumping, and if
> that is the case, the dump can fail to dump a page even if it is not
> caused by a critical error. instead, a message that shows how many
> pages
> failed as been logged.
> 

OK - makes sense - might want to zero out the buffer in this case (or
fill it with CC's or somesuch) to avoid misinterpreting the data.

> > 3. As others have commented, you really should log this command to
> > xend.log with log.info()
> >    (especially if you leave in the -L and -C options!) and the log
> > should include info on
> >    whether or not a pause/crash was done - this is very important
> when
> > you go back to try
> >    and work out why a domain paused or stopped unexpectedly!
> 
> i am not sure if i am misundertanding your comment, but they are
logged
> into Xend.log. since they are combined with xm pause/unpause and
> destory, they are also logged as well by default.

OK - just wanted to make sure the results were output to the log as well
as to the person who issues the command...

Simon

RE: [PATCH][XEN]xm dump command add on

[Ken Hironaka]

Simon,

Comments are enclosed.
On Wed, 2006-08-16 at 22:47 -0400, Graham, Simon wrote:
> > > 1. in xc_domain_dumpcore_via_callback why not just 'goto error_out;'
> > if
> > > sts<0? (as is done
> > >    if the callback returns an error)
> > >
> > just because dumping failed on one page, it doesn't mean that the
> whole
> > dump is useless. in fact, there are many cases where the debugger
> would
> > like to see even a little information to help him/her. so i think it
> is
> > better to keep on dumping even if an error has occured. especially
> with
> > live dump, the page table state can change even while dumping, and if
> > that is the case, the dump can fail to dump a page even if it is not
> > caused by a critical error. instead, a message that shows how many
> > pages
> > failed as been logged.

Ok, I will release a new patch that fill failed dump pages with 0s.

best regards,
Ken

Re: [PATCH][XEN]xm dump command add on

[John Levon]

On Thu, Aug 17, 2006 at 05:13:42PM +0900, Ken Hironaka wrote:

> Ok, I will release a new patch that fill failed dump pages with 0s.

Sounds like another thing to consider in any new dump format: a list of
which pages couldn't be dumped?

regards
john

Re: [PATCH][XEN]xm dump command add on

[Ken Hironaka]

john,

 When you say a list of failed pages, do you mean something like the
indices of the pages which failed? I am thinking of perhaps outputting
the indices to a seperate file at the same path as the dump-core file.
Of course, the corresponding dump-core file will be added to the
list-file.

best regards,
Ken

On Mon, 2006-08-21 at 16:48 +0100, John Levon wrote:
> On Thu, Aug 17, 2006 at 05:13:42PM +0900, Ken Hironaka wrote:
> 
> > Ok, I will release a new patch that fill failed dump pages with 0s.
> 
> Sounds like another thing to consider in any new dump format: a list
of
> which pages couldn't be dumped?
> 
> regards
> john