dump memory when host pci device is used by guest
v2 cover dump memory when host pci device is used by guest
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.html
We have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Note:
1. The guest should be x86 or x86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.
4. If the guest OS is 32 bit and the memory size is larger than 4G, the vmcore is elf64 format. You should use the gdb which is built with --enable-64-bit-bfd.
Changes from v1 to v2:
1. fix virt addr in the vmcore.
Wen Congyang (5):
Add API to create memory mapping list
Add API to check whether a physical address is I/O address
target-i386: implement cpu_get_memory_mapping()
Add API to get memory mapping
introduce a new monitor command 'dump' to dump guest's memory
Makefile.target | 9 +- cpu-all.h | 10 + cpu-common.h | 1 + dump.c | 722 ++++++++++++++++++++++++++++++++++++++++++++++++++ dump.h | 6 + exec.c | 20 ++ hmp-commands.hx | 16 ++ memory_mapping.c | 183 +++++++++++++ memory_mapping.h | 30 ++ monitor.c | 3 + qmp-commands.hx | 24 ++ target-i386/helper.c | 239 +++++++++++++++++ 12 files changed, 1259 insertions(+), 4 deletions(-) create mode 100644 dump.c create mode 100644 dump.h create mode 100644 memory_mapping.c create mode 100644 memory_mapping.h
RFC PATCH v2 1/5 Add API to create memory mapping list
The memory mapping list stores virtual address and physical address mapping. The folloing patch will use this information to create PT_LOAD in the vmcore.
--- Makefile.target | 1 + memory_mapping.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ memory_mapping.h | 29 ++++++++++++ 3 files changed, 160 insertions(+), 0 deletions(-) create mode 100644 memory_mapping.c create mode 100644 memory_mapping.h
Makefile.target +1 -0
@@ -205,6 +205,7 @@ obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-y += memory.o +obj-y += memory_mapping.o LIBS+=-lz QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
memory_mapping.c new file +130 -0
130 lines not on this page · raw message
memory_mapping.h new file +29 -0
@@ -0,0 +1,29 @@ +#ifndef MEMORY_MAPPING_H +#define MEMORY_MAPPING_H + +#include "qemu-queue.h" + +typedef struct MemoryMapping { +target_phys_addr_t phys_addr; +target_ulong virt_addr; +ram_addr_t length; +QTAILQ_ENTRY (MemoryMapping) next; +}MemoryMapping ; + +typedef struct MemoryMappingList { +unsigned int num; + QTAILQ_HEAD(, MemoryMapping) head; +}MemoryMappingList ; + +void create_new_memory_mapping_head (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length); +void add_to_memory_mapping (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length); + +void free_memory_mapping_list (MemoryMappingList *list); + +#endif
Signature · 1 line
1.7.1
Re: [Qemu-devel] [RFC][PATCH 1/5 v2] Add API to create memory mapping list
Quoted 78 lines · Wen Congyang
The memory mapping list stores virtual address and physical address mapping. The folloing patch will use this information to create
PT_LOADin the vmcore.Signed-off-by Wen Congyang <[email protected]>--- Makefile.target | 1 + memory_mapping.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ memory_mapping.h | 29 ++++++++++++ 3 files changed, 160 insertions(+), 0 deletions(-) create mode 100644 memory_mapping.c create mode 100644 memory_mapping.hMakefile.target +1
-0@@ -205,6 +205,7 @@ obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-y += memory.o +obj-y += memory_mapping.o LIBS+=-lz QEMU_CFLAGS += $(VNC_TLS_CFLAGS)memory_mapping.c new file +48
-0@@ -0,0 +1,130 @@ +/* +* QEMU memory mapping +* +* Copyright Fujitsu, Corp. 2011 +* +* Authors: +* Wen Congyang <[email protected]> +* +* This work is licensed under the terms of the GNU GPL, version 2. See +* the COPYING file in the top-level directory. +* +*/ + +#include "cpu.h" +#include "cpu-all.h" +#include "memory_mapping.h" + +static MemoryMapping *last_mapping; + +static void create_new_memory_mapping (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length) +{ +MemoryMapping *memory_mapping, *p; + + memory_mapping =g_malloc (sizeof (MemoryMapping)); + memory_mapping->phys_addr = phys_addr; + memory_mapping->virt_addr = virt_addr; + memory_mapping->length = length; + last_mapping = memory_mapping; + list->num++; + QTAILQ_FOREACH(p, &list->head, next) { +if (p->phys_addr >= memory_mapping->phys_addr) { +QTAILQ_INSERT_BEFORE (p, memory_mapping, next); +return ; + } + } +QTAILQ_INSERT_TAIL (&list->head, memory_mapping, next); +return ; +} + +void create_new_memory_mapping_head (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length) +{ +MemoryMapping *memory_mapping;
memory_mapping.c line 58
Quoted 10 lines
memory_mapping.c new file +10
-0+ + memory_mapping =g_malloc (sizeof (MemoryMapping)); + memory_mapping->phys_addr = phys_addr; + memory_mapping->virt_addr = virt_addr; + memory_mapping->length = length; + last_mapping = memory_mapping; + list->num++; +QTAILQ_INSERT_HEAD (&list->head, memory_mapping, next); +return ; +}
Isn't create_new_memory_mapping_head just a special case of create_new_memory_mapping? And can't add_to_memory_mapping be used or extended so that create_new_memory_mapping_head becomes obsolete? Documenting the API would help at least me understanding the different semantics.
Quoted 58 lines
memory_mapping.c new file +58
-0+ +void add_to_memory_mapping (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length) +{ +MemoryMapping *memory_mapping; + +if (QTAILQ_EMPTY (&list->head)) { +create_new_memory_mapping (list, phys_addr, virt_addr, length); +return ; + } + +if (last_mapping) { +if ((phys_addr == (last_mapping->phys_addr + last_mapping->length)) && + (virt_addr == (last_mapping->virt_addr + last_mapping->length))) { + last_mapping->length += length; +return ; + } + } + + QTAILQ_FOREACH(memory_mapping, &list->head, next) { + last_mapping = memory_mapping; +if ((phys_addr == (last_mapping->phys_addr + last_mapping->length)) && + (virt_addr == (last_mapping->virt_addr + last_mapping->length))) { + last_mapping->length += length; +return ; + } + +if (!(phys_addr >= (last_mapping->phys_addr)) || + !(phys_addr < (last_mapping->phys_addr + last_mapping->length))) { +/* last_mapping does not contain this region */ +continue ; + } +if (!(virt_addr >= (last_mapping->virt_addr)) || + !(virt_addr < (last_mapping->virt_addr + last_mapping->length))) { +/* last_mapping does not contain this region */ +continue ; + } +if ((virt_addr - last_mapping->virt_addr) != + (phys_addr - last_mapping->phys_addr)) { +/* +* last_mapping contains this region, but we should create another +* mapping region. +*/ +break ; + } + +/* merge this region into last_mapping */ +if ((virt_addr + length) > + (last_mapping->virt_addr + last_mapping->length)) { + last_mapping->length = virt_addr + length - last_mapping->virt_addr; + } +return ; + } + +/* this region can not be merged into any existed memory mapping. */ +create_new_memory_mapping (list, phys_addr, virt_addr, length);
memory_mapping.c line 126
Quoted 10 lines
memory_mapping.c new file +10
-0+return ; +} + +void free_memory_mapping_list (MemoryMappingList *list) +{ +MemoryMapping *p, *q; + + QTAILQ_FOREACH_SAFE(p, &list->head, next, q) { +QTAILQ_REMOVE (&list->head, p, next); +g_free (p);
Can't last_mapping still point to this object?
Quoted 29 lines
memory_mapping.c new file +4
-0+ } + + list->num =0 ; +}memory_mapping.h new file +19
-0@@ -0,0 +1,29 @@ +#ifndef MEMORY_MAPPING_H +#define MEMORY_MAPPING_H + +#include "qemu-queue.h" + +typedef struct MemoryMapping { +target_phys_addr_t phys_addr; +target_ulong virt_addr; +ram_addr_t length; +QTAILQ_ENTRY (MemoryMapping) next; +}MemoryMapping ; + +typedef struct MemoryMappingList { +unsigned int num; + QTAILQ_HEAD(, MemoryMapping) head; +}MemoryMappingList ; + +void create_new_memory_mapping_head(MemoryMappingList *list, + target_phys_addr_t phys_addr,
memory_mapping.h line 29
Quoted 10 lines
memory_mapping.h new file +10
-0+target_phys_addr_t virt_addr, + ram_addr_t length); +void add_to_memory_mapping (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length); + +void free_memory_mapping_list (MemoryMappingList *list); + +#endif
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC][PATCH 1/5 v2] Add API to create memory mapping list
At 12/13/2011 09:03 PM, Jan Kiszka Write:
Quoted 87 lines · Wen Congyang
Quoted 86 lines
The memory mapping list stores virtual address and physical address mapping. The folloing patch will use this information to create
PT_LOADin the vmcore.Signed-off-by Wen Congyang <[email protected]>--- Makefile.target | 1 + memory_mapping.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ memory_mapping.h | 29 ++++++++++++ 3 files changed, 160 insertions(+), 0 deletions(-) create mode 100644 memory_mapping.c create mode 100644 memory_mapping.hMakefile.target +1
-0@@ -205,6 +205,7 @@ obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-y += memory.o +obj-y += memory_mapping.o LIBS+=-lz QEMU_CFLAGS += $(VNC_TLS_CFLAGS)memory_mapping.c new file +56
-0@@ -0,0 +1,130 @@ +/* +* QEMU memory mapping +* +* Copyright Fujitsu, Corp. 2011 +* +* Authors: +* Wen Congyang <[email protected]> +* +* This work is licensed under the terms of the GNU GPL, version 2. See +* the COPYING file in the top-level directory. +* +*/ + +#include "cpu.h" +#include "cpu-all.h" +#include "memory_mapping.h" + +static MemoryMapping *last_mapping; + +static void create_new_memory_mapping (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length) +{ +MemoryMapping *memory_mapping, *p; + + memory_mapping =g_malloc (sizeof (MemoryMapping)); + memory_mapping->phys_addr = phys_addr; + memory_mapping->virt_addr = virt_addr; + memory_mapping->length = length; + last_mapping = memory_mapping; + list->num++; + QTAILQ_FOREACH(p, &list->head, next) { +if (p->phys_addr >= memory_mapping->phys_addr) { +QTAILQ_INSERT_BEFORE (p, memory_mapping, next); +return ; + } + } +QTAILQ_INSERT_TAIL (&list->head, memory_mapping, next); +return ; +} + +void create_new_memory_mapping_head (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length) +{ +MemoryMapping *memory_mapping; + + memory_mapping =g_malloc (sizeof (MemoryMapping)); + memory_mapping->phys_addr = phys_addr; + memory_mapping->virt_addr = virt_addr; + memory_mapping->length = length; + last_mapping = memory_mapping; + list->num++; +QTAILQ_INSERT_HEAD (&list->head, memory_mapping, next);
Quoted 8 lines
memory_mapping.c line 58
Quoted 2 lines
memory_mapping.c new file +2
-0+return ; +}Isn't
create_new_memory_mapping_headjust a special case ofcreate_new_memory_mapping? And can'tadd_to_memory_mappingbe used or extended so thatcreate_new_memory_mapping_headbecomes obsolete? Documenting the API would help at least me understanding the different semantics.
The memory mapping list is sorted by physical address. If the memory mapping's num is greater than 2^16-2(the length of the value that contains program header table entry count is 16 bit), we must drop some mappings...
I drop the memory mappings according to physical address, so I sort it.
But crash will use the first PT_LOAD to calculate phys_offset, so I add the API create_new_memory_mapping_head() to add a specified memory mapping at the head of the list.
Do you have any better idea?
Quoted 63 lines
Quoted 63 lines
memory_mapping.c new file +62
-0+ +void add_to_memory_mapping (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length) +{ +MemoryMapping *memory_mapping; + +if (QTAILQ_EMPTY (&list->head)) { +create_new_memory_mapping (list, phys_addr, virt_addr, length); +return ; + } + +if (last_mapping) { +if ((phys_addr == (last_mapping->phys_addr + last_mapping->length)) && + (virt_addr == (last_mapping->virt_addr + last_mapping->length))) { + last_mapping->length += length; +return ; + } + } + + QTAILQ_FOREACH(memory_mapping, &list->head, next) { + last_mapping = memory_mapping; +if ((phys_addr == (last_mapping->phys_addr + last_mapping->length)) && + (virt_addr == (last_mapping->virt_addr + last_mapping->length))) { + last_mapping->length += length; +return ; + } + +if (!(phys_addr >= (last_mapping->phys_addr)) || + !(phys_addr < (last_mapping->phys_addr + last_mapping->length))) { +/* last_mapping does not contain this region */ +continue ; + } +if (!(virt_addr >= (last_mapping->virt_addr)) || + !(virt_addr < (last_mapping->virt_addr + last_mapping->length))) { +/* last_mapping does not contain this region */ +continue ; + } +if ((virt_addr - last_mapping->virt_addr) != + (phys_addr - last_mapping->phys_addr)) { +/* +* last_mapping contains this region, but we should create another +* mapping region. +*/ +break ; + } + +/* merge this region into last_mapping */ +if ((virt_addr + length) > + (last_mapping->virt_addr + last_mapping->length)) { + last_mapping->length = virt_addr + length - last_mapping->virt_addr; + } +return ; + } + +/* this region can not be merged into any existed memory mapping. */ +create_new_memory_mapping (list, phys_addr, virt_addr, length); +return ; +} + +void free_memory_mapping_list (MemoryMappingList *list)
Quoted 8 lines
memory_mapping.c line 126
Quoted 6 lines
memory_mapping.c new file +6
-0+{ +MemoryMapping *p, *q; + + QTAILQ_FOREACH_SAFE(p, &list->head, next, q) { +QTAILQ_REMOVE (&list->head, p, next); +g_free (p);Can't
last_mappingstill point to this object?
Yes, will fix it(set last_mapping to NULL in get memory mapping())
Thanks for your comment
Wen Congyang
Quoted 43 lines
Quoted 30 lines
memory_mapping.c new file +4
-0+ } + + list->num =0 ; +}memory_mapping.h new file +19
-0@@ -0,0 +1,29 @@ +#ifndef MEMORY_MAPPING_H +#define MEMORY_MAPPING_H + +#include "qemu-queue.h" + +typedef struct MemoryMapping { +target_phys_addr_t phys_addr; +target_ulong virt_addr; +ram_addr_t length; +QTAILQ_ENTRY (MemoryMapping) next; +}MemoryMapping ; + +typedef struct MemoryMappingList { +unsigned int num; + QTAILQ_HEAD(, MemoryMapping) head; +}MemoryMappingList ; + +void create_new_memory_mapping_head(MemoryMappingList *list, + target_phys_addr_t phys_addr,memory_mapping.h line 29
Quoted 10 lines
memory_mapping.h new file +10
-0+target_phys_addr_t virt_addr, + ram_addr_t length); +void add_to_memory_mapping (MemoryMappingList *list, +target_phys_addr_t phys_addr, +target_phys_addr_t virt_addr, +ram_addr_t length); + +void free_memory_mapping_list (MemoryMappingList *list); + +#endif Jan
RFC PATCH v2 2/5 Add API to check whether a physical address is I/O address
In reply to Wen CongyangThis API will be used in the following patch.
--- cpu-common.h | 1 + exec.c | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 0 deletions(-)
cpu-common.h +1 -0
@@ -123,6 +123,7 @@ struct CPUPhysMemoryClient {QLIST_ENTRY (CPUPhysMemoryClient) list; }; +bool is_io_addr (target_phys_addr_t phys_addr);void cpu_register_phys_memory_client (CPUPhysMemoryClient *);void cpu_unregister_phys_memory_client (CPUPhysMemoryClient *);
exec.c +20 -0
@@ -4786,3 +4786,23 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)#undef env#endif + +bool is_io_addr (target_phys_addr_t phys_addr) +{ +ram_addr_t pd; +PhysPageDesc *p; + + p =phys_page_find (phys_addr >>TARGET_PAGE_BITS ); +if (!p) { + pd =IO_MEM_UNASSIGNED ; + }else { + pd = p->phys_offset; + } + +if ((pd & ~TARGET_PAGE_MASK ) >IO_MEM_ROM && !(pd &IO_MEM_ROMD )) { +/* I/O region */ +return true; + } + +return false; +}
Signature · 1 line
1.7.1
RFC PATCH v2 3/5 target-i386: implement cpu_get_memory_mapping()
In reply to Wen CongyangWalk cpu's page table and collect all virtual address and physical address mapping. Then, add these mapping into memory mapping list.
--- cpu-all.h | 10 ++ target-i386/helper.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 249 insertions(+), 0 deletions(-)
cpu-all.h +10 -0
@@ -22,6 +22,7 @@#include "qemu-common.h" #include "qemu-tls.h" #include "cpu-common.h" +#include "memory_mapping.h" /* some important defines: *
@@ -584,4 +585,13 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);int cpu_memory_rw_debug (CPUState *env,target_ulong addr,uint8_t *buf,int len,int is_write); +#if defined(TARGET_I386 ) +void cpu_get_memory_mapping (MemoryMappingList *list,CPUState *env); +#else +static inline void cpu_get_memory_mapping (MemoryMappingList *list, +CPUState *env) +{ +} +#endif +#endif /* CPU_ALL_H */
target-i386/helper.c +239 -0
242 lines not on this page · raw message
Signature · 1 line
1.7.1
RFC PATCH v2 4/5 Add API to get memory mapping
In reply to Wen CongyangAdd API to get all virtual address and physical address mapping. If there is no virtual address for some physical address, the virtual address is 0.
--- memory_mapping.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ memory_mapping.h | 1 + 2 files changed, 54 insertions(+), 0 deletions(-)
memory_mapping.c +53 -0
@@ -128,3 +128,56 @@ void free_memory_mapping_list(MemoryMappingList *list) list->num =0 ; } + +void get_memory_mapping (MemoryMappingList *list) +{ +CPUState *env; +MemoryMapping *memory_mapping; +RAMBlock *block; +ram_addr_t offset, length; + +for (env = first_cpu; env !=NULL ; env = env->next_cpu) { +cpu_get_memory_mapping (list, env); + } + +/* some memory may be not mapped, add them into memory mapping's list */ + QLIST_FOREACH(block, &ram_list.blocks, next) { + offset = block->offset; + length = block->length; + + QTAILQ_FOREACH(memory_mapping, &list->head, next) { +if (memory_mapping->phys_addr >= (offset + length)) { +/* +* memory_mapping'list does not conatin the region +* [offset, offset+length) +*/ +create_new_memory_mapping (list, offset,0 , length); +break ; + } + +if ((memory_mapping->phys_addr + memory_mapping->length) <= + offset) { +continue ; + } + +if (memory_mapping->phys_addr > offset) { +/* +* memory_mapping'list does not conatin the region +* [offset, memory_mapping->phys_addr) +*/ +create_new_memory_mapping (list, offset,0 , + memory_mapping->phys_addr - offset); + } + +if ((offset + length) <= + (memory_mapping->phys_addr + memory_mapping->length)) { +break ; + } + length -= memory_mapping->phys_addr + memory_mapping->length - + offset; + offset = memory_mapping->phys_addr + memory_mapping->length; + } + } + +return ; +}
memory_mapping.h +1 -0
@@ -25,5 +25,6 @@ void add_to_memory_mapping(MemoryMappingList *list, ram_addr_t length);void free_memory_mapping_list (MemoryMappingList *list); +void get_memory_mapping (MemoryMappingList *list);#endif
Signature · 1 line
1.7.1
RFC PATCH 1/1 introduce a new monitor command 'dump' to dump guest's memory
In reply to Wen Congyang--- Makefile.target | 8 +- dump.c | 722 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ dump.h | 6 + hmp-commands.hx | 16 ++ monitor.c | 3 + qmp-commands.hx | 24 ++ 6 files changed, 775 insertions(+), 4 deletions(-) create mode 100644 dump.c create mode 100644 dump.h
Makefile.target +4 -4
@@ -118,7 +118,7 @@ $(call set-vpath, $(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR) -I$(SRC_PATH)/linux-user obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \ elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o \ - user-exec.o $(oslib-obj-y) + user-exec.o $(oslib-obj-y) dump.o obj-$(TARGET_HAS_BFLT) += flatload.o
@@ -156,7 +156,7 @@ LDFLAGS+=-Wl,-segaddr,__STD_PROG_ZONE,0x1000 -image_base 0x0e000000 LIBS+=-lmx obj-y = main.o commpage.o machload.o mmap.o signal.o syscall.o thunk.o \ - gdbstub.o user-exec.o + gdbstub.o user-exec.o dump.o obj-i386-y += ioport-user.o
@@ -178,7 +178,7 @@ $(call set-vpath, $(SRC_PATH)/bsd-user) QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user -I$(SRC_PATH)/bsd-user/$(TARGET_ARCH) obj-y = main.o bsdload.o elfload.o mmap.o signal.o strace.o syscall.o \ - gdbstub.o uaccess.o user-exec.o + gdbstub.o uaccess.o user-exec.o dump.o obj-i386-y += ioport-user.o
@@ -194,7 +194,7 @@ endif #CONFIG_BSD_USER # System emulator target ifdef CONFIG_SOFTMMU -obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o ioport.o +obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o ioport.o dump.o # virtio has to be here due to weird dependency between PCI and virtio-net. # need to fix this properly obj-$(CONFIG_NO_PCI) += pci-stub.o
dump.c new file +722 -0
722 lines not on this page · raw message
dump.h new file +6 -0
@@ -0,0 +1,6 @@ +#ifndef DUMP_H +#define DUMP_H + +int do_dump (Monitor *mon,const QDict *qdict,QObject **ret_data); + +#endif
hmp-commands.hx +16 -0
@@ -772,6 +772,22 @@ Migrate to @var{uri} (using -d to not wait for completion). ETEXI { + .name = "dump", + .args_type = "file:s", + .params = "file", + .help = "dump to file", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_dump, + }, + + +STEXI +@item dump @var{file} +@findex dump +Dump to @var{file}. +ETEXI + + { .name = "migrate_cancel", .args_type = "", .params = "",
monitor.c +3 -0
@@ -73,6 +73,9 @@#endif #include "hw/lm32_pic.h" +/* for dump */ +#include "dump.h" +//#define DEBUG //#define DEBUG_COMPLETION
qmp-commands.hx +24 -0
@@ -485,6 +485,30 @@ Notes: EQMP { + .name = "dump", + .args_type = "file:s", + .params = "file", + .help = "dump to file", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_dump, + }, + +SQMP +dump +------- + +Dump to file. + +Arguments: None. + +Example: + +-> { "execute": "dump", "arguments": { "file": "fd:dump" } } +<- { "return": {} } + +EQMP + + { .name = "migrate_cancel", .args_type = "", .params = "",
Signature · 1 line
1.7.1
Hello Wen,
From: Wen Congyang <[email protected]>
Subject: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Fri, 09 Dec 2011 15:57:26 +0800
Quoted 9 lines
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.htmlWe have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Quoted 7 lines
Note:
1. The guest should be x86 orx86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.
I guess still the current implementation breaks vmalloc'ed area that needs page tables originally located in the first 640kB, right? If you want to do so in a correct way, you need to identify a position of backup region and get data of 1st kernel's page tables.
But it needs debugging information of guest kernel, and I don't think it good idea that qemu uses too guest-specific information.
On the other hand, I have a basic question. Can this command used for creating live dump? or crash dump only?
Thanks.
HATAYAMA, Daisuke
Hi, hatayama-san
At 12/13/2011 11:12 AM, HATAYAMA Daisuke Write:
Quoted 23 lines
Hello Wen,
From: Wen Congyang <[email protected]>
Subject: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Fri, 09 Dec 2011 15:57:26 +0800Quoted 9 lines
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.htmlWe have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Quoted 7 lines
Note:
1. The guest should be x86 orx86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.
Quoted 4 lines
I guess still the current implementation breaks vmalloc'ed area that needs page tables originally located in the first 640kB, right? If you want to do so in a correct way, you need to identify a position of backup region and get data of 1st kernel's page tables.
I do not know anything about vmalloc'ed area. Can you explain it more detailed?
Quoted 6 lines
But it needs debugging information of guest kernel, and I don't think it good idea that qemu uses too guest-specific information.
On the other hand, I have a basic question. Can this command used for creating live dump? or crash dump only?
Do you mean dump guest's memory while it is running(do not stop the guest)? If so, this command can not be used for creating live dump.
Thanks
Wen Congyang
Quoted 5 lines
Thanks.
HATAYAMA, Daisuke
From: Wen Congyang <[email protected]>
Subject: Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Tue, 13 Dec 2011 11:35:53 +0800
Quoted 26 lines
Hi, hatayama-san
At 12/13/2011 11:12 AM, HATAYAMA Daisuke Write:
Quoted 23 lines
Hello Wen,
From: Wen Congyang <[email protected]>
Subject: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Fri, 09 Dec 2011 15:57:26 +0800Quoted 16 lines
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.htmlWe have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Note:
1. The guest should be x86 orx86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.
Quoted 8 lines
Quoted 4 lines
I guess still the current implementation breaks vmalloc'ed area that needs page tables originally located in the first 640kB, right? If you want to do so in a correct way, you need to identify a position of backup region and get data of 1st kernel's page tables.
I do not know anything about vmalloc'ed area. Can you explain it more detailed?
It's memory area not straight-mapped. To read the area, it's necessary to look up guest machine's page tables. If I understand correctly, your current implementation translates the vmalloc'ed area so that the generated vmcore is linearly mapped w.r.t. virtual-address for gdb to work.
kdump saves the first 640kB physical memory into the backup region. I guess, for some vmcores created by the current implementation, gdb and crash cannot see the vmalloc'ed memory area that needs page tables placed at the 640kB region, correctly. For example, try to use mod sub-command. Kernel modules are allocated on vmalloc'ed area.
I have developped a very similar logic for sadump. Look at sadump.c in crash. Logic itself is very simple, but debugging information is necessary. Documentation/kdump/kdump.txt and the following paper explains backup region mechanism very well, and the implementaion around there remains same now.
http://lse.sourceforge.net/kdump/documentation/ols2oo5-kdump-paper.pdf
On the other hand, have you written patch for crash to read this vmcore? I expect it's possible by a little fix to kcore code.
Quoted 4 lines
Do you mean dump guest's memory while it is running(do not stop the guest)? If so, this command can not be used for creating live dump.
I mean dump that keeps machine running as you say. Do you have plan for live dump?
Thanks.
HATAYAMA, Daisuke
At 12/13/2011 02:01 PM, HATAYAMA Daisuke Write:
Quoted 39 lines
From: Wen Congyang <[email protected]>
Subject: Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Tue, 13 Dec 2011 11:35:53 +0800Quoted 26 lines
Hi, hatayama-san
At 12/13/2011 11:12 AM, HATAYAMA Daisuke Write:
Quoted 23 lines
Hello Wen,
From: Wen Congyang <[email protected]>
Subject: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Fri, 09 Dec 2011 15:57:26 +0800Quoted 16 lines
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.htmlWe have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Note:
1. The guest should be x86 orx86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.Quoted 8 lines
Quoted 4 lines
I guess still the current implementation breaks vmalloc'ed area that needs page tables originally located in the first 640kB, right? If you want to do so in a correct way, you need to identify a position of backup region and get data of 1st kernel's page tables.
I do not know anything about vmalloc'ed area. Can you explain it more detailed?
Quoted 5 lines
It's memory area not straight-mapped. To read the area, it's necessary to look up guest machine's page tables. If I understand correctly, your current implementation translates the vmalloc'ed area so that the generated vmcore is linearly mapped w.r.t. virtual-address for gdb to work.
Do you mean the page table for vmalloc'ed area is stored in first 640KB, and it may be overwriten by the second kernel(this region has been backed up)?
Quoted 4 lines
kdump saves the first 640kB physical memory into the backup region. I guess, for some vmcores created by the current implementation, gdb and crash cannot see the vmalloc'ed memory area that needs page tables
Hmm, IIRC, crash do not use CPU's page table. gdb use the information in PT_LOAD to read memory area.
Quoted 8 lines
placed at the 640kB region, correctly. For example, try to use mod sub-command. Kernel modules are allocated on vmalloc'ed area.
I have developped a very similar logic for sadump. Look at
sadump.cin crash. Logic itself is very simple, but debugging information is necessary.Documentation/kdump/kdump.txtand the following paper explains backup region mechanism very well, and the implementaion around there remains same now.
Hmm, we can not use debugging information on qemu sied.
Quoted 5 lines
http://lse.sourceforge.net/kdump/documentation/ols2oo5-kdump-paper.pdf
On the other hand, have you written patch for crash to read this vmcore? I expect it's possible by a little fix to kcore code.
crash can read this vmcore without any change.
Thanks
Wen Congyang.
Quoted 13 lines
Quoted 5 lines
Do you mean dump guest's memory while it is running(do not stop the guest)? If so, this command can not be used for creating live dump.
I mean dump that keeps machine running as you say. Do you have plan for live dump?
Thanks.
HATAYAMA, Daisuke
From: Wen Congyang <[email protected]>
Subject: Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Tue, 13 Dec 2011 17:20:24 +0800
Quoted 40 lines
At 12/13/2011 02:01 PM, HATAYAMA Daisuke Write:
Quoted 39 lines
From: Wen Congyang <[email protected]>
Subject: Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Tue, 13 Dec 2011 11:35:53 +0800Quoted 34 lines
Hi, hatayama-san
At 12/13/2011 11:12 AM, HATAYAMA Daisuke Write:
Quoted 23 lines
Hello Wen,
From: Wen Congyang <[email protected]>
Subject: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Fri, 09 Dec 2011 15:57:26 +0800Quoted 9 lines
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.htmlWe have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Quoted 7 lines
Note:
1. The guest should be x86 orx86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.Quoted 4 lines
I guess still the current implementation breaks vmalloc'ed area that needs page tables originally located in the first 640kB, right? If you want to do so in a correct way, you need to identify a position of backup region and get data of 1st kernel's page tables.
I do not know anything about vmalloc'ed area. Can you explain it more detailed?
Quoted 9 lines
Quoted 5 lines
It's memory area not straight-mapped. To read the area, it's necessary to look up guest machine's page tables. If I understand correctly, your current implementation translates the vmalloc'ed area so that the generated vmcore is linearly mapped w.r.t. virtual-address for gdb to work.
Do you mean the page table for vmalloc'ed area is stored in first 640KB, and it may be overwriten by the second kernel(this region has been backed up)?
This might be wrong.. I've locally tried to ensure this but I have not done yet.
I make sure at least pgtlist_data could be within the first 640kB:
crash> log
<cut>
No NUMA configuration found
Faking a node at 0000000000000000-000000007f800000 Bootmem setup node 0 0000000000000000-000000007f800000
NODE_DATA [0000000000011000 - 0000000000044fff] <-- this bootmap [0000000000045000 - 0000000000054eff] pages 10
(7 early reservations) ==> bootmem [0000000000 - 007f800000]
#0 [0000000000 - 0000001000] BIOS data page ==> [0000000000 - 0000001000]
#1 [0000006000 - 0000008000] TRAMPOLINE ==> [0000006000 - 0000008000]
And I had ever had the vmcore created after entering 2nd kernel where I cannot see module data using mod sub-command, which was resolved by re-reading the address to the corresponding backup region.
I guess becuase crash uses page table on memory, this affects paging badly.
I want to look into this more but I don't have such vmcore now because I lost them accidentally... I tried to reproduce this some times yesterday but didn't succeed. The vmcore above is one of them.
Quoted 8 lines
Quoted 4 lines
kdump saves the first 640kB physical memory into the backup region. I guess, for some vmcores created by the current implementation, gdb and crash cannot see the vmalloc'ed memory area that needs page tables
Hmm, IIRC, crash do not use CPU's page table. gdb use the information in
PT_LOADto read memory area.
I was confused this. Your dump command uses CPU's page table.
So on the qemu side you can get page table over a whole physical address, right? If so, contents themselves are not broken, I think.
Quoted 11 lines
Quoted 8 lines
placed at the 640kB region, correctly. For example, try to use mod sub-command. Kernel modules are allocated on vmalloc'ed area.
I have developped a very similar logic for sadump. Look at
sadump.cin crash. Logic itself is very simple, but debugging information is necessary.Documentation/kdump/kdump.txtand the following paper explains backup region mechanism very well, and the implementaion around there remains same now.Hmm, we can not use debugging information on qemu sied.
How about re-reading them later in crash? Users want to see the 1st kernel rather than 2nd kernel.
To do it, the dump format must be able to be distingished from crash. Which does function in crash read vmcores created by this command? kcore, or netdump?
Thanks.
HATAYAMA, Daisuke
At 12/15/2011 09:30 AM, HATAYAMA Daisuke Write:
Quoted 57 lines
From: Wen Congyang <[email protected]>
Subject: Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Tue, 13 Dec 2011 17:20:24 +0800Quoted 40 lines
At 12/13/2011 02:01 PM, HATAYAMA Daisuke Write:
Quoted 39 lines
From: Wen Congyang <[email protected]>
Subject: Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Tue, 13 Dec 2011 11:35:53 +0800Quoted 34 lines
Hi, hatayama-san
At 12/13/2011 11:12 AM, HATAYAMA Daisuke Write:
Quoted 23 lines
Hello Wen,
From: Wen Congyang <[email protected]>
Subject: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest
Date: Fri, 09 Dec 2011 15:57:26 +0800Quoted 9 lines
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.htmlWe have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Quoted 7 lines
Note:
1. The guest should be x86 orx86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.Quoted 4 lines
I guess still the current implementation breaks vmalloc'ed area that needs page tables originally located in the first 640kB, right? If you want to do so in a correct way, you need to identify a position of backup region and get data of 1st kernel's page tables.
I do not know anything about vmalloc'ed area. Can you explain it more detailed?
Quoted 9 lines
Quoted 5 lines
It's memory area not straight-mapped. To read the area, it's necessary to look up guest machine's page tables. If I understand correctly, your current implementation translates the vmalloc'ed area so that the generated vmcore is linearly mapped w.r.t. virtual-address for gdb to work.
Do you mean the page table for vmalloc'ed area is stored in first 640KB, and it may be overwriten by the second kernel(this region has been backed up)?
This might be wrong.. I've locally tried to ensure this but I have not done yet.
Quoted 8 lines
I make sure at least
pgtlist_datacould be within the first 640kB:crash> log
<cut>
No NUMA configuration found
Faking a node at 0000000000000000-000000007f800000 Bootmem setup node 0 0000000000000000-000000007f800000
NODE_DATA[0000000000011000 -0000000000044fff] <-- this
Only kernel built with CONFIG_NUMA has this. This config is only enabled on RHEL x86_64. I do not have such env on hand now.
Quoted 39 lines
bootmap [0000000000045000 -
0000000000054eff] pages 10
(7 early reservations) ==> bootmem [0000000000 -007f800000]
#0 [0000000000 - 0000001000] BIOS data page ==> [0000000000 - 0000001000]
#1 [0000006000 - 0000008000] TRAMPOLINE ==> [0000006000 - 0000008000]And I had ever had the vmcore created after entering 2nd kernel where I cannot see module data using mod sub-command, which was resolved by re-reading the address to the corresponding backup region.
I guess becuase crash uses page table on memory, this affects paging badly.
I want to look into this more but I don't have such vmcore now because I lost them accidentally... I tried to reproduce this some times yesterday but didn't succeed. The vmcore above is one of them.
Quoted 8 lines
Quoted 4 lines
kdump saves the first 640kB physical memory into the backup region. I guess, for some vmcores created by the current implementation, gdb and crash cannot see the vmalloc'ed memory area that needs page tables
Hmm, IIRC, crash do not use CPU's page table. gdb use the information in
PT_LOADto read memory area.I was confused this. Your dump command uses CPU's page table.
So on the qemu side you can get page table over a whole physical address, right? If so, contents themselves are not broken, I think.
Quoted 9 lines
Quoted 8 lines
placed at the 640kB region, correctly. For example, try to use mod sub-command. Kernel modules are allocated on vmalloc'ed area.
I have developped a very similar logic for sadump. Look at
sadump.cin crash. Logic itself is very simple, but debugging information is necessary.Documentation/kdump/kdump.txtand the following paper explains backup region mechanism very well, and the implementaion around there remains same now.
Quoted 5 lines
Quoted 2 lines
Hmm, we can not use debugging information on qemu sied.
How about re-reading them later in crash? Users want to see the 1st kernel rather than 2nd kernel.
A easy way to see the 1st kernel is: specify --machdep phys_base=xxx in the command line.
Thanks
Wen Congyang
Quoted 9 lines
To do it, the dump format must be able to be distingished from crash. Which does function in crash read vmcores created by this command? kcore, or netdump?
Thanks.
HATAYAMA, Daisuke
Quoted 38 lines · Wen Congyang
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.htmlWe have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Note:
1. The guest should be x86 orx86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.
4. If the guest OS is 32 bit and the memory size is larger than 4G, the vmcore is elf64 format. You should use the gdb which is built with --enable-64-bit-bfd.Changes from v1 to v2:
1. fix virt addr in the vmcore.Wen Congyang (5):
Add API to create memory mapping list
Add API to check whether a physical address is I/O address
target-i386: implementcpu_get_memory_mapping()
Add API to get memory mapping
introduce a new monitor command 'dump' to dump guest's memoryMakefile.target | 9 +- cpu-all.h | 10 + cpu-common.h | 1 + dump.c | 722 ++++++++++++++++++++++++++++++++++++++++++++++++++ dump.h | 6 + exec.c | 20 ++ hmp-commands.hx | 16 ++ memory_mapping.c | 183 +++++++++++++ memory_mapping.h | 30 ++
Quoted 8 lines
monitor.c | 3 + qmp-commands.hx | 24 ++ target-i386/helper.c | 239 +++++++++++++++++ 12 files changed, 1259 insertions(+), 4 deletions(-) create mode 100644 dump.c create mode 100644 dump.h create mode 100644 memory_mapping.c create mode 100644 memory_mapping.h
A general remark regarding code organization: Please factor out the target specific bits and push them into target-*/dump.[ch] or whatever appropriate file in that folder. Ugly #ifdefs should be avoided in generic code as far as possible.
Thanks,
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
At 12/13/2011 08:55 PM, Jan Kiszka Write:
Quoted 48 lines · Wen Congyang
Quoted 38 lines
Hi, all
'virsh dump' can not work when host pci device is used by guest. We have discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.htmlWe have determined to introduce a new command dump to dump memory. The core file's format can be elf.
Note:
1. The guest should be x86 orx86_64. The other arch is not supported.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can work by specifying '--machdep phys_addr=xxx' in the command line. The reason is that the second kernel will update the page table, and we can not get the page table for the first kernel.
4. If the guest OS is 32 bit and the memory size is larger than 4G, the vmcore is elf64 format. You should use the gdb which is built with --enable-64-bit-bfd.Changes from v1 to v2:
1. fix virt addr in the vmcore.Wen Congyang (5):
Add API to create memory mapping list
Add API to check whether a physical address is I/O address
target-i386: implementcpu_get_memory_mapping()
Add API to get memory mapping
introduce a new monitor command 'dump' to dump guest's memoryMakefile.target | 9 +- cpu-all.h | 10 + cpu-common.h | 1 + dump.c | 722 ++++++++++++++++++++++++++++++++++++++++++++++++++ dump.h | 6 + exec.c | 20 ++ hmp-commands.hx | 16 ++ memory_mapping.c | 183 +++++++++++++ memory_mapping.h | 30 ++Quoted 8 lines
monitor.c | 3 + qmp-commands.hx | 24 ++ target-i386/helper.c | 239 +++++++++++++++++ 12 files changed, 1259 insertions(+), 4 deletions(-) create mode 100644 dump.c create mode 100644 dump.h create mode 100644 memory_mapping.c create mode 100644 memory_mapping.h
Quoted 4 lines
A general remark regarding code organization: Please factor out the target specific bits and push them into target-*/dump.[ch] or whatever appropriate file in that folder. Ugly #ifdefs should be avoided in generic code as far as possible.
OK. I will fix it. Thanks for pointing it out.
Thanks
Wen Congyang
Quoted 4 lines
Thanks,
Jan