On Feb 3, 2015, at 9:21 AM, Dave Anderson <anderson@redhat.com> wrote:


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://urldefense.proofpoint.com/v2/url?u=https-3A__code.google.com_p_vmsnparser_&d=AwICaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=g2Vka_25x09RSowRkQw8pA&m=3J4IwGSLqW8ZHsWMJNPAavx2BBitMJWjVmt9ezDNot0&s=UPT-x-iKlFSBKY6T8UoyujHjGnXRAGaBr1hrAnxlPIg&e=
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