On Sat, Oct 29, 2016 at 12:35:38AM +0800, Xiao Guangrong wrote:
> The buffer is used to save the FIT info for all the presented nvdimm
> devices which is updated after the nvdimm device is plugged or
> unplugged. In the later patch, it will be used to construct NVDIMM
> ACPI _FIT method which reflects the presented nvdimm devices after
> nvdimm hotplug
>
> As FIT buffer can not completely mapped into guest address space,
> OSPM will exit to QEMU multiple times, however, there is the race
> condition - FIT may be changed during these multiple exits, so that
> some rules are introduced:
> 1) the user should hold the @lock to access the buffer and
I don't understand the purpose of the QEMUMutex lock. Don't all threads
calling nvdimm/acpi code hold the QEMU global mutex (main loop and vcpu
threads)?
> -static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
> +static void nvdimm_init_fit_buffer(NvdimmFitBuffer *fit_buf)
> +{
> + qemu_mutex_init(&fit_buf->lock);
> + fit_buf->fit = g_array_new(false, true /* clear */, 1);
Is it possible to call nvdimm_build_device_structure() here? That way
we don't duplicate the g_array_new() details.
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 5783442..d835e62 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -945,10 +945,21 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
> goto child_realize_fail;
> }
> }
> +
> if (dev->hotplugged) {
> device_reset(dev);
> }
> dev->pending_deleted_event = false;
> + dev->realized = value;
> +
> + if (hotplug_ctrl) {
> + hotplug_handler_post_plug(hotplug_ctrl, dev, &local_err);
> + }
> +
> + if (local_err != NULL) {
> + dev->realized = value;
dev->realized = value was already set above.
In order to preserve semantics you would need a bool old_value =
dev->realized which you can restore in post_realize_fail. QEMU current
does not assign dev->realized = value when there is a failure!
Attachment:
signature.asc
Description: PGP signature
