qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] pc: fail memory hot-plug/unplug with -no-acpi and Q35 machine type
@ 2017-12-22  1:51 Haozhong Zhang
  2017-12-22 15:17 ` Igor Mammedov
  2017-12-22 16:12 ` Paolo Bonzini
  0 siblings, 2 replies; 3+ messages in thread
From: Haozhong Zhang @ 2017-12-22  1:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, mst,
	Haozhong Zhang

When -no-acpi option is used with Q35 machine type, no guest ACPI is
built, but the ACPI device is still created, so only checking the
presence of ACPI device before memory plug/unplug is not enough in
such cases. Check whether ACPI is disabled globally in addition and
fail memory plug/unplug if it's disabled.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 hw/i386/pc.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3fcf318a95..55686bf5d8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1695,9 +1695,14 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
         align = memory_region_get_alignment(mr);
     }
 
-    if (!pcms->acpi_dev) {
+    /*
+     * When -no-acpi is used with Q35 machine type, no ACPI is built,
+     * but pcms->acpi_dev is still created. Check !acpi_enabled in
+     * addition to cover this case.
+     */
+    if (!pcms->acpi_dev || !acpi_enabled) {
         error_setg(&local_err,
-                   "memory hotplug is not enabled: missing acpi device");
+                   "memory hotplug is not enabled: missing acpi device or acpi disabled");
         goto out;
     }
 
@@ -1729,9 +1734,14 @@ static void pc_dimm_unplug_request(HotplugHandler *hotplug_dev,
     Error *local_err = NULL;
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 
-    if (!pcms->acpi_dev) {
+    /*
+     * When -no-acpi is used with Q35 machine type, no ACPI is built,
+     * but pcms->acpi_dev is still created. Check !acpi_enabled in
+     * addition to cover this case.
+     */
+    if (!pcms->acpi_dev || !acpi_enabled) {
         error_setg(&local_err,
-                   "memory hotplug is not enabled: missing acpi device");
+                   "memory hotplug is not enabled: missing acpi device or acpi disabled");
         goto out;
     }
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] pc: fail memory hot-plug/unplug with -no-acpi and Q35 machine type
  2017-12-22  1:51 [Qemu-devel] [PATCH] pc: fail memory hot-plug/unplug with -no-acpi and Q35 machine type Haozhong Zhang
@ 2017-12-22 15:17 ` Igor Mammedov
  2017-12-22 16:12 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Igor Mammedov @ 2017-12-22 15:17 UTC (permalink / raw)
  To: Haozhong Zhang
  Cc: qemu-devel, Paolo Bonzini, mst, Eduardo Habkost,
	Richard Henderson

On Fri, 22 Dec 2017 09:51:20 +0800
Haozhong Zhang <haozhong.zhang@intel.com> wrote:

> When -no-acpi option is used with Q35 machine type, no guest ACPI is
> built, but the ACPI device is still created, so only checking the
> presence of ACPI device before memory plug/unplug is not enough in
> such cases. Check whether ACPI is disabled globally in addition and
> fail memory plug/unplug if it's disabled.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
we do not expose dimms in E820, so there is no point in allowing them
being added (hot or cold-plug) without acpi enabled as guest
won't be able to know where they are located.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/pc.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3fcf318a95..55686bf5d8 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1695,9 +1695,14 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
>          align = memory_region_get_alignment(mr);
>      }
>  
> -    if (!pcms->acpi_dev) {
> +    /*
> +     * When -no-acpi is used with Q35 machine type, no ACPI is built,
> +     * but pcms->acpi_dev is still created. Check !acpi_enabled in
> +     * addition to cover this case.
> +     */
> +    if (!pcms->acpi_dev || !acpi_enabled) {
>          error_setg(&local_err,
> -                   "memory hotplug is not enabled: missing acpi device");
> +                   "memory hotplug is not enabled: missing acpi device or acpi disabled");
>          goto out;
>      }
>  
> @@ -1729,9 +1734,14 @@ static void pc_dimm_unplug_request(HotplugHandler *hotplug_dev,
>      Error *local_err = NULL;
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  
> -    if (!pcms->acpi_dev) {
> +    /*
> +     * When -no-acpi is used with Q35 machine type, no ACPI is built,
> +     * but pcms->acpi_dev is still created. Check !acpi_enabled in
> +     * addition to cover this case.
> +     */
> +    if (!pcms->acpi_dev || !acpi_enabled) {
>          error_setg(&local_err,
> -                   "memory hotplug is not enabled: missing acpi device");
> +                   "memory hotplug is not enabled: missing acpi device or acpi disabled");
>          goto out;
>      }
>  

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] pc: fail memory hot-plug/unplug with -no-acpi and Q35 machine type
  2017-12-22  1:51 [Qemu-devel] [PATCH] pc: fail memory hot-plug/unplug with -no-acpi and Q35 machine type Haozhong Zhang
  2017-12-22 15:17 ` Igor Mammedov
@ 2017-12-22 16:12 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2017-12-22 16:12 UTC (permalink / raw)
  To: Haozhong Zhang, qemu-devel; +Cc: Richard Henderson, Eduardo Habkost, mst

On 22/12/2017 02:51, Haozhong Zhang wrote:
> When -no-acpi option is used with Q35 machine type, no guest ACPI is
> built, but the ACPI device is still created, so only checking the
> presence of ACPI device before memory plug/unplug is not enough in
> such cases. Check whether ACPI is disabled globally in addition and
> fail memory plug/unplug if it's disabled.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
>  hw/i386/pc.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3fcf318a95..55686bf5d8 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1695,9 +1695,14 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
>          align = memory_region_get_alignment(mr);
>      }
>  
> -    if (!pcms->acpi_dev) {
> +    /*
> +     * When -no-acpi is used with Q35 machine type, no ACPI is built,
> +     * but pcms->acpi_dev is still created. Check !acpi_enabled in
> +     * addition to cover this case.
> +     */
> +    if (!pcms->acpi_dev || !acpi_enabled) {
>          error_setg(&local_err,
> -                   "memory hotplug is not enabled: missing acpi device");
> +                   "memory hotplug is not enabled: missing acpi device or acpi disabled");
>          goto out;
>      }
>  
> @@ -1729,9 +1734,14 @@ static void pc_dimm_unplug_request(HotplugHandler *hotplug_dev,
>      Error *local_err = NULL;
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  
> -    if (!pcms->acpi_dev) {
> +    /*
> +     * When -no-acpi is used with Q35 machine type, no ACPI is built,
> +     * but pcms->acpi_dev is still created. Check !acpi_enabled in
> +     * addition to cover this case.
> +     */
> +    if (!pcms->acpi_dev || !acpi_enabled) {
>          error_setg(&local_err,
> -                   "memory hotplug is not enabled: missing acpi device");
> +                   "memory hotplug is not enabled: missing acpi device or acpi disabled");
>          goto out;
>      }
>  
> 

Queued, thanks.

Paolo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-12-22 16:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-22  1:51 [Qemu-devel] [PATCH] pc: fail memory hot-plug/unplug with -no-acpi and Q35 machine type Haozhong Zhang
2017-12-22 15:17 ` Igor Mammedov
2017-12-22 16:12 ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).