[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] xl: disable events earlier for shutdown event
On Thu, Feb 02, 2017 at 04:05:08PM +0000, Ian Jackson wrote: > Wei Liu writes ("Re: [PATCH 2/2] xl: disable events earlier for shutdown > event"): > > On Thu, Feb 02, 2017 at 03:52:41PM +0000, Ian Jackson wrote: > > > But I think I don't really understand what the original bug is. > > > > The original bug is that: > > Ah. > > > 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. > > Surely the right answer is to make evdisable_disk_ejects tolerate > whatever it finds in diskws (including diskws==NULL). It could be > idempotent too. > No, handling NULL is not enough. It could well be possible that diskws is not NULL but then num_disks grows, thus making evdisable_disk_ejects access out of bound. The other solution is to have a dedicated counter for diskws instead of relying on num_disks. ---8<--- From 43dc119327923ad6237e2d4d7bde5ec1147d9007 Mon Sep 17 00:00:00 2001 From: Wei Liu <wei.liu2@xxxxxxxxxx> Date: Thu, 2 Feb 2017 16:16:53 +0000 Subject: [PATCH] xl: track size of diskws with a dedicated counter The num_disks field can change during guest lifetime. Don't use that as the size of diskws, use a dedicated counter instead. Also free diskws and reset diskws to NULL after disabling events so that it will be automatically re-created when the guest reboots. Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx> --- tools/libxl/xl_cmdimpl.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index b25ac6efa0..358757fd09 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -2811,6 +2811,7 @@ static int create_domain(struct domain_create *dom_info) int ret, rc; libxl_evgen_domain_death *deathw = NULL; libxl_evgen_disk_eject **diskws = NULL; /* one per disk */ + unsigned int num_diskws = 0; void *config_data = 0; int config_len = 0; int restore_fd = -1; @@ -3119,8 +3120,9 @@ start: diskws = xmalloc(sizeof(*diskws) * d_config.num_disks); for (i = 0; i < d_config.num_disks; i++) diskws[i] = NULL; + num_diskws = d_config.num_disks; } - for (i = 0; i < d_config.num_disks; i++) { + for (i = 0; i < num_diskws; i++) { if (d_config.disks[i].removable) { ret = libxl_evenable_disk_eject(ctx, domid, d_config.disks[i].vdev, 0, &diskws[i]); @@ -3157,7 +3159,10 @@ start: libxl_event_free(ctx, event); libxl_evdisable_domain_death(ctx, deathw); deathw = NULL; - evdisable_disk_ejects(diskws, d_config.num_disks); + evdisable_disk_ejects(diskws, num_diskws); + free(diskws); + diskws = NULL; + num_diskws = 0; /* discard any other events which may have been generated */ while (!(ret = libxl_event_check(ctx, &event, LIBXL_EVENTMASK_ALL, 0,0))) { -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |