diff mbox series

[for-6.2,v6,7/7] memory_hotplug.c: send DEVICE_UNPLUG_ERROR in acpi_memory_hotplug_write()

Message ID 20210719200827.1507276-8-danielhb413@gmail.com
State New
Headers show
Series DEVICE_UNPLUG_ERROR QAPI event | expand

Commit Message

Daniel Henrique Barboza July 19, 2021, 8:08 p.m. UTC 
MEM_UNPLUG_ERROR is deprecated since the introduction of
DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of
MEM_UNPLUG_ERROR is pending.

CC: Michael S. Tsirkin <mst@redhat.com>
CC: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/acpi/memory_hotplug.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Michael S. Tsirkin July 19, 2021, 9:13 p.m. UTC | #1 
On Mon, Jul 19, 2021 at 05:08:27PM -0300, Daniel Henrique Barboza wrote:
> MEM_UNPLUG_ERROR is deprecated since the introduction of
> DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of
> MEM_UNPLUG_ERROR is pending.
> 
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Pls merge with rest of series.

> ---
>  hw/acpi/memory_hotplug.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index e37acb0367..a0772fe083 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -8,6 +8,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-acpi.h"
>  #include "qapi/qapi-events-machine.h"
> +#include "qapi/qapi-events-qdev.h"
>  
>  #define MEMORY_SLOTS_NUMBER          "MDNR"
>  #define MEMORY_HOTPLUG_IO_REGION     "HPMR"
> @@ -181,10 +182,19 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>  
>                  trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
>  
> +                /*
> +                 * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR
> +                 * while the deprecation of MEM_UNPLUG_ERROR is
> +                 * pending.
> +                 */
>                  if (dev->id) {
>                      qapi_event_send_mem_unplug_error(dev->id, error_pretty);
>                  }
>  
> +                qapi_event_send_device_unplug_error(!!dev->id, dev->id,
> +                                                    dev->canonical_path,
> +                                                    true, error_pretty);
> +
>                  error_free(local_err);
>                  break;
>              }
> -- 
> 2.31.1
David Gibson July 21, 2021, 6:23 a.m. UTC | #2 
On Mon, Jul 19, 2021 at 05:13:44PM -0400, Michael S. Tsirkin wrote:
> On Mon, Jul 19, 2021 at 05:08:27PM -0300, Daniel Henrique Barboza wrote:
> > MEM_UNPLUG_ERROR is deprecated since the introduction of
> > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of
> > MEM_UNPLUG_ERROR is pending.
> > 
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > CC: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Michael, can I assume I have your ack for 1/7 as well?
 
> Pls merge with rest of series.
> 
> > ---
> >  hw/acpi/memory_hotplug.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> > index e37acb0367..a0772fe083 100644
> > --- a/hw/acpi/memory_hotplug.c
> > +++ b/hw/acpi/memory_hotplug.c
> > @@ -8,6 +8,7 @@
> >  #include "qapi/error.h"
> >  #include "qapi/qapi-events-acpi.h"
> >  #include "qapi/qapi-events-machine.h"
> > +#include "qapi/qapi-events-qdev.h"
> >  
> >  #define MEMORY_SLOTS_NUMBER          "MDNR"
> >  #define MEMORY_HOTPLUG_IO_REGION     "HPMR"
> > @@ -181,10 +182,19 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
> >  
> >                  trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
> >  
> > +                /*
> > +                 * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR
> > +                 * while the deprecation of MEM_UNPLUG_ERROR is
> > +                 * pending.
> > +                 */
> >                  if (dev->id) {
> >                      qapi_event_send_mem_unplug_error(dev->id, error_pretty);
> >                  }
> >  
> > +                qapi_event_send_device_unplug_error(!!dev->id, dev->id,
> > +                                                    dev->canonical_path,
> > +                                                    true, error_pretty);
> > +
> >                  error_free(local_err);
> >                  break;
> >              }
>
Markus Armbruster Aug. 7, 2021, 2:09 p.m. UTC | #3 
Daniel Henrique Barboza <danielhb413@gmail.com> writes:

