Re: [PATCH 2/2] xl: disable events earlier for shutdown event

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Feb 02, 2017 at 03:52:41PM +0000, Ian Jackson wrote:
> Wei Liu writes ("[PATCH 2/2] xl: disable events earlier for shutdown event"):
> > We need to disable event machinery when the guest shuts down.  It
> > doesn't really matter where we disable it as long as it is within the
> > branch for shutdown event.
> 
> I don't think this is necessary.  My intent was that libxl_ctx_free
> would automatically disable all event generation, and that it is
> therefore not necessary to call _disable.
> 
> This isn't documented particularly clearly, but libxl_event.h says:
> 
>   * An evgen is associated with the libxl_ctx used for its creation.
>   * After libxl_ctx_free, all corresponding evgen handles become
>   * invalid and must no longer be passed to evdisable.
> 
> This implies it's legal to call libxl_ctx_free with some events
> enabled.
> 

That's right. But this is not saying there is bug in the event
machinery.

> But I think I don't really understand what the original bug is.
> 

The original bug is that:

1. boot a guest with no disks, so diskws is NULL, num_disks == 0
2. some removable disks are added, and domain config updated
3. guest reboots, xl gets shutdown event
4. handle_domain_death gets the latest d_config, num_disks != 0 now
5. try to disable disk eject events with evdisable_disk_ejects -> BOOM

So basically 5 needs to happen before 4. Moving that snippet seems to do
the trick, and then freeing diskws + setting it to NULL makes the
reboot path automatically re-register diskws.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel