[PATCH v2 0/5] Hypervisor CPU Baseline Cleanups and Fixes

Collin Walling posted 5 patches 3 years, 7 months ago
Test syntax-check failed
Failed in applying to current master (apply log)
src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 8 deletions(-)
[PATCH v2 0/5] Hypervisor CPU Baseline Cleanups and Fixes
Posted by Collin Walling 3 years, 7 months ago
The following patches provide some TLC to the hypervisor CPU baseline
handler within the qemu_driver code.

#1 checks for the cpu-model-expansion capability before
executing the baseline handler since it is used for feature expansion.

#2 fixes a styling where a < 0 condition was missing from one of the 
if (function()) lines for consistency's sake.

#3 will check if the cpu definition(s) are valid and contain a model
name.

#4 checks the cpu definition(s) model names against the model names
known by the hypervisor. This patch must come before #5.

#5 will allow the baseline command to be ran with a single cpu
definition, whereas before the command would simply fail with an
unhelpful error message. A CPU model expansion will be performed in
this case, which will produce the same result as if the model were
actually baselined.

Note: without patch #4, #5 can result in a segfault in the case where
a single CPU model is provided and the model is not recognized by the
hypervisor. This is because cpu model expansion will return 0 and the
result will be NULL. 

Since the QMP response returns "GenericError" in the case of a missing
CPU model, or if the command is not supported (and perhaps for other
reasons I am unsure of -- the response does not explicitly detail that
the CPU model provided was erroneous), we cannot rely on this 
response always meaning there was a missing CPU model.

So, to be safe-and-sure, the CPU model is checked against the list of
CPU models known to the hypervisor prior to baselining / expanding
(which were retrieved at some point previously during libvirt init).

Collin Walling (5):
  qemu: check for model-expansion cap before baselining
  qemu: fix one instance of rc check styling in baseline
  qemu: report error if missing model name when baselining
  qemu: check if cpu model is supported before baselining
  qemu: fix error message when baselining with a single cpu

 src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 8 deletions(-)

-- 
2.26.2

Re: [PATCH v2 0/5] Hypervisor CPU Baseline Cleanups and Fixes
Posted by Collin Walling 3 years, 6 months ago
Polite ping. Have there been relevant updates elsewhere that I might've
missed? Thanks!


On 9/24/20 8:22 PM, Collin Walling wrote:
> The following patches provide some TLC to the hypervisor CPU baseline
> handler within the qemu_driver code.
> 
> #1 checks for the cpu-model-expansion capability before
> executing the baseline handler since it is used for feature expansion.
> 
> #2 fixes a styling where a < 0 condition was missing from one of the 
> if (function()) lines for consistency's sake.
> 
> #3 will check if the cpu definition(s) are valid and contain a model
> name.
> 
> #4 checks the cpu definition(s) model names against the model names
> known by the hypervisor. This patch must come before #5.
> 
> #5 will allow the baseline command to be ran with a single cpu
> definition, whereas before the command would simply fail with an
> unhelpful error message. A CPU model expansion will be performed in
> this case, which will produce the same result as if the model were
> actually baselined.
> 
> Note: without patch #4, #5 can result in a segfault in the case where
> a single CPU model is provided and the model is not recognized by the
> hypervisor. This is because cpu model expansion will return 0 and the
> result will be NULL. 
> 
> Since the QMP response returns "GenericError" in the case of a missing
> CPU model, or if the command is not supported (and perhaps for other
> reasons I am unsure of -- the response does not explicitly detail that
> the CPU model provided was erroneous), we cannot rely on this 
> response always meaning there was a missing CPU model.
> 
> So, to be safe-and-sure, the CPU model is checked against the list of
> CPU models known to the hypervisor prior to baselining / expanding
> (which were retrieved at some point previously during libvirt init).
> 
> Collin Walling (5):
>   qemu: check for model-expansion cap before baselining
>   qemu: fix one instance of rc check styling in baseline
>   qemu: report error if missing model name when baselining
>   qemu: check if cpu model is supported before baselining
>   qemu: fix error message when baselining with a single cpu
> 
>  src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 8 deletions(-)
> 


-- 
Regards,
Collin

Stay safe and stay healthy

Re: [PATCH v2 0/5] Hypervisor CPU Baseline Cleanups and Fixes
Posted by Jiri Denemark 3 years, 5 months ago
On Mon, Oct 26, 2020 at 12:12:40 -0400, Collin Walling wrote:
> Polite ping. Have there been relevant updates elsewhere that I might've
> missed? Thanks!

Oops, sorry about the delay. I replaced the for loop with
virDomainCapsCPUModelsGet in patch 4/5 as mentioned in my reply to it
and pushed the series to avoid further delays.

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Re: [PATCH v2 0/5] Hypervisor CPU Baseline Cleanups and Fixes
Posted by Collin Walling 3 years, 5 months ago
On 11/24/20 3:05 PM, Jiri Denemark wrote:
> On Mon, Oct 26, 2020 at 12:12:40 -0400, Collin Walling wrote:
>> Polite ping. Have there been relevant updates elsewhere that I might've
>> missed? Thanks!
> 
> Oops, sorry about the delay. I replaced the for loop with
> virDomainCapsCPUModelsGet in patch 4/5 as mentioned in my reply to it
> and pushed the series to avoid further delays.
> 
> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
> 

No worries at all. Thanks for taking the time to review and push the
patches :)

Enjoy the holidays!

-- 
Regards,
Collin

Stay safe and stay healthy