Re: [Xen-devel] [PATCH][XEN]xm dump command add on

[Ken Hironaka <hironaka.ken@xxxxxxxxxxxxxxxx>]

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
> 


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