[PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[Heiko Carstens]

So, how about the patch below in order to fix the issue Faisan
brought up for lspcu?

>From 0f0f80b71e90b6ab58d9f6009b8356bb42235916 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Wed, 8 Aug 2012 10:52:47 +0200
Subject: [PATCH] lscpu: limit options --all, --online, --offline to parsable
 and extended output

Passing the --all, --online or --offline options for the output summary
doesn't make much sense. It should be limited to the two list output options.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 sys-utils/lscpu.1 |    9 ++++++---
 sys-utils/lscpu.c |   12 ++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/sys-utils/lscpu.1 b/sys-utils/lscpu.1
index 41dae83..f7e34b9 100644
--- a/sys-utils/lscpu.1
+++ b/sys-utils/lscpu.1
@@ -81,13 +81,16 @@ and hypervisor support CPU polarization.
 .SH OPTIONS
 .TP
 .BR \-a , " \-\-all"
-Include online and offline CPUs in the output (default for -e).
+Include lines for online and offline CPUs in the output (default for -e). This
+option may only specified together with option -e or -p.
 .TP
 .BR \-b , " \-\-online"
-Limit the output to online CPUs (default for -p).
+Limit the output to online CPUs (default for -p). This option may only
+be specified together with option -e or -p.
 .TP
 .BR \-c , " \-\-offline"
-Limit the output to offline CPUs.
+Limit the output to offline CPUs. This option may only be specified together
+with option -e or -p.
 .TP
 .BR \-e , " \-\-extended " \fI[=list]\fP
 Display the CPU information in human readable format.
diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
index b93998a..25a0273 100644
--- a/sys-utils/lscpu.c
+++ b/sys-utils/lscpu.c
@@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
 	struct lscpu_desc _desc = { .flags = 0 }, *desc = &_desc;
 	int c, i;
 	int columns[ARRAY_SIZE(coldescs)], ncolumns = 0;
+	int cpu_modifier_specified = 0;
 
 	static const struct option longopts[] = {
 		{ "all",        no_argument,       0, 'a' },
@@ -1276,12 +1277,15 @@ int main(int argc, char *argv[])
 		switch (c) {
 		case 'a':
 			mod->online = mod->offline = 1;
+			cpu_modifier_specified = 1;
 			break;
 		case 'b':
 			mod->online = 1;
+			cpu_modifier_specified = 1;
 			break;
 		case 'c':
 			mod->offline = 1;
+			cpu_modifier_specified = 1;
 			break;
 		case 'h':
 			usage(stdout);
@@ -1316,6 +1320,14 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	if (cpu_modifier_specified && mod->mode == OUTPUT_SUMMARY) {
+		fprintf(stderr,
+			_("%s: options --all, --online and --offline may only "
+			  "be used with options --extended or --parsable.\n"),
+			program_invocation_short_name);
+		return EXIT_FAILURE;
+	}
+
 	if (argc != optind)
 		usage(stderr);
 
-- 
1.7.9.5

Re: [PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[faizan husain]

On Wednesday 08 August 2012 02:29 PM, Heiko Carstens wrote:
> So, how about the patch below in order to fix the issue Faisan
> brought up for lspcu?
>
>  From 0f0f80b71e90b6ab58d9f6009b8356bb42235916 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> Date: Wed, 8 Aug 2012 10:52:47 +0200
> Subject: [PATCH] lscpu: limit options --all, --online, --offline to parsable
>   and extended output
>
> Passing the --all, --online or --offline options for the output summary
> doesn't make much sense. It should be limited to the two list output options.
>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>   sys-utils/lscpu.1 |    9 ++++++---
>   sys-utils/lscpu.c |   12 ++++++++++++
>   2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/sys-utils/lscpu.1 b/sys-utils/lscpu.1
> index 41dae83..f7e34b9 100644
> --- a/sys-utils/lscpu.1
> +++ b/sys-utils/lscpu.1
> @@ -81,13 +81,16 @@ and hypervisor support CPU polarization.
>   .SH OPTIONS
>   .TP
>   .BR \-a , " \-\-all"
> -Include online and offline CPUs in the output (default for -e).
> +Include lines for online and offline CPUs in the output (default for -e). This
> +option may only specified together with option -e or -p.
>   .TP
>   .BR \-b , " \-\-online"
> -Limit the output to online CPUs (default for -p).
> +Limit the output to online CPUs (default for -p). This option may only
> +be specified together with option -e or -p.
>   .TP
>   .BR \-c , " \-\-offline"
> -Limit the output to offline CPUs.
> +Limit the output to offline CPUs. This option may only be specified together
> +with option -e or -p.
>   .TP
>   .BR \-e , " \-\-extended " \fI[=list]\fP
>   Display the CPU information in human readable format.
> diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
> index b93998a..25a0273 100644
> --- a/sys-utils/lscpu.c
> +++ b/sys-utils/lscpu.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>   	struct lscpu_desc _desc = { .flags = 0 }, *desc = &_desc;
>   	int c, i;
>   	int columns[ARRAY_SIZE(coldescs)], ncolumns = 0;
> +	int cpu_modifier_specified = 0;
>
>   	static const struct option longopts[] = {
>   		{ "all",        no_argument,       0, 'a' },
> @@ -1276,12 +1277,15 @@ int main(int argc, char *argv[])
>   		switch (c) {
>   		case 'a':
>   			mod->online = mod->offline = 1;
> +			cpu_modifier_specified = 1;
>   			break;
>   		case 'b':
>   			mod->online = 1;
> +			cpu_modifier_specified = 1;
>   			break;
>   		case 'c':
>   			mod->offline = 1;
> +			cpu_modifier_specified = 1;
>   			break;
>   		case 'h':
>   			usage(stdout);
> @@ -1316,6 +1320,14 @@ int main(int argc, char *argv[])
>   		}
>   	}
>
> +	if (cpu_modifier_specified && mod->mode == OUTPUT_SUMMARY) {
> +		fprintf(stderr,
> +			_("%s: options --all, --online and --offline may only "
> +			  "be used with options --extended or --parsable.\n"),
> +			program_invocation_short_name);
> +		return EXIT_FAILURE;
> +	}
> +
>   	if (argc != optind)
>   		usage(stderr);
>
Looks good to me.


Thanks
Faizan

Re: [PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[faizan husain]

On Wednesday 08 August 2012 02:29 PM, Heiko Carstens wrote:
> So, how about the patch below in order to fix the issue Faisan
> brought up for lspcu?
>
>  From 0f0f80b71e90b6ab58d9f6009b8356bb42235916 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> Date: Wed, 8 Aug 2012 10:52:47 +0200
> Subject: [PATCH] lscpu: limit options --all, --online, --offline to parsable
>   and extended output
>
> Passing the --all, --online or --offline options for the output summary
> doesn't make much sense. It should be limited to the two list output options.
>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>   sys-utils/lscpu.1 |    9 ++++++---
>   sys-utils/lscpu.c |   12 ++++++++++++
>   2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/sys-utils/lscpu.1 b/sys-utils/lscpu.1
> index 41dae83..f7e34b9 100644
> --- a/sys-utils/lscpu.1
> +++ b/sys-utils/lscpu.1
> @@ -81,13 +81,16 @@ and hypervisor support CPU polarization.
>   .SH OPTIONS
>   .TP
>   .BR \-a , " \-\-all"
> -Include online and offline CPUs in the output (default for -e).
> +Include lines for online and offline CPUs in the output (default for -e). This
> +option may only specified together with option -e or -p.
>   .TP
>   .BR \-b , " \-\-online"
> -Limit the output to online CPUs (default for -p).
> +Limit the output to online CPUs (default for -p). This option may only
> +be specified together with option -e or -p.
>   .TP
>   .BR \-c , " \-\-offline"
> -Limit the output to offline CPUs.
> +Limit the output to offline CPUs. This option may only be specified together
> +with option -e or -p.
>   .TP
>   .BR \-e , " \-\-extended " \fI[=list]\fP
>   Display the CPU information in human readable format.
> diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
> index b93998a..25a0273 100644
> --- a/sys-utils/lscpu.c
> +++ b/sys-utils/lscpu.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>   	struct lscpu_desc _desc = { .flags = 0 }, *desc = &_desc;
>   	int c, i;
>   	int columns[ARRAY_SIZE(coldescs)], ncolumns = 0;
> +	int cpu_modifier_specified = 0;
>
>   	static const struct option longopts[] = {
>   		{ "all",        no_argument,       0, 'a' },
> @@ -1276,12 +1277,15 @@ int main(int argc, char *argv[])
>   		switch (c) {
>   		case 'a':
>   			mod->online = mod->offline = 1;
> +			cpu_modifier_specified = 1;
>   			break;
>   		case 'b':
>   			mod->online = 1;
> +			cpu_modifier_specified = 1;
>   			break;
>   		case 'c':
>   			mod->offline = 1;
> +			cpu_modifier_specified = 1;
>   			break;
>   		case 'h':
>   			usage(stdout);
> @@ -1316,6 +1320,14 @@ int main(int argc, char *argv[])
>   		}
>   	}
>
> +	if (cpu_modifier_specified && mod->mode == OUTPUT_SUMMARY) {
> +		fprintf(stderr,
> +			_("%s: options --all, --online and --offline may only "
> +			  "be used with options --extended or --parsable.\n"),
> +			program_invocation_short_name);
> +		return EXIT_FAILURE;
> +	}
> +
>   	if (argc != optind)
>   		usage(stderr);
>
but...

don't you think we are missing on other column information

  [root@mx3650m2 ~]# lscpu -a
Architecture:          x86_64
CPU op-mode(s):        32-bit, 64-bit
Byte Order:            Little Endian
CPU(s):                16
On-line CPU(s) list:   0-5,7-13,15
Off-line CPU(s) list:  6,14
Thread(s) per core:    2
Core(s) per socket:    3
Socket(s):             2
NUMA node(s):          2
Vendor ID:             GenuineIntel
CPU family:            6
Model:                 26
Stepping:              5
CPU MHz:               1596.000
BogoMIPS:              5865.80
Virtualization:        VT-x
L1d cache:             32K
L1i cache:             32K
L2 cache:              256K
L3 cache:              8192K
NUMA node0 CPU(s):     0-3,8-11
NUMA node1 CPU(s):     4,5,7,12,13,15


Thanks
Faizan

Re: [PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[Heiko Carstens]

On Thu, Aug 09, 2012 at 11:48:57AM +0530, faizan husain wrote:
> On Wednesday 08 August 2012 02:29 PM, Heiko Carstens wrote:
> >So, how about the patch below in order to fix the issue Faisan
> >brought up for lspcu?
> >
> > From 0f0f80b71e90b6ab58d9f6009b8356bb42235916 Mon Sep 17 00:00:00 2001
> >From: Heiko Carstens <heiko.carstens@de.ibm.com>
> >Date: Wed, 8 Aug 2012 10:52:47 +0200
> >Subject: [PATCH] lscpu: limit options --all, --online, --offline to parsable
> >  and extended output
> >
> >Passing the --all, --online or --offline options for the output summary
> >doesn't make much sense. It should be limited to the two list output options.

[...]

> >
> >+	if (cpu_modifier_specified && mod->mode == OUTPUT_SUMMARY) {
> >+		fprintf(stderr,
> >+			_("%s: options --all, --online and --offline may only "
> >+			  "be used with options --extended or --parsable.\n"),
> >+			program_invocation_short_name);
> >+		return EXIT_FAILURE;
> >+	}
> >+
> >  	if (argc != optind)
> >  		usage(stderr);
> >
> but...
> 
> don't you think we are missing on other column information
> 
>  [root@mx3650m2 ~]# lscpu -a

That wouldn't work with the patch above applied (and without the patch
applied there wouldn't be any difference in the output, regardless if you
would specify "-a" or not).

So.. I don't understand what your concern is.

Besides that personally I would prefer to _only_ update the man page and
don't change the code at all.

> Architecture:          x86_64
> CPU op-mode(s):        32-bit, 64-bit
> Byte Order:            Little Endian
> CPU(s):                16
> On-line CPU(s) list:   0-5,7-13,15
> Off-line CPU(s) list:  6,14
> Thread(s) per core:    2
> Core(s) per socket:    3
> Socket(s):             2
> NUMA node(s):          2
> Vendor ID:             GenuineIntel
> CPU family:            6
> Model:                 26
> Stepping:              5
> CPU MHz:               1596.000
> BogoMIPS:              5865.80
> Virtualization:        VT-x
> L1d cache:             32K
> L1i cache:             32K
> L2 cache:              256K
> L3 cache:              8192K
> NUMA node0 CPU(s):     0-3,8-11
> NUMA node1 CPU(s):     4,5,7,12,13,15

Re: [PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[faizan husain]

On Thursday 09 August 2012 12:27 PM, Heiko Carstens wrote:
> On Thu, Aug 09, 2012 at 11:48:57AM +0530, faizan husain wrote:
>> On Wednesday 08 August 2012 02:29 PM, Heiko Carstens wrote:
>>> So, how about the patch below in order to fix the issue Faisan
>>> brought up for lspcu?
>>>
>>>  From 0f0f80b71e90b6ab58d9f6009b8356bb42235916 Mon Sep 17 00:00:00 2001
>>> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>>> Date: Wed, 8 Aug 2012 10:52:47 +0200
>>> Subject: [PATCH] lscpu: limit options --all, --online, --offline to parsable
>>>   and extended output
>>>
>>> Passing the --all, --online or --offline options for the output summary
>>> doesn't make much sense. It should be limited to the two list output options.
> [...]
>
>>> +	if (cpu_modifier_specified && mod->mode == OUTPUT_SUMMARY) {
>>> +		fprintf(stderr,
>>> +			_("%s: options --all, --online and --offline may only "
>>> +			  "be used with options --extended or --parsable.\n"),
>>> +			program_invocation_short_name);
>>> +		return EXIT_FAILURE;
>>> +	}
>>> +
>>>   	if (argc != optind)
>>>   		usage(stderr);
>>>
>> but...
>>
>> don't you think we are missing on other column information
>>
>>   [root@mx3650m2 ~]# lscpu -a
> That wouldn't work with the patch above applied (and without the patch
> applied there wouldn't be any difference in the output, regardless if you
> would specify "-a" or not).
>
> So.. I don't understand what your concern is.

My concern here is, earlier we used to print all the column information with -a, -b(limit online and other column info), -c(limit offline and other column info.

but with your patch we are certainly going to miss other information?

here is output from you patch:

[root@mx3650m2 ~]# lscpu -a
lscpu: options --all, --online and --offline may only be used with options --extended or --parsable.

[root@mx3650m2 util-linux-2.21.1]# lscpu -ae

CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE

0   0    0      0    0:0:0:0       yes

1   0    0      1    1:1:1:0       yes

2   0    0      2    2:2:2:0       yes

3   -    -      -    :::           no

4   1    1      3    3:3:3:1       yes

5   -    -      -    :::           no

6   1    1      4    4:4:4:1       yes

7   -    -      -    :::           no

8   0    0      0    0:0:0:0       yes

9   -    -      -    :::           no

10  0    0      2    2:2:2:0       yes

11  0    0      5    5:5:5:0       yes

12  1    1      3    3:3:3:1       yes

13  1    1      6    6:6:6:1       yes

14  1    1      4    4:4:4:1       yes

15  1    1      7    7:7:7:1       yes

[root@mx3650m2 util-linux-2.21.1]# lscpu -be

CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE

0   0    0      0    0:0:0:0       yes

1   0    0      1    1:1:1:0       yes

2   0    0      2    2:2:2:0       yes

4   1    1      3    3:3:3:1       yes

6   1    1      4    4:4:4:1       yes

8   0    0      0    0:0:0:0       yes

10  0    0      2    2:2:2:0       yes

11  0    0      5    5:5:5:0       yes

12  1    1      3    3:3:3:1       yes

13  1    1      6    6:6:6:1       yes

14  1    1      4    4:4:4:1       yes

15  1    1      7    7:7:7:1       yes

[root@mx3650m2 util-linux-2.21.1]# lscpu -ce

CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE

3   -    -      -    :::           no

5   -    -      -    :::           no

7   -    -      -    :::           no

9   -    -      -    :::           no

[root@mx3650m2 util-linux-2.21.1]#
so only CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE are printed with your patch.
other column like Architecture, CPU op-mode(s), Byte Order, Thread(s) per core, vendors ID, CPU family etc... user are going to miss.

Thanks
Faizan

> Besides that personally I would prefer to _only_ update the man page and
> don't change the code at all.
>
>> Architecture:          x86_64
>> CPU op-mode(s):        32-bit, 64-bit
>> Byte Order:            Little Endian
>> CPU(s):                16
>> On-line CPU(s) list:   0-5,7-13,15
>> Off-line CPU(s) list:  6,14
>> Thread(s) per core:    2
>> Core(s) per socket:    3
>> Socket(s):             2
>> NUMA node(s):          2
>> Vendor ID:             GenuineIntel
>> CPU family:            6
>> Model:                 26
>> Stepping:              5
>> CPU MHz:               1596.000
>> BogoMIPS:              5865.80
>> Virtualization:        VT-x
>> L1d cache:             32K
>> L1i cache:             32K
>> L2 cache:              256K
>> L3 cache:              8192K
>> NUMA node0 CPU(s):     0-3,8-11
>> NUMA node1 CPU(s):     4,5,7,12,13,15

Re: [PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[Heiko Carstens]

On Thu, Aug 09, 2012 at 12:49:02PM +0530, faizan husain wrote:
> On Thursday 09 August 2012 12:27 PM, Heiko Carstens wrote:
> >>  [root@mx3650m2 ~]# lscpu -a
> >That wouldn't work with the patch above applied (and without the patch
> >applied there wouldn't be any difference in the output, regardless if you
> >would specify "-a" or not).
> >
> >So.. I don't understand what your concern is.
> 
> My concern here is, earlier we used to print all the column information with
> -a, -b(limit online and other column info), -c(limit offline and other column info.
> 
> but with your patch we are certainly going to miss other information?
> 
> here is output from you patch:
> 
> [root@mx3650m2 ~]# lscpu -a
> lscpu: options --all, --online and --offline may only be used with options --extended or --parsable.
> 
> [root@mx3650m2 util-linux-2.21.1]# lscpu -ae
> 
> CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE
> 0   0    0      0    0:0:0:0       yes
> 1   0    0      1    1:1:1:0       yes

[...]

> so only CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE are printed with your patch.
> other column like Architecture, CPU op-mode(s), Byte Order, Thread(s) per core,
> vendors ID, CPU family etc... user are going to miss.

lscpu never printed columns for architecture, threads per core etc. for the
extended and parsable output. My patch doesn't make any difference here.
Also it doesn't make any sense to print such static and identical information
for each and every cpu line.

The purpose of the extended and parsable output is to give the user an overview
of the cpu topology and per cpu attributes.
It's purpose is certainly not to give an overview of the whole system.

Re: [PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[faizan husain]

On Thursday 09 August 2012 01:28 PM, Heiko Carstens wrote:
> On Thu, Aug 09, 2012 at 12:49:02PM +0530, faizan husain wrote:
>> On Thursday 09 August 2012 12:27 PM, Heiko Carstens wrote:
>>>>   [root@mx3650m2 ~]# lscpu -a
>>> That wouldn't work with the patch above applied (and without the patch
>>> applied there wouldn't be any difference in the output, regardless if you
>>> would specify "-a" or not).
>>>
>>> So.. I don't understand what your concern is.
>> My concern here is, earlier we used to print all the column information with
>> -a, -b(limit online and other column info), -c(limit offline and other column info.
>>
>> but with your patch we are certainly going to miss other information?
>>
>> here is output from you patch:
>>
>> [root@mx3650m2 ~]# lscpu -a
>> lscpu: options --all, --online and --offline may only be used with options --extended or --parsable.
>>
>> [root@mx3650m2 util-linux-2.21.1]# lscpu -ae
>>
>> CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE
>> 0   0    0      0    0:0:0:0       yes
>> 1   0    0      1    1:1:1:0       yes
> [...]
>
>> so only CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE are printed with your patch.
>> other column like Architecture, CPU op-mode(s), Byte Order, Thread(s) per core,
>> vendors ID, CPU family etc... user are going to miss.
> lscpu never printed columns for architecture, threads per core etc. for the
> extended and parsable output. My patch doesn't make any difference here.
> Also it doesn't make any sense to print such static and identical information
> for each and every cpu line.
>
> The purpose of the extended and parsable output is to give the user an overview
> of the cpu topology and per cpu attributes.
> It's purpose is certainly not to give an overview of the whole system.

Sorry for creating confusion here,that is correct, lscpu never printed columns for architecture, threads per core etc. for the
extended and parsable output.
what i feel is earlier we used to have lot of other CPU Architecture specific information printed (see man page DESCRIPTION) with -a, -b ,-c option
but with your patch as -a, -b and -c are used only with -e and -p option those column information are suppressed.

Thanks

Faizan

Re: [PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[Heiko Carstens]

On Thu, Aug 09, 2012 at 01:57:15PM +0530, faizan husain wrote:
> Sorry for creating confusion here,that is correct, lscpu never printed
> columns for architecture, threads per core etc. for the extended and
> parsable output.
> what i feel is earlier we used to have lot of other CPU Architecture
> specific information printed (see man page DESCRIPTION) with -a, -b ,-c
> option but with your patch as -a, -b and -c are used only with -e and -p
> option those column information are suppressed.

Try "lscpu" without any options.

Re: [PATCH] lscpu: limit options --all, --online, --offline to parsable and extended output

[Karel Zak]

On Wed, Aug 08, 2012 at 10:59:11AM +0200, Heiko Carstens wrote:
>  sys-utils/lscpu.1 |    9 ++++++---
>  sys-utils/lscpu.c |   12 ++++++++++++
>  2 files changed, 18 insertions(+), 3 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com