[Crash-utility] faulty use of 'set' command when running against the xen hypervisor

[Itsuro ODA]

Hi Dave,

Your patches looks good for me.
Thank you for avoiding pointless options for the hypervisor
analysis.

Thanks.
Itsuro Oda

Dave Anderson said:
>
> I suppose it should also require an error-catcher if you actually try to
> set some bogus "context" value:
>
> @@ -2144,6 +2153,8 @@ cmd_set(void)
>                                          "on" : "off");
>                          return;
>
> +               } else if (XEN_HYPER_MODE()) {
> +                       error(FATAL, "invalid argument for the Xen
> hypervisor\n");
>                  } else if (runtime) {
>                          ulong pid, task;
>
> to change the behavior from this:
>
>    crash> set 1
>    set: invalid task or pid value: 1
>    crash
>
> to this:
>
>    crash> set 1
>    set: invalid argument for the Xen hypervisor
>    crash>
>
> Dave
>
>
>
> Dave Anderson wrote:
>>
>> Cai Quan bumped into another problem when running against the xen
>> hypervisor,
>> where entering the "set" command alone with no arguments generates a
>> SIGSEGV.
>>
>> I also note that "set -c #" and "set -p" options make no sense either:
>>
>>   # crash --xen_phys_start 3ee00000 xen-syms-2.6.18-92.el5 vmcore
>>
>>   crash 4.0-7.2
>>   Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008  Red Hat, Inc.
>>   Copyright (C) 2004, 2005, 2006  IBM Corporation
>>   Copyright (C) 1999-2006  Hewlett-Packard Co
>>   Copyright (C) 2005, 2006  Fujitsu Limited
>>   Copyright (C) 2006, 2007  VA Linux Systems Japan K.K.
>>   Copyright (C) 2005  NEC Corporation
>>   Copyright (C) 1999, 2002, 2007  Silicon Graphics, Inc.
>>   Copyright (C) 1999, 2000, 2001, 2002  Mission Critical Linux, Inc.
>>   This program is free software, covered by the GNU General Public
>> License,
>>   and you are welcome to change it and/or distribute copies of it under
>>   certain conditions.  Enter "help copying" to see the conditions.
>>   This program has absolutely no warranty.  Enter "help warranty" for
>> details.
>>
>>   GNU gdb 6.1
>>   Copyright 2004 Free Software Foundation, Inc.
>>   GDB is free software, covered by the GNU General Public License, and
>> you are
>>   welcome to change it and/or distribute copies of it under certain
>> conditions.
>>   Type "show copying" to see the conditions.
>>   There is absolutely no warranty for GDB.  Type "show warranty" for
>> details.
>>   This GDB was configured as "x86_64-unknown-linux-gnu"...
>>
>>      KERNEL: xen-syms-2.6.18-92.el5
>>   DEBUGINFO: ./xen-syms-2.6.18-92.el5.debug
>>    DUMPFILE: vmcore
>>        CPUS: 2
>>     DOMAINS: 4
>>      UPTIME: 00:34:18
>>     MACHINE: Intel(R) Pentium(R) 4 CPU 3.40GHz  (3400 Mhz)
>>      MEMORY: 1 GB
>>     PCPU-ID: 1
>>        PCPU: ffff83003f05ff28
>>     VCPU-ID: 1
>>        VCPU: ffff83003eef6080  (VCPU_RUNNING)
>>   DOMAIN-ID: 0
>>      DOMAIN: ffff83003eef8080  (DOMAIN_RUNNING)
>>       STATE: CRASH
>>
>>   crash> set -c 0
>>   set: invalid cpu number: system has only 0 cpu
>>   crash> set -p
>>   set: no panic task found!
>>   crash> set
>>   Segmentation fault
>>
>>   #
>>
>> Admittedly it's a nonsensical usages of "set", since there's no concept
>> of
>> a PID "context" in the hypervisor.
>>
>> The attached patch changes the behavior to:
>>
>>   crash> set -c 0
>>   set: -c option not supported on this architecture or kernel
>>   crash> set -p
>>   set: -p option not supported on this architecture or kernel
>>   crash> set
>>   set: requires an option with the Xen hypervisor
>>   crash>
>>
>> Itsura, are you OK with the attached patch?
>>
>> Thanks,
>>   Dave
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> --- crash/tools.c.orig
>> +++ crash/tools.c
>> @@ -1673,6 +1673,9 @@ cmd_set(void)
>>                  switch(c)
>>  		{
>>  		case 'c':
>> +			if (XEN_HYPER_MODE())
>> +				option_not_supported(c);
>> +
>>  			if (!runtime) {
>>  				error(INFO,
>>  				    "cpu setting not allowed from .%src\n",
>> @@ -1689,6 +1692,9 @@ cmd_set(void)
>>  			return;
>>
>>  		case 'p':
>> +			if (XEN_HYPER_MODE())
>> +				option_not_supported(c);
>> +
>>  			if (!runtime)
>>  				return;
>>
>> @@ -1723,7 +1729,10 @@ cmd_set(void)
>>  	}
>>
>>  	if (!args[optind]) {
>> -		if (runtime)
>> +		if (XEN_HYPER_MODE())
>> +			error(INFO,
>> +			    "requires an option with the Xen hypervisor\n");
>> +		else if (runtime)
>>  			show_context(CURRENT_CONTEXT());
>>  		return;
>>  	}
>>
>>
>> ------------------------------------------------------------------------
>>
>> --
>> Crash-utility mailing list
>> Crash-utility at redhat.com
>> https://www.redhat.com/mailman/listinfo/crash-utility
>
>


-- 
Itsuro ODA <oda at valinux.co.jp>