> MEM_UNPLUG_ERROR is deprecated since the introduction of
> DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of
> MEM_UNPLUG_ERROR is pending.
>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/acpi/memory_hotplug.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index e37acb0367..a0772fe083 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -8,6 +8,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-acpi.h"
>  #include "qapi/qapi-events-machine.h"
> +#include "qapi/qapi-events-qdev.h"
>  
>  #define MEMORY_SLOTS_NUMBER          "MDNR"
>  #define MEMORY_HOTPLUG_IO_REGION     "HPMR"
> @@ -181,10 +182,19 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>  
>                  trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
>  
> +                /*
> +                 * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR
> +                 * while the deprecation of MEM_UNPLUG_ERROR is
> +                 * pending.
> +                 */
>                  if (dev->id) {
>                      qapi_event_send_mem_unplug_error(dev->id, error_pretty);
>                  }
>  
> +                qapi_event_send_device_unplug_error(!!dev->id, dev->id,
> +                                                    dev->canonical_path,
> +                                                    true, error_pretty);
> +
>                  error_free(local_err);
>                  break;
>              }

Unlike the previous patch, "msg" is present even when !dev->id.  Makes
me doubt the previous patch's conditional passing of "msg" some more.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
David Gibson Aug. 9, 2021, 3:41 a.m. UTC | #4 
On Sat, Aug 07, 2021 at 04:09:40PM +0200, Markus Armbruster wrote:
> Daniel Henrique Barboza <danielhb413@gmail.com> writes:
> 
> > MEM_UNPLUG_ERROR is deprecated since the introduction of
> > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of
> > MEM_UNPLUG_ERROR is pending.
> >
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > CC: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> > ---
> >  hw/acpi/memory_hotplug.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> > index e37acb0367..a0772fe083 100644
> > --- a/hw/acpi/memory_hotplug.c
> > +++ b/hw/acpi/memory_hotplug.c
> > @@ -8,6 +8,7 @@
> >  #include "qapi/error.h"
> >  #include "qapi/qapi-events-acpi.h"
> >  #include "qapi/qapi-events-machine.h"
> > +#include "qapi/qapi-events-qdev.h"
> >  
> >  #define MEMORY_SLOTS_NUMBER          "MDNR"
> >  #define MEMORY_HOTPLUG_IO_REGION     "HPMR"
> > @@ -181,10 +182,19 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
> >  
> >                  trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
> >  
> > +                /*
> > +                 * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR
> > +                 * while the deprecation of MEM_UNPLUG_ERROR is
> > +                 * pending.
> > +                 */
> >                  if (dev->id) {
> >                      qapi_event_send_mem_unplug_error(dev->id, error_pretty);
> >                  }
> >  
> > +                qapi_event_send_device_unplug_error(!!dev->id, dev->id,
> > +                                                    dev->canonical_path,
> > +                                                    true, error_pretty);
> > +
> >                  error_free(local_err);
> >                  break;
> >              }
> 
> Unlike the previous patch, "msg" is present even when !dev->id.  Makes
> me doubt the previous patch's conditional passing of "msg" some more.

Daniel, if you can address Markus' comments and send another spin,
I'll replace the draft I have in ppc-for-6.2 with the new version.
diff mbox series

Patch

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index e37acb0367..a0772fe083 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -8,6 +8,7 @@ 
 #include "qapi/error.h"
 #include "qapi/qapi-events-acpi.h"
 #include "qapi/qapi-events-machine.h"
+#include "qapi/qapi-events-qdev.h"
 
 #define MEMORY_SLOTS_NUMBER          "MDNR"
 #define MEMORY_HOTPLUG_IO_REGION     "HPMR"
@@ -181,10 +182,19 @@  static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
 
                 trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
 
+                /*
+                 * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR
+                 * while the deprecation of MEM_UNPLUG_ERROR is
+                 * pending.
+                 */
                 if (dev->id) {
                     qapi_event_send_mem_unplug_error(dev->id, error_pretty);
                 }
 
+                qapi_event_send_device_unplug_error(!!dev->id, dev->id,
+                                                    dev->canonical_path,
+                                                    true, error_pretty);
+
                 error_free(local_err);
                 break;
             }