Patchwork block: handle -EBUSY in bdrv_commit_all()

login
register
mail settings
Submitter Stefan Hajnoczi
Date March 5, 2012, 6:10 p.m.
Message ID <1330971011-31646-1-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/144751/
State New
Headers show

Comments

Stefan Hajnoczi - March 5, 2012, 6:10 p.m.
Monitor operations that manipulate image files must not execute while a
background job (like image streaming) is in progress.  This prevents
corruptions from happening when two pieces of code are manipulating the
image file without knowledge of each other.

The monitor "commit" command raises QERR_DEVICE_IN_USE when
bdrv_commit() returns -EBUSY but "commit all" has no error handling.
This is easy to fix, although note that we do not deliver a detailed
error about which device was busy in the "commit all" case.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c    |    8 ++++++--
 block.h    |    2 +-
 blockdev.c |    9 ++++++---
 3 files changed, 13 insertions(+), 6 deletions(-)
Kevin Wolf - March 6, 2012, 10:01 a.m.
Am 05.03.2012 19:10, schrieb Stefan Hajnoczi:
> Monitor operations that manipulate image files must not execute while a
> background job (like image streaming) is in progress.  This prevents
> corruptions from happening when two pieces of code are manipulating the
> image file without knowledge of each other.
> 
> The monitor "commit" command raises QERR_DEVICE_IN_USE when
> bdrv_commit() returns -EBUSY but "commit all" has no error handling.
> This is easy to fix, although note that we do not deliver a detailed
> error about which device was busy in the "commit all" case.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Thanks, applied to the block branch.

Kevin

Patch

diff --git a/block.c b/block.c
index 52ffe14..b88ee90 100644
--- a/block.c
+++ b/block.c
@@ -1244,13 +1244,17 @@  ro_cleanup:
     return ret;
 }
 
-void bdrv_commit_all(void)
+int bdrv_commit_all(void)
 {
     BlockDriverState *bs;
 
     QTAILQ_FOREACH(bs, &bdrv_states, list) {
-        bdrv_commit(bs);
+        int ret = bdrv_commit(bs);
+        if (ret < 0) {
+            return ret;
+        }
     }
+    return 0;
 }
 
 struct BdrvTrackedRequest {
diff --git a/block.h b/block.h
index 48d0bf3..415bb17 100644
--- a/block.h
+++ b/block.h
@@ -165,7 +165,7 @@  int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
 void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs);
 int bdrv_commit(BlockDriverState *bs);
-void bdrv_commit_all(void);
+int bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
     const char *backing_file, const char *backing_fmt);
 void bdrv_register(BlockDriver *bdrv);
diff --git a/blockdev.c b/blockdev.c
index d78aa51..c6c66ee 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -627,12 +627,15 @@  void do_commit(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
     BlockDriverState *bs;
+    int ret;
 
     if (!strcmp(device, "all")) {
-        bdrv_commit_all();
+        ret = bdrv_commit_all();
+        if (ret == -EBUSY) {
+            qerror_report(QERR_DEVICE_IN_USE, device);
+            return;
+        }
     } else {
-        int ret;
-
         bs = bdrv_find(device);
         if (!bs) {
             qerror_report(QERR_DEVICE_NOT_FOUND, device);