[Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure

Marc-André Lureau posted 4 patches 8 years, 8 months ago
[Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Marc-André Lureau 8 years, 8 months ago
One way or another, the guest could communicate various dump info (via
guest agent or vmcoreinfo device) and populate that structure. It can
then be used to augment the dump with various details, as done in the
following patch.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/sysemu/dump-info.h | 18 ++++++++++++++++++
 dump.c                     |  3 +++
 2 files changed, 21 insertions(+)
 create mode 100644 include/sysemu/dump-info.h

diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
new file mode 100644
index 0000000000..d2378e15e2
--- /dev/null
+++ b/include/sysemu/dump-info.h
@@ -0,0 +1,18 @@
+#ifndef DUMP_INFO_H
+#define DUMP_INFO_H
+
+typedef struct DumpInfo {
+    bool received;
+    /* kernel base address */
+    bool has_phys_base;
+    uint64_t phys_base;
+    /* "_text" symbol location */
+    bool has_text;
+    uint64_t text;
+    /* the content of /sys/kernel/vmcoreinfo on Linux */
+    char *vmcoreinfo;
+} DumpInfo;
+
+extern DumpInfo dump_info;
+
+#endif /* DUMP_INFO_H */
diff --git a/dump.c b/dump.c
index d9090a24cc..bdf3270f02 100644
--- a/dump.c
+++ b/dump.c
@@ -20,6 +20,7 @@
 #include "monitor/monitor.h"
 #include "sysemu/kvm.h"
 #include "sysemu/dump.h"
+#include "sysemu/dump-info.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/memory_mapping.h"
 #include "sysemu/cpus.h"
@@ -38,6 +39,8 @@
 #define ELF_MACHINE_UNAME "Unknown"
 #endif
 
+DumpInfo dump_info = { 0, };
+
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val)
 {
     if (s->dump_info.d_endian == ELFDATA2LSB) {
-- 
2.13.0.91.g00982b8dd


Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Laszlo Ersek 8 years, 8 months ago
On 06/01/17 15:03, Marc-André Lureau wrote:
> One way or another, the guest could communicate various dump info (via
> guest agent or vmcoreinfo device) and populate that structure. It can
> then be used to augment the dump with various details, as done in the
> following patch.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/sysemu/dump-info.h | 18 ++++++++++++++++++
>  dump.c                     |  3 +++
>  2 files changed, 21 insertions(+)
>  create mode 100644 include/sysemu/dump-info.h
> 
> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
> new file mode 100644
> index 0000000000..d2378e15e2
> --- /dev/null
> +++ b/include/sysemu/dump-info.h
> @@ -0,0 +1,18 @@
> +#ifndef DUMP_INFO_H
> +#define DUMP_INFO_H
> +
> +typedef struct DumpInfo {
> +    bool received;
> +    /* kernel base address */
> +    bool has_phys_base;
> +    uint64_t phys_base;
> +    /* "_text" symbol location */
> +    bool has_text;
> +    uint64_t text;
> +    /* the content of /sys/kernel/vmcoreinfo on Linux */
> +    char *vmcoreinfo;
> +} DumpInfo;
> +
> +extern DumpInfo dump_info;
> +
> +#endif /* DUMP_INFO_H */
> diff --git a/dump.c b/dump.c
> index d9090a24cc..bdf3270f02 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -20,6 +20,7 @@
>  #include "monitor/monitor.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/dump.h"
> +#include "sysemu/dump-info.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/memory_mapping.h"
>  #include "sysemu/cpus.h"
> @@ -38,6 +39,8 @@
>  #define ELF_MACHINE_UNAME "Unknown"
>  #endif
>  
> +DumpInfo dump_info = { 0, };
> +
>  uint16_t cpu_to_dump16(DumpState *s, uint16_t val)
>  {
>      if (s->dump_info.d_endian == ELFDATA2LSB) {
> 

Can you please spell out, in the commit message, the reason for
introducing a new header file? (I suspect your reason, but it should be
documented explicitly.)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Eric Blake 8 years, 8 months ago
On 06/01/2017 01:06 PM, Laszlo Ersek wrote:
> On 06/01/17 15:03, Marc-André Lureau wrote:
>> One way or another, the guest could communicate various dump info (via
>> guest agent or vmcoreinfo device) and populate that structure. It can
>> then be used to augment the dump with various details, as done in the
>> following patch.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  include/sysemu/dump-info.h | 18 ++++++++++++++++++
>>  dump.c                     |  3 +++
>>  2 files changed, 21 insertions(+)
>>  create mode 100644 include/sysemu/dump-info.h
>>
>> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
>> new file mode 100644
>> index 0000000000..d2378e15e2
>> --- /dev/null
>> +++ b/include/sysemu/dump-info.h
>> @@ -0,0 +1,18 @@
>> +#ifndef DUMP_INFO_H
>> +#define DUMP_INFO_H

>>
> 
> Can you please spell out, in the commit message, the reason for
> introducing a new header file? (I suspect your reason, but it should be
> documented explicitly.)

Also, should you have a copyright header in the new file?  And does
MAINTAINERS cover it?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Marc-André Lureau 8 years, 8 months ago
Hi

On Thu, Jun 1, 2017 at 10:19 PM Eric Blake <eblake@redhat.com> wrote:

> On 06/01/2017 01:06 PM, Laszlo Ersek wrote:
> > On 06/01/17 15:03, Marc-André Lureau wrote:
> >> One way or another, the guest could communicate various dump info (via
> >> guest agent or vmcoreinfo device) and populate that structure. It can
> >> then be used to augment the dump with various details, as done in the
> >> following patch.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> ---
> >>  include/sysemu/dump-info.h | 18 ++++++++++++++++++
> >>  dump.c                     |  3 +++
> >>  2 files changed, 21 insertions(+)
> >>  create mode 100644 include/sysemu/dump-info.h
> >>
> >> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
> >> new file mode 100644
> >> index 0000000000..d2378e15e2
> >> --- /dev/null
> >> +++ b/include/sysemu/dump-info.h
> >> @@ -0,0 +1,18 @@
> >> +#ifndef DUMP_INFO_H
> >> +#define DUMP_INFO_H
>
> >>
> >
> > Can you please spell out, in the commit message, the reason for
> > introducing a new header file? (I suspect your reason, but it should be
> > documented explicitly.)
>
> Also, should you have a copyright header in the new file?  And does
> MAINTAINERS cover it?
>

None of the dump support is covered. Based on commit history, I can suggest
Wen Congyang, as original author.
Sadly, Qiao Nuohan cannot be reached with his mail today (anyone knows if
he is still contributing?). Laszlo has done significant changes and reviews
too. I can also propose myself to help with reviews.

Wen or Laszla, do you want to be the main maintainer?
-- 
Marc-André Lureau
Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Marc-André Lureau 8 years, 8 months ago
Hi

On Fri, Jun 2, 2017 at 1:46 PM Marc-André Lureau <marcandre.lureau@gmail.com>
wrote:

> Hi
>
> On Thu, Jun 1, 2017 at 10:19 PM Eric Blake <eblake@redhat.com> wrote:
>
>> On 06/01/2017 01:06 PM, Laszlo Ersek wrote:
>> > On 06/01/17 15:03, Marc-André Lureau wrote:
>> >> One way or another, the guest could communicate various dump info (via
>> >> guest agent or vmcoreinfo device) and populate that structure. It can
>> >> then be used to augment the dump with various details, as done in the
>> >> following patch.
>> >>
>> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >> ---
>> >>  include/sysemu/dump-info.h | 18 ++++++++++++++++++
>> >>  dump.c                     |  3 +++
>> >>  2 files changed, 21 insertions(+)
>> >>  create mode 100644 include/sysemu/dump-info.h
>> >>
>> >> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
>> >> new file mode 100644
>> >> index 0000000000..d2378e15e2
>> >> --- /dev/null
>> >> +++ b/include/sysemu/dump-info.h
>> >> @@ -0,0 +1,18 @@
>> >> +#ifndef DUMP_INFO_H
>> >> +#define DUMP_INFO_H
>>
>> >>
>> >
>> > Can you please spell out, in the commit message, the reason for
>> > introducing a new header file? (I suspect your reason, but it should be
>> > documented explicitly.)
>>
>> Also, should you have a copyright header in the new file?  And does
>> MAINTAINERS cover it?
>>
>
> None of the dump support is covered. Based on commit history, I can
> suggest Wen Congyang, as original author.
> Sadly, Qiao Nuohan cannot be reached with his mail today (anyone knows if
> he is still contributing?). Laszlo has done significant changes and reviews
> too. I can also propose myself to help with reviews.
>
> Wen or Laszla, do you want to be the main maintainer?
>

(sorry for the typo)

or rather "Supported" ("Someone is actually paid to look after this"
according to MAINTAINERS)
-- 
Marc-André Lureau
Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Laszlo Ersek 8 years, 8 months ago
On 06/02/17 11:55, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Jun 2, 2017 at 1:46 PM Marc-André Lureau <marcandre.lureau@gmail.com>
> wrote:
> 
>> Hi
>>
>> On Thu, Jun 1, 2017 at 10:19 PM Eric Blake <eblake@redhat.com> wrote:
>>
>>> On 06/01/2017 01:06 PM, Laszlo Ersek wrote:
>>>> On 06/01/17 15:03, Marc-André Lureau wrote:
>>>>> One way or another, the guest could communicate various dump info (via
>>>>> guest agent or vmcoreinfo device) and populate that structure. It can
>>>>> then be used to augment the dump with various details, as done in the
>>>>> following patch.
>>>>>
>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>> ---
>>>>>  include/sysemu/dump-info.h | 18 ++++++++++++++++++
>>>>>  dump.c                     |  3 +++
>>>>>  2 files changed, 21 insertions(+)
>>>>>  create mode 100644 include/sysemu/dump-info.h
>>>>>
>>>>> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
>>>>> new file mode 100644
>>>>> index 0000000000..d2378e15e2
>>>>> --- /dev/null
>>>>> +++ b/include/sysemu/dump-info.h
>>>>> @@ -0,0 +1,18 @@
>>>>> +#ifndef DUMP_INFO_H
>>>>> +#define DUMP_INFO_H
>>>
>>>>>
>>>>
>>>> Can you please spell out, in the commit message, the reason for
>>>> introducing a new header file? (I suspect your reason, but it should be
>>>> documented explicitly.)
>>>
>>> Also, should you have a copyright header in the new file?  And does
>>> MAINTAINERS cover it?
>>>
>>
>> None of the dump support is covered. Based on commit history, I can
>> suggest Wen Congyang, as original author.
>> Sadly, Qiao Nuohan cannot be reached with his mail today (anyone knows if
>> he is still contributing?). Laszlo has done significant changes and reviews
>> too. I can also propose myself to help with reviews.
>>
>> Wen or Laszla, do you want to be the main maintainer?
>>
> 
> (sorry for the typo)
> 
> or rather "Supported" ("Someone is actually paid to look after this"
> according to MAINTAINERS)
> 

Thanks, but I don't wish to have an official responsibility for this
feature. My current responsibilities are more than enough to keep me busy.

If you'd like to volunteer for maintaining the dump stuff, you have my
blessing of course (whatever weight that might carry).

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Zhang Chen 8 years, 8 months ago

On 06/03/2017 06:49 AM, Laszlo Ersek wrote:
> On 06/02/17 11:55, Marc-André Lureau wrote:
>> Hi
>>
>> On Fri, Jun 2, 2017 at 1:46 PM Marc-André Lureau <marcandre.lureau@gmail.com>
>> wrote:
>>
>>> Hi
>>>
>>> On Thu, Jun 1, 2017 at 10:19 PM Eric Blake <eblake@redhat.com> wrote:
>>>
>>>> On 06/01/2017 01:06 PM, Laszlo Ersek wrote:
>>>>> On 06/01/17 15:03, Marc-André Lureau wrote:
>>>>>> One way or another, the guest could communicate various dump info (via
>>>>>> guest agent or vmcoreinfo device) and populate that structure. It can
>>>>>> then be used to augment the dump with various details, as done in the
>>>>>> following patch.
>>>>>>
>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>> ---
>>>>>>   include/sysemu/dump-info.h | 18 ++++++++++++++++++
>>>>>>   dump.c                     |  3 +++
>>>>>>   2 files changed, 21 insertions(+)
>>>>>>   create mode 100644 include/sysemu/dump-info.h
>>>>>>
>>>>>> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
>>>>>> new file mode 100644
>>>>>> index 0000000000..d2378e15e2
>>>>>> --- /dev/null
>>>>>> +++ b/include/sysemu/dump-info.h
>>>>>> @@ -0,0 +1,18 @@
>>>>>> +#ifndef DUMP_INFO_H
>>>>>> +#define DUMP_INFO_H
>>>>> Can you please spell out, in the commit message, the reason for
>>>>> introducing a new header file? (I suspect your reason, but it should be
>>>>> documented explicitly.)
>>>> Also, should you have a copyright header in the new file?  And does
>>>> MAINTAINERS cover it?
>>>>
>>> None of the dump support is covered. Based on commit history, I can
>>> suggest Wen Congyang, as original author.
>>> Sadly, Qiao Nuohan cannot be reached with his mail today (anyone knows if
>>> he is still contributing?). Laszlo has done significant changes and reviews
>>> too. I can also propose myself to help with reviews.
>>>
>>> Wen or Laszla, do you want to be the main maintainer?

I can cc Qiao Nuohan<qiaonuohan@huawei.com>...

Thanks
Zhang Chen

>>>
>> (sorry for the typo)
>>
>> or rather "Supported" ("Someone is actually paid to look after this"
>> according to MAINTAINERS)
>>
> Thanks, but I don't wish to have an official responsibility for this
> feature. My current responsibilities are more than enough to keep me busy.
>
> If you'd like to volunteer for maintaining the dump stuff, you have my
> blessing of course (whatever weight that might carry).
>
> Thanks
> Laszlo
>
>
>
>

-- 
Thanks
Zhang Chen




Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Laszlo Ersek 8 years, 8 months ago
On 06/04/17 17:56, Zhang Chen wrote:
> 
> 
> On 06/03/2017 06:49 AM, Laszlo Ersek wrote:
>> On 06/02/17 11:55, Marc-André Lureau wrote:
>>> Hi
>>>
>>> On Fri, Jun 2, 2017 at 1:46 PM Marc-André Lureau
>>> <marcandre.lureau@gmail.com>
>>> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Jun 1, 2017 at 10:19 PM Eric Blake <eblake@redhat.com> wrote:
>>>>
>>>>> On 06/01/2017 01:06 PM, Laszlo Ersek wrote:
>>>>>> On 06/01/17 15:03, Marc-André Lureau wrote:
>>>>>>> One way or another, the guest could communicate various dump info
>>>>>>> (via
>>>>>>> guest agent or vmcoreinfo device) and populate that structure. It
>>>>>>> can
>>>>>>> then be used to augment the dump with various details, as done in
>>>>>>> the
>>>>>>> following patch.
>>>>>>>
>>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>> ---
>>>>>>>   include/sysemu/dump-info.h | 18 ++++++++++++++++++
>>>>>>>   dump.c                     |  3 +++
>>>>>>>   2 files changed, 21 insertions(+)
>>>>>>>   create mode 100644 include/sysemu/dump-info.h
>>>>>>>
>>>>>>> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
>>>>>>> new file mode 100644
>>>>>>> index 0000000000..d2378e15e2
>>>>>>> --- /dev/null
>>>>>>> +++ b/include/sysemu/dump-info.h
>>>>>>> @@ -0,0 +1,18 @@
>>>>>>> +#ifndef DUMP_INFO_H
>>>>>>> +#define DUMP_INFO_H
>>>>>> Can you please spell out, in the commit message, the reason for
>>>>>> introducing a new header file? (I suspect your reason, but it
>>>>>> should be
>>>>>> documented explicitly.)
>>>>> Also, should you have a copyright header in the new file?  And does
>>>>> MAINTAINERS cover it?
>>>>>
>>>> None of the dump support is covered. Based on commit history, I can
>>>> suggest Wen Congyang, as original author.
>>>> Sadly, Qiao Nuohan cannot be reached with his mail today (anyone
>>>> knows if
>>>> he is still contributing?). Laszlo has done significant changes and
>>>> reviews
>>>> too. I can also propose myself to help with reviews.
>>>>
>>>> Wen or Laszla, do you want to be the main maintainer?
> 
> I can cc Qiao Nuohan<qiaonuohan@huawei.com>...

That would be great, thanks!
Laszlo

> 
> Thanks
> Zhang Chen
> 
>>>>
>>> (sorry for the typo)
>>>
>>> or rather "Supported" ("Someone is actually paid to look after this"
>>> according to MAINTAINERS)
>>>
>> Thanks, but I don't wish to have an official responsibility for this
>> feature. My current responsibilities are more than enough to keep me
>> busy.
>>
>> If you'd like to volunteer for maintaining the dump stuff, you have my
>> blessing of course (whatever weight that might carry).
>>
>> Thanks
>> Laszlo
>>
>>
>>
>>
> 


Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Andrew Jones 8 years, 8 months ago
On Fri, Jun 02, 2017 at 09:46:35AM +0000, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jun 1, 2017 at 10:19 PM Eric Blake <eblake@redhat.com> wrote:
> 
> > On 06/01/2017 01:06 PM, Laszlo Ersek wrote:
> > > On 06/01/17 15:03, Marc-André Lureau wrote:
> > >> One way or another, the guest could communicate various dump info (via
> > >> guest agent or vmcoreinfo device) and populate that structure. It can
> > >> then be used to augment the dump with various details, as done in the
> > >> following patch.
> > >>
> > >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >> ---
> > >>  include/sysemu/dump-info.h | 18 ++++++++++++++++++
> > >>  dump.c                     |  3 +++
> > >>  2 files changed, 21 insertions(+)
> > >>  create mode 100644 include/sysemu/dump-info.h
> > >>
> > >> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
> > >> new file mode 100644
> > >> index 0000000000..d2378e15e2
> > >> --- /dev/null
> > >> +++ b/include/sysemu/dump-info.h
> > >> @@ -0,0 +1,18 @@
> > >> +#ifndef DUMP_INFO_H
> > >> +#define DUMP_INFO_H
> >
> > >>
> > >
> > > Can you please spell out, in the commit message, the reason for
> > > introducing a new header file? (I suspect your reason, but it should be
> > > documented explicitly.)
> >
> > Also, should you have a copyright header in the new file?  And does
> > MAINTAINERS cover it?
> >
> 
> None of the dump support is covered. Based on commit history, I can suggest
> Wen Congyang, as original author.
> Sadly, Qiao Nuohan cannot be reached with his mail today (anyone knows if
> he is still contributing?). Laszlo has done significant changes and reviews
> too. I can also propose myself to help with reviews.
> 
> Wen or Laszla, do you want to be the main maintainer?

What about Peter Xu (CC'ed)? He'll have to state whether he has time and
interest, but I see he's contributed quite a bit to dump.c.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 1/4] dump: add DumpInfo structure
Posted by Peter Xu 8 years, 8 months ago
On Mon, Jun 05, 2017 at 09:24:38AM +0200, Andrew Jones wrote:
> On Fri, Jun 02, 2017 at 09:46:35AM +0000, Marc-André Lureau wrote:
> > Hi
> > 
> > On Thu, Jun 1, 2017 at 10:19 PM Eric Blake <eblake@redhat.com> wrote:
> > 
> > > On 06/01/2017 01:06 PM, Laszlo Ersek wrote:
> > > > On 06/01/17 15:03, Marc-André Lureau wrote:
> > > >> One way or another, the guest could communicate various dump info (via
> > > >> guest agent or vmcoreinfo device) and populate that structure. It can
> > > >> then be used to augment the dump with various details, as done in the
> > > >> following patch.
> > > >>
> > > >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > >> ---
> > > >>  include/sysemu/dump-info.h | 18 ++++++++++++++++++
> > > >>  dump.c                     |  3 +++
> > > >>  2 files changed, 21 insertions(+)
> > > >>  create mode 100644 include/sysemu/dump-info.h
> > > >>
> > > >> diff --git a/include/sysemu/dump-info.h b/include/sysemu/dump-info.h
> > > >> new file mode 100644
> > > >> index 0000000000..d2378e15e2
> > > >> --- /dev/null
> > > >> +++ b/include/sysemu/dump-info.h
> > > >> @@ -0,0 +1,18 @@
> > > >> +#ifndef DUMP_INFO_H
> > > >> +#define DUMP_INFO_H
> > >
> > > >>
> > > >
> > > > Can you please spell out, in the commit message, the reason for
> > > > introducing a new header file? (I suspect your reason, but it should be
> > > > documented explicitly.)
> > >
> > > Also, should you have a copyright header in the new file?  And does
> > > MAINTAINERS cover it?
> > >
> > 
> > None of the dump support is covered. Based on commit history, I can suggest
> > Wen Congyang, as original author.
> > Sadly, Qiao Nuohan cannot be reached with his mail today (anyone knows if
> > he is still contributing?). Laszlo has done significant changes and reviews
> > too. I can also propose myself to help with reviews.
> > 
> > Wen or Laszla, do you want to be the main maintainer?
> 
> What about Peter Xu (CC'ed)? He'll have to state whether he has time and
> interest, but I see he's contributed quite a bit to dump.c.

I believe those were the first commits of mine when working on
detached dump, after that I didn't really work on it anymore. So I
guess I may not be the best candidate.

Also, I admit my bandwidth is limited as well recently. Thanks Drew
for mentioning me anyway. :-)

-- 
Peter Xu