Re: [Xen-devel] [PATCH] tools/xl: refuse to set number of vcpus to 0 via xl vcpu-set

[Juergen Gross]

On 03/09/18 14:26, Jan Beulich wrote:
 On 03.09.18 at 13:59,  wrote:
>> --- a/tools/xl/xl_vcpu.c
>> +++ b/tools/xl/xl_vcpu.c
>> @@ -341,6 +341,10 @@ static int vcpuset(uint32_t domid, const char* 
>> nr_vcpus, int check_host)
>>  fprintf(stderr, "Error: Invalid argument.\n");
>>  return 1;
>>  }
>> +if (nr_vcpus == 0) {
>> +fprintf(stderr, "Error: Setting number of vcpus to 0 isn't 
>> allowed.\n");
>> +return 1;
>> +}
> 
> This message is liable to be confusing when the string passed in
> represents a non-zero number which, when converted to
> unsigned int, yields zero. I think more thorough input checking is
> needed here. main_vcpupin(), for example, deliberately uses a
> type wider than seemingly necessary, to avoid such an issue.
> 
> I also wonder whether rejecting zero here is really the job of the
> frontend, rather than libxl.

Yes, this would be better.

As the number ov vcpus is determined by counting the bits in the
cpumap, and this value is an int, I guess we should limit the upper
bound to INT_MAX.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/xl: refuse to set number of vcpus to 0 via xl vcpu-set

[Jan Beulich]

>>> On 03.09.18 at 13:59,  wrote:
> --- a/tools/xl/xl_vcpu.c
> +++ b/tools/xl/xl_vcpu.c
> @@ -341,6 +341,10 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, 
> int check_host)
>  fprintf(stderr, "Error: Invalid argument.\n");
>  return 1;
>  }
> +if (nr_vcpus == 0) {
> +fprintf(stderr, "Error: Setting number of vcpus to 0 isn't 
> allowed.\n");
> +return 1;
> +}

This message is liable to be confusing when the string passed in
represents a non-zero number which, when converted to
unsigned int, yields zero. I think more thorough input checking is
needed here. main_vcpupin(), for example, deliberately uses a
type wider than seemingly necessary, to avoid such an issue.

I also wonder whether rejecting zero here is really the job of the
frontend, rather than libxl.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH] tools/xl: refuse to set number of vcpus to 0 via xl vcpu-set

[Juergen Gross]

Trying to set the number of vcpus of a domain to 0 isn't refused.
We should not allow that.

Signed-off-by: Juergen Gross 
---
 tools/xl/xl_vcpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tools/xl/xl_vcpu.c b/tools/xl/xl_vcpu.c
index aef486864c..45ac01beda 100644
--- a/tools/xl/xl_vcpu.c
+++ b/tools/xl/xl_vcpu.c
@@ -341,6 +341,10 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, 
int check_host)
 fprintf(stderr, "Error: Invalid argument.\n");
 return 1;
 }
+if (nr_vcpus == 0) {
+fprintf(stderr, "Error: Setting number of vcpus to 0 isn't 
allowed.\n");
+return 1;
+}
 
 /*
  * Maximum amount of vCPUS the guest is allowed to set is limited
-- 
2.16.4


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel