[Crash-utility] patch to read vmware vmss file

[Dave Anderson]

Hello Dyno,

A couple more items to address.  

In the Makefile, the vmware_vmss.c build does not have a dependency on vmware_vmss.h:

  VMWARE_HFILES=vmware_vmss.h

  vmware_vmss.o: ${GENERIC_HFILES} ${REDHAT_HFILES} vmware_vmss.c
          ${CC} -c ${CRASH_CFLAGS} vmware_vmss.c ${WARNING_OPTIONS} ${WARNING_ERROR}

Another question -- vmware_vmss.h has just one #include:

  #include <stdbool.h>

That header file is not used in any of the top-level crash sources, and the gdb configuration
has a bunch of stuff where it checks for the existence of stdbool.h.

I note that the file is not located in /usr/include/stdbool.h, but does exist on 
my build system:

  $ find /usr/include -name stdbool.h
  /usr/include/c++/4.7.2/tr1/stdbool.h
  $ rpm -qf /usr/include/c++/4.7.2/tr1/stdbool.h
  libstdc++-devel-4.7.2-2.fc17.x86_64
  $ 

I'm not sure if there's some compiler magic that's picking it up from that location?

But just to prevent introducing *any* possible build errors on other systems that do
not have the file installed, would it be possible for you to change the "bool compressed" 
declaration to be an integer?

And lastly, please add a GPL header to the two source files.  You could use
sadump.h/sadump.c as templates.

Thanks,
  Dave

 

----- Original Message -----
> 
> 
> ----- Original Message -----
> > hi,
> > 
> > vmss file is VMware virtual machine snapshot file and contains all the
> > necessary memory dump that crash requires.
> > there is public available parse to read the format.
> > https://code.google.com/p/vmsnparser/
> > there is vmss2core ( https://labs.vmware.com/flings/vmss2core ) to convert it
> > to standard core file.
> > and this patch just enables crash to read it directly.
> > 
> > rgds,
> > Dyno
> 
> Hello Dyno,
> 
> Thank you very much for finally doing this.  It is a welcome addition.
> 
> I cannot do much but compile-test it, so it would be helpful
> if you could provide a sample VMSS vmcore available for future
> testing.  Can you make one available (preferably small but with
> more than one cpu)?
> 
> I have two small issues that I'd like you to address.  First,
> please add is_vmware_vmss and vmware_vmss_init prototypes to
> the others you have placed in defs.h:
> 
> $ make warn
> ... [ cut ] ...
> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6  main.c -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
> main.c: In function 'main':
> main.c:644:4: warning: implicit declaration of function 'is_vmware_vmss'
> [-Wimplicit-function-declaration]
> ... [ cut ] ...
> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6  filesys.c -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
> filesys.c: In function 'memory_source_init':
> filesys.c:249:4: warning: implicit declaration of function 'vmware_vmss_init'
> [-Wimplicit-function-declaration]
> ...
> 
> Secondly, I do not want to separate out MEMORY_SOURCES2/VMWARE_VMSS as you
> have done.  I understand that you did it because of the exhaustion
> of pc->flags bits.  However, we can simply do the same thing that was done
> when the SADUMP facility was added, which is to move one of the pre-existing
> pc->flags bits into pc->flags2.  There are several of them that can be
> safely moved.
> 
> I suggest moving GET_TIMESTAMP to pc->flags2.  Looking at its use from
> cscope, it is only used in these 3 places:
> 
>   C symbol: GET_TIMESTAMP
> 
>     File     Function    Line
>   0 defs.h   <global>    207 #define GET_TIMESTAMP (0x100000ULL)
>   1 kernel.c kernel_init 204 if (pc->flags & GET_TIMESTAMP) {
>   2 main.c   main        340 pc->flags |= GET_TIMESTAMP;
> 
> So please move GET_TIMESTAMP to pc->flags2, fix kernel_init() and main(),
> assign 0x100000ULL to VMWARE_VMSS, and put VMWARE_VMSS in MEMORY_SOURCES.
> 
> Again, I really appreciate your effort.  Can you repost this on short order?
> I am planning on doing the crash-7.1.0 release by the end of the week, and
> I would like to include this feature.
> 
> Thanks,
>   Dave