[9/9] libxl_disk: Implement missing timeout for libxl_cdrom_insert

Commit Message

Anthony PERARD April 9, 2019, 4:45 p.m. UTC

Since the previous patch "libxl_disk: Use ev_qmp in libxl_cdrom_insert",
there are no kind of timeout anymore, add one back.

Signed-off-by: Anthony PERARD <[e-mail]>
---
 tools/libxl/libxl_disk.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Ian Jackson June 4, 2019, 5:47 p.m. UTC | #1

Anthony PERARD writes ("[PATCH 9/9] libxl_disk: Implement missing timeout for libxl_cdrom_insert"):
> Since the previous patch "libxl_disk: Use ev_qmp in libxl_cdrom_insert",
> there are no kind of timeout anymore, add one back.

Hrm.  The patch itself looks good, so

Acked-by: Ian Jackson <[e-mail]>

But I wonder if this could somehow be placed earlier to preserve
bisectability.

Thanks,
Ian.

Anthony PERARD June 5, 2019, 2:22 p.m. UTC | #2

On Tue, Jun 04, 2019 at 06:47:32PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH 9/9] libxl_disk: Implement missing timeout for libxl_cdrom_insert"):
> > Since the previous patch "libxl_disk: Use ev_qmp in libxl_cdrom_insert",
> > there are no kind of timeout anymore, add one back.
> 
> Hrm.  The patch itself looks good, so
> 
> Acked-by: Ian Jackson <[e-mail]>
> 
> But I wonder if this could somehow be placed earlier to preserve
> bisectability.

I think it would be possible to place the patch right after "libxl_disk:
Cut libxl_cdrom_insert into steps", even though the timeout will never
get a chance to actually fire. (Before "Use ev_qmp ...", everything is
synchronous.)

Ian Jackson June 5, 2019, 2:33 p.m. UTC | #3

Anthony PERARD writes ("Re: [PATCH 9/9] libxl_disk: Implement missing timeout for libxl_cdrom_insert"):
> On Tue, Jun 04, 2019 at 06:47:32PM +0100, Ian Jackson wrote:
> > But I wonder if this could somehow be placed earlier to preserve
> > bisectability.
> 
> I think it would be possible to place the patch right after "libxl_disk:
> Cut libxl_cdrom_insert into steps", even though the timeout will never
> get a chance to actually fire. (Before "Use ev_qmp ...", everything is
> synchronous.)

Sounds good to me.

Ian.

Patch

diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index 785c8a27e7..8ccbdf0da0 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -669,6 +669,7 @@  typedef struct {
     libxl__domain_qmp_lock *qmp_lock;
     int dm_ver;
     libxl__ev_qmp qmp;
+    libxl__ev_time time;
 } libxl__cdrom_insert_state;
 
 static void cdrom_insert_ejected(libxl__egc *egc, libxl__ev_qmp *,
@@ -677,6 +678,9 @@  static void cdrom_insert_addfd_cb(libxl__egc *egc, libxl__ev_qmp *,
                                   const libxl__json_object *, int rc);
 static void cdrom_insert_inserted(libxl__egc *egc, libxl__ev_qmp *,
                                   const libxl__json_object *, int rc);
+static void cdrom_insert_timout(libxl__egc *egc, libxl__ev_time *ev,
+                                const struct timeval *requested_abs,
+                                int rc);
 static void cdrom_insert_done(libxl__egc *egc,
                               libxl__cdrom_insert_state *cis,
                               int rc);
@@ -697,6 +701,7 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     cis->disk = disk;
     libxl_device_disk_init(&cis->disk_saved);
     libxl_device_disk_copy(ctx, &cis->disk_saved, disk);
+    libxl__ev_time_init(&cis->time);
     libxl__ev_qmp_init(&cis->qmp);
     cis->qmp.ao = ao;
     cis->qmp.domid = domid;
@@ -755,6 +760,11 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         goto out;
     }
 
+    rc = libxl__ev_time_register_rel(ao, &cis->time,
+                                     cdrom_insert_timout,
+                                     LIBXL_HOTPLUG_TIMEOUT * 1000);
+    if (rc) goto out;
+
     /* We need to eject the original image first.
      * JSON is not updated.
      */
@@ -1030,12 +1040,23 @@  static void cdrom_insert_inserted(libxl__egc *egc,
     cdrom_insert_done(egc, cis, rc); /* must be last */
 }
 
+static void cdrom_insert_timout(libxl__egc *egc, libxl__ev_time *ev,
+                                const struct timeval *requested_abs,
+                                int rc)
+{
+    EGC_GC;
+    libxl__cdrom_insert_state *cis = CONTAINER_OF(ev, *cis, time);
+    LOGD(ERROR, cis->domid, "cdrom insertion timed out");
+    cdrom_insert_done(egc, cis, rc);
+}
+
 static void cdrom_insert_done(libxl__egc *egc,
                               libxl__cdrom_insert_state *cis,
                               int rc)
 {
     EGC_GC;
 
+    libxl__ev_time_deregister(gc, &cis->time);
     libxl__ev_qmp_dispose(gc, &cis->qmp);
     if (cis->qmp.payload_fd >= 0) close(cis->qmp.payload_fd);
     if (cis->qmp_lock) libxl__unlock_domain_qmp(cis->qmp_lock);