Re: [PATCH 0/7] Upgrade the remote access to a xen domain.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx)

Thanks,
  Dave


----- 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@xxxxxxxxxxx>
> 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@xxxxxxxxxxx>
> ---
>   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@xxxxxxxxxxx>
> >>>>
> >>>> 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
> >>
> 
> 

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility





[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux