xen-blkback: sync I/O after backend disconnected

When backend disconnect, sync IO requests to the disk.

---
 drivers/block/xen-blkback/xenbus.c |    8 ++++++++
 1 file changed, 8 insertions(+)

index 1a87f5b..da39d61 100644
@@ -35,6 +35,7 @@ static void connect(struct backend_info *);
 static int connect_ring(struct backend_info *);
 static void backend_changed(struct xenbus_watch *, const char **,
 			    unsigned int);
+static void xen_vbd_sync(struct xen_vbd *vbd);
 
 struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
 {

@@ -232,6 +233,7 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
 		free_vm_area(blkif->blk_ring_area);
 		blkif->blk_rings.common.sring = NULL;
 	}
+	xen_vbd_sync(&blkif->vbd);
 }
 
 void xen_blkif_free(struct xen_blkif *blkif)

@@ -332,6 +334,12 @@ static void xen_vbd_free(struct xen_vbd *vbd)
 	vbd->bdev = NULL;
 }
 
+static void xen_vbd_sync(struct xen_vbd *vbd)
+{
+	if (vbd->bdev)
+		fsync_bdev(vbd->bdev);
+}
+
 static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
 			  unsigned major, unsigned minor, int readonly,
 			  int cdrom)

Care to explain why?

Also you'll just need a sync_blockdev, fsync_bdev does far to many things that don't make any sense when you don't have a file system mounted on a device.

I was thinking it might be a good idea to do that when a disk (file, LVM, real block device) is released from a guest just in case there is some outstanding I/Os. But then I realized that we bypasses the page cache anyhow - so there should be no outstanding I/O requests - unless they are in the disk queue.

And the guest would normally issues a FLUSH when unmounting the disk. Hm, I wonder what the conditions are when we forcibly kill the guest - there might be outstanding I/Os in the disk's cache - at which point we should probably sync the write cache, no?

When backend disconnect, I think we'd better flush all dirty data to the disk ASAP.

xen-blkback support physical device and loopback file, so I think here should be fsync_bdev()?

Thanks,
Joe

Then please state this in the commit log, and even better the code also.

No, and in fact sync_blockdev is also wrong when thinking about it. blkback always submits bios directly to the device, so flushing any kernel caches won't do you any help. The only thing that might make sense to flush would be the disk write cache using a blkdev_issue_flush.

Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] Re: [patch] xen-blkback: sync I/O after backend disconnected"):

If we forcibly kill the guest we don't care about its IO in flight. After all we are throwing away everything that the guest has in its queue.

Bear in mind that the reason for forcibly killing (or perhaps forcibly detaching) might be that the underlying device has wedged somehow. It would be annoying if that prevented even a force detach.

Or to put it in other words: force detach and force kill should be lossy. Their whole point is to be the lossy version.

Ian.

Why don't we care about it? It might have become wedged but that does not mean we don't want to preserve as much as possible.

We end up finishing all of the outstanding I/Os that guest has sent. When all of the bio_submit callbacks have finished then we release the guest backend. So adding in a 'SYNC' to the queue would force the disk cache to flush all the writes at least.

Hm, I think you still end up holding on the request queue and not freeing the blkback thread and all of its goodies until the bio callbacks have completed. When the device gets wedged they become wedged too. You have to wait for the error handler to give up and then the avalanche of 'write page lost' and 'error secxtor XX' along with the -ENXIO being returned ends up in the bio callbacks.

What I am saying is that the force detach is still stuck waiting if the device has wedged - irregardless of this patch.