[Crash-utility] [PATCH 0/7] Upgrade the remote access to a xen domain.
Don Slutz
dslutz at verizon.com
Wed Nov 20 23:07:48 UTC 2013
On 11/20/13 14:58, Dave Anderson wrote:
> Hi Don,
>
> This v2 version, plus the REMOTE_MEMSRC() patch below, is queued for
> crash-7.0.4. How would you like the changelog message to read?
> Tentatively I've got this:
>
> - Resurrection of the remote analysis capability for use with the
> "xen-crashd" daemon running on a Xen Dom0 host, which communicates
> with a paused or shutdown DomU guest kernel. The daemon can be
> accessed like so:
>
> $ crash localhost:5991,/dev/xenmem vmlinux
>
> (dslutz at verizon.com)
>
> Thanks,
> Dave
>
That changelog message looks good. However the default port that I have posted to xen-devel is 5001. So it would help to change 5991 to 5001. (I was testing remotely a lot, and had opened port 5991 in the firewall. So I just got use to using 5991...)
-Don Slutz
> ----- Original Message -----
>> On 11/19/13 14:28, Dave Anderson wrote:
>>> Hi Don,
>>>
>>> I still need some help understanding this "resurrection" of the remote
>>> access facility, i.e., the new meanings of the REMOTE_xxx flags/macros.
>>>
>>> The currently-existing comment for the is_remote_daemon() function
>>> in remote.c describes the (obsolete) usage of the facility:
>>>
>>> /*
>>> * Parse, verify and establish a connection with the network daemon
>>> * specified on the crash command line.
>>> *
>>> * The format is:
>>> [remote-hostname]:port[,remote-namelist][,remote-dumpfile]
>>> *
>>> * where everything but the port number is optional, and the
>>> remote-namelist
>>> * and remote-dumpfile can be reversed.
>>> *
>>> * 1. The default remote host is the local host.
>>> * 2. The default dumpfile is /dev/mem.
>>> * 3. If no remote-namelist and remote-dumpfile are given, the daemon
>>> * is queried for a kernel that matches the remote /proc/version.
>>> * If no local kernel namelist is entered, the remote version will
>>> * be copied locally when fd_init() is called.
>>> * 4. If a remote-dumpfile is given with no remote namelist, it is
>>> presumed
>>> * that the kernel namelist will be entered locally.
>>> */
>>>
>>> Back then, the remote "crashd" daemon could be contacted for:
>>>
>>> (1) live access to the remote machine's kernel using its /dev/mem, or
>>> (2) accessing a vmlinux/vmcore pair that existed on the remote machine.
>>>
>>> Can you explain this new usage? It sounds like there is a remote daemon
>>> running on a remote Dom0 kernel, and that your new functionality can only
>>> be used to:
>>>
>>> (1) access a running domU guest hosted by that remote system, or
>>> (2) access a paused domU guest hosted by that remote system.
>>>
>>> And that there is also a device that the the remote daemon uses
>>> to access the memory of the guest, be it paused or live.
>>>
>>> Am I in the ballpark?
>> Yes.
>>
>> The currently proposed code for Dom0, does not support (1) mostly
>> because it seems much better to me to be able to poke around and look at
>> a fixed state of the domU.
>>
>> There is a (3) in xen: guest has shutdown. A shutdown guest has
>> subtypes like "requested shutdown", "requested reboot", "detected
>> crash", etc. Xen will set a guest in shutdown state when a double fault
>> or triple fault happens. A paused guest is the same as an unpaused
>> guest in shutdown state. Using a much older Xen, I known that that
>> version treated a double fault is the same as a reboot request. The bug
>> I was tracking down turned out to be a extra swapgs which will cause
>> many linux kernel's to double fault on a page fault.
>>
>> I was also looking to be able to use old versions of crash so as to
>> make it easier to use and/or deploy. Since the 7.0.3 or older version of
>> crash only support "live access to the remote machine's kernel using its
>> /dev/mem "; I picked to default to be a paused system. Since crash
>> thinks the remote system is live, it keeps fetching state which is not
>> changing.
>>
>> The "device that the the remote daemon uses ", is more a defined
>> ABI which has 2 layers libxl and libxc (what I all xc_* routines). In
>> this case I am using libxc calls. None of these require the guest to be
>> paused.
>>
>> So it would not be hard to change the Xen code to support new
>> option (1) if there is need for such a mode. I could see adding some
>> sort of pause control as an extension to crash.
>>
>> I have recently learn of another Xen tool: gdbsx. It is designed
>> to connect gdb to a running guest and pause and resume the guest on
>> demand. While I have not done very much with it, it looks to be
>> powerful. Since it is just gdb, it is missing crash's kernel support.
>>
>>> Dave
>> While working on this email, I found out that it looks better to me to
>> include REMOTE_PAUSED() in REMOTE_MEMSRC(). I.E. either add the following:
>>
>> From b4093734b2db8666f155032ffd9637dcfda0fc0b Mon Sep 17 00:00:00 2001
>> From: Don Slutz <dslutz at verizon.com>
>> Date: Wed, 20 Nov 2013 07:17:51 -0500
>> Subject: [PATCH] Add REMOTE_PAUSED() to REMOTE_MEMSRC()
>>
>> And remove the redunent 'REMOTE_MEMSRC() || REMOTE_PAUSED()'.
>>
>> Signed-off-by: Don Slutz <dslutz at verizon.com>
>> ---
>> defs.h | 2 +-
>> memory.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/defs.h b/defs.h
>> index dae9662..2ec5540 100755
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -254,7 +254,7 @@ struct number_option {
>> #define REMOTE_ACTIVE() (pc->flags & REM_LIVE_SYSTEM)
>> #define REMOTE_DUMPFILE() \
>> (pc->flags & (REM_NETDUMP|REM_MCLXCD|REM_LKCD|REM_S390D))
>> -#define REMOTE_MEMSRC() (REMOTE_ACTIVE() || REMOTE_DUMPFILE())
>> +#define REMOTE_MEMSRC() (REMOTE_ACTIVE() || REMOTE_PAUSED() ||
>> REMOTE_DUMPFILE())
>> #define LKCD_DUMPFILE() (pc->flags & (LKCD|REM_LKCD))
>> #define NETDUMP_DUMPFILE() (pc->flags & (NETDUMP|REM_NETDUMP))
>> #define DISKDUMP_DUMPFILE() (pc->flags & DISKDUMP)
>> diff --git a/memory.c b/memory.c
>> index cf8da2c..d17ba3c 100755
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -14903,7 +14903,7 @@ memory_page_size(void)
>> if (machdep->pagesize)
>> return machdep->pagesize;
>>
>> - if (REMOTE_MEMSRC() || REMOTE_PAUSED())
>> + if (REMOTE_MEMSRC())
>> return remote_page_size();
>>
>> switch (pc->flags & MEMORY_SOURCES)
>> --
>> 1.7.11.7
>>
>> or adjust patch v2 7/7 to also do this.
>>
>>
>> -Don Slutz
>>> ----- Original Message -----
>>>> On 11/19/13 11:08, Dave Anderson wrote:
>>>>> ----- Original Message -----
>>>>>> From: Don Slutz <dslutz at verizon.com>
>>>>>>
>>>>>> Currently crash 4.0 and later using the code:
>>>>>>
>>>>>> http://lists.xen.org/archives/html/xen-devel/2013-11/msg02569.html
>>>>>>
>>>>>> There is some very limited documention on crashes remote ptotocol in:
>>>>>>
>>>>>> http://lists.xen.org/archives/html/xen-devel/2013-11/msg02352.html
>>>>>>
>>>>>> and some fixes in the attachment
>>>>>> 0001-xen-crashd-Connect-crash-with-domain.patch
>>>>>> from the 1st link above.
>>>>>>
>>>>>> How ever there are issues with the current code:
>>>>>>
>>>>>> 1) The remote protocol has a minor issue (which I have not been able to
>>>>>> happen) based on the fact TCP/IP is stream based protocol. This
>>>>>> means
>>>>>> a RECV or a SEND may not do the fully requested size of data. In
>>>>>> fact
>>>>>> the current code assumes that the amount of data that a SEND is
>>>>>> called
>>>>>> with will all be read with a single RECV.
>>>>>>
>>>>>> 2) The most common mismatch between crash and older kernels is
>>>>>> phys_base. In the remote case, see if the remote server supports
>>>>>> vitrual memory access, and if so, see if phys_base can be
>>>>>> retreived.
>>>>>>
>>>>>> 3) crash assumes that the remote system is active and can not return
>>>>>> currect IP and SP to do a better back trace.
>>>>>>
>>>>>> 4) enable a non-active mode of remote access.
>>>>>>
>>>>>> This patch set attempts to fix these:
>>>>>>
>>>>>> The fix for #1 I have called NIL mode (patch 1).
>>>>>> The fix for #2 uses get_remote_phys_base (patch 3).
>>>>>> The fix for #3 uses get_remote_regs (patch 5).
>>>>>> The fix for #4 uses special file /dev/xenmem (patch 7).
>>>>>> It also REMOTE_NIL() to indicate "remote paused system".
>>>>>>
>>>>>> Don Slutz (7):
>>>>>> Add NIL mode to remote.
>>>>>> remote_proc_version: NULL terminate passed buffer on error.
>>>>>> Add get_remote_phys_base.
>>>>>> Add remote_vtop.
>>>>>> bt: get remote live registers if possible.
>>>>>> Add get_remote_cr3
>>>>>> Add support for non-live remote.
>>>>>>
>>>>>> defs.h | 8 +-
>>>>>> kernel.c | 25 +++-
>>>>>> memory.c | 25 +++-
>>>>>> remote.c | 462
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++----------
>>>>>> x86_64.c | 24 ++++
>>>>>> 5 files changed, 465 insertions(+), 79 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 1.8.4
>>>>>
>>>>> Hi Don,
>>>>>
>>>>> A few comments...
>>>>>
>>>>> I don't really understand what entity the code is communicating with,
>>>>> so for the most part, I'm primarily interested in making sure that your
>>>>> patches in no way can affect the normal usage of the crash utility.
>>>>>
>>>>> That being said, is the term "NIL mode" some kind of Xen-ism?
>>>> Nope, it is term I came up with to describe the protocol change.
>>>>> It wasn't
>>>>> until I saw the display_sys_stats() function that I even understood that
>>>>> REMOTE_NIL mean "(remote paused system)". Anyway, if you made it
>>>>> something
>>>>> like "REMOTE_PAUSED" or something like that, the code might make some
>>>>> sense
>>>>> to the untrained eye.
>>>> Will do. Looking back at it, I ended up overloading the term. :(
>>>>> And it's not entirely clear to me what REMOTE_ACTIVE() means now? It
>>>>> used
>>>>> to imply communicating with a remote crash daeamon that accessed the live
>>>>> system that the daemon was running on. Has that meaning been changed to
>>>>> mean
>>>>> that crash is communicating with a daemon that's accessing a "live" Xen
>>>>> guest
>>>>> kernel?
>>>> The meaning is not as clear. Since the existing versions of crash already
>>>> have REMOTE_ACTIVE() and I wanted the Xen utility to be usable to them, in
>>>> both cases the "live" Xen guest kernel is being accessed while it is
>>>> paused.
>>>> I added REMOTE_PAUSED() to have a few changes to crash to correctly
>>>> understand that the guest is paused. As part of this I add the fake file
>>>> "/dev/xenmem".
>>>>> I'm confused about these two changes, where you re-patch your own patch:
>>>>>
>>>>> - else
>>>>> + else if (REMOTE_ACTIVE() && ((bt->task == tt->this_task) ||
>>>>> is_task_active(bt->task))) {
>>>>> + if (get_remote_regs(bt, &eip, &esp))
>>>>> + machdep->get_stack_frame(bt, &eip, &esp);
>>>>> + } else
>>>>>
>>>>> followed by:
>>>>>
>>>>> - else if (REMOTE_ACTIVE() && ((bt->task == tt->this_task) ||
>>>>> is_task_active(bt->task))) {
>>>>> + else if (REMOTE_NIL() && ((bt->task == tt->this_task) ||
>>>>> is_task_active(bt->task))) {
>>>>> if (get_remote_regs(bt, &eip, &esp))
>>>>> machdep->get_stack_frame(bt, &eip, &esp);
>>>>> } else
>>>> Clearly I need to adjust the patch order.
>>>>> If I'm not mistaken, it would seem to be impossible for "(bt->task ==
>>>>> tt->this_task)"
>>>>> to ever be true in the REMOTE_NIL() case? As I recall tt->this_task used
>>>>> to be the
>>>>> PID of the remote daemon itself, which would be accessing the memory of
>>>>> its
>>>>> own kernel.
>>>>> So how could the pid of the remote daemon ever be the target of a back
>>>>> trace of a
>>>>> paused guest? And so I don't understand why you first made an if clause
>>>>> for
>>>>> REMOTE_ACTIVE() to begin with -- whatever that means now -- and then
>>>>> changed it to
>>>>> REMOTE_NIL()?
>>>> This came about from some old changes from 2012 to a crash version 4.0 for
>>>> other work. I had not thought of /dev/xenmem at the time, and just copied
>>>> the condition from above. I see no reason not to drop "bt->task ==
>>>> tt->this_task". Will check that the right actions are still done.
>>>>> In any case, since the ultimate change is only interested in
>>>>> REMOTE_NIL(),
>>>>> can you make it all less of an eyesore by encapulating its if case
>>>>> something like:
>>>>>
>>>>> else if (SADUMP_DUMPFILE())
>>>>> get_sadump_regs(bt, &eip, &esp);
>>>>> + else if (REMOTE_NIL()) {
>>>>> + if (!((bt->task == tt->this_task) ||
>>>>> is_task_active(bt->task)) ||
>>>>> + !get_remote_regs(bt, &eip, &esp))
>>>>> + machdep->get_stack_frame(bt, &eip, &esp);
>>>>> } else
>>>>> machdep->get_stack_frame(bt, &eip, &esp);
>>>>>
>>>>> Note above, for consistency's sake with the rest of the crash utility,
>>>>> can you make get_remote_regs() return non-zero when it successfully gets
>>>>> the registers, and zero if it fails?
>>>> Sure.
>>>>> And lastly, can you fix these?
>>>>>
>>>>> $ make warn
>>>>> ...
>>>>> remote.c: In function ‘validate_phys_base’:
>>>>> remote.c:2302:25: warning: format ‘%llx’ expects argument of type
>>>>> ‘long long unsigned int’, but argument 3 has type ‘physaddr_t’
>>>>> [-Wformat]
>>>>> remote.c:2302:25: warning: format ‘%llx’ expects argument of type
>>>>> ‘long long unsigned int’, but argument 4 has type ‘physaddr_t’
>>>>> [-Wformat]
>>>>> remote.c:2302:25: warning: format ‘%llx’ expects argument of type
>>>>> ‘long long unsigned int’, but argument 5 has type ‘physaddr_t’
>>>>> [-Wformat]
>>>>> remote.c:2302:25: warning: format ‘%llx’ expects argument of type
>>>>> ‘long long unsigned int’, but argument 6 has type ‘physaddr_t’
>>>>> [-Wformat]
>>>>> remote.c: In function ‘remote_vtop’:
>>>>> remote.c:2361:2: warning: format ‘%llx’ expects argument of type
>>>>> ‘long long unsigned int’, but argument 4 has type ‘physaddr_t’
>>>>> [-Wformat]
>>>>> remote.c:2367:4: warning: format ‘%llx’ expects argument of type
>>>>> ‘long long unsigned int’, but argument 4 has type ‘physaddr_t’
>>>>> [-Wformat]
>>>>> remote.c:2352:18: warning: variable ‘p3’ set but not used
>>>>> [-Wunused-but-set-variable]
>>>>> remote.c:2352:13: warning: variable ‘p2’ set but not used
>>>>> [-Wunused-but-set-variable]
>>>>> remote.c:2352:8: warning: variable ‘p1’ set but not used
>>>>> [-Wunused-but-set-variable]
>>>>> remote.c: In function ‘get_remote_regs’:
>>>>> remote.c:2392:33: warning: unused variable ‘p6’ [-Wunused-variable]
>>>>> remote.c:2392:28: warning: variable ‘p5’ set but not used
>>>>> [-Wunused-but-set-variable]
>>>>> remote.c:2392:18: warning: variable ‘p3’ set but not used
>>>>> [-Wunused-but-set-variable]
>>>>> remote.c:2392:8: warning: variable ‘p1’ set but not used
>>>>> [-Wunused-but-set-variable]
>>>>> remote.c: In function ‘get_remote_cr3’:
>>>>> remote.c:2449:13: warning: variable ‘p2’ set but not used
>>>>> [-Wunused-but-set-variable]
>>>>> remote.c:2449:8: warning: variable ‘p1’ set but not used
>>>>> [-Wunused-but-set-variable]
>>>>> ...
>>>> Yes.
>>>>
>>>>> And I'm not interested in appending these to the 4 files that you
>>>>> changed:
>>>>>
>>>>> +/*
>>>>> + * Specify Emacs local variables so the formating
>>>>> + * of the code stays the same.
>>>>> + *
>>>>> + * Local variables:
>>>>> + * mode: C
>>>>> + * c-set-style: "BSD"
>>>>> + * c-basic-offset: 8
>>>>> + * indent-tabs-mode: t
>>>>> + * End:
>>>>> + */
>>>>>
>>>>> Thanks,
>>>>> Dave
>>>> Ok, that is 2 votes to drop this, so I will.
>>>> -Don Slutz
>>>>
>>
More information about the Crash-utility
mailing list