[Qemu-devel] [PATCH 3/3] qapi: Introduce blockdev-query-group-snapshot-failure

[Jeff Cody]

In the case of a failure in a group snapshot, it is possible for
multiple file image failures to occur - for instance, failure of
an original snapshot, and then failure of one or more of the
attempted reopens of the original.

Knowing all of the file images which failed could be useful or
critical information, so this command returns a list of strings
containing the filenames of all failures from the last
invocation of blockdev-group-snapshot-sync.

Signed-off-by: Jeff Cody <address@hidden>
---
 blockdev.c       |   42 +++++++++++++++++++++++++++++++++++++++++-
 qapi-schema.json |   25 +++++++++++++++++++++++++
 qmp-commands.hx  |    5 +++++
 3 files changed, 71 insertions(+), 1 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0149720..cb44af5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -727,6 +727,7 @@ typedef struct BlkGroupSnapshotData {
     QSIMPLEQ_ENTRY(BlkGroupSnapshotData) entry;
 } BlkGroupSnapshotData;
 
+SnapshotFailList *group_snap_fail_list;
 /*
  * 'Atomic' group snapshots.  The snapshots are taken as a set, and if any fail
  *  then we attempt to undo all of the pivots performed.
@@ -739,10 +740,24 @@ void qmp_blockdev_group_snapshot_sync(SnapshotDevList 
*dev_list,
     SnapshotDev *dev_info = NULL;
     BlkGroupSnapshotData *snap_entry;
     BlockDriver *proto_drv;
+    SnapshotFailList *fail_entry = group_snap_fail_list;
 
     QSIMPLEQ_HEAD(gsnp_list, BlkGroupSnapshotData) gsnp_list;
     QSIMPLEQ_INIT(&gsnp_list);
 
+    /*
+     * clear out our failure list first, and reclaim memory
+     * we maintain the list, so if a group snapshot fails
+     * we can be queried about which devices failed
+     */
+    SnapshotFailList *fail_entry_next = NULL;
+    while (NULL != fail_entry) {
+        g_free(fail_entry->value);
+        fail_entry_next = fail_entry->next;
+        g_free(fail_entry);
+        fail_entry = fail_entry_next;
+    }
+
     /* We don't do anything in this loop that commits us to the snapshot */
     while (NULL != dev_entry) {
         dev_info = dev_entry->value;
@@ -815,6 +830,16 @@ void qmp_blockdev_group_snapshot_sync(SnapshotDevList 
*dev_list,
          */
         if (ret != 0) {
             error_set(errp, QERR_OPEN_FILE_FAILED, snap_entry->snapshot_file);
+            /*
+             * We bail on the first failure, but add the failed filename to the
+             * return list in case any of the rollback pivots fail as well
+             */
+            SnapshotFailList *failure;
+            failure = g_malloc0(sizeof(SnapshotFailList));
+            failure->value = g_malloc0(sizeof(*failure->value));
+            failure->value->failed_file = g_strdup(snap_entry->snapshot_file);
+            failure->next = group_snap_fail_list;
+            group_snap_fail_list = failure;
             goto error_rollback;
         }
     }
@@ -829,7 +854,17 @@ error_rollback:
             ret = bdrv_open(snap_entry->bs, snap_entry->old_filename,
                             snap_entry->flags, snap_entry->old_drv);
             if (ret != 0) {
-                /* This is very very bad */
+                /*
+                 * This is very very bad.  Make sure the caller is aware
+                 * of which files failed, since there could be more than
+                 * one
+                 */
+                SnapshotFailList *failure;
+                failure = g_malloc0(sizeof(SnapshotFailList));
+                failure->value = g_malloc0(sizeof(*failure->value));
+                failure->value->failed_file = 
g_strdup(snap_entry->old_filename);
+                failure->next = group_snap_fail_list;
+                group_snap_fail_list = failure;
                 error_set(errp, QERR_OPEN_FILE_FAILED,
                           snap_entry->old_filename);
             }
@@ -843,6 +878,11 @@ exit:
     return;
 }
 
+SnapshotFailList *qmp_blockdev_query_group_snapshot_failure(Error **errp)
+{
+    return group_snap_fail_list;
+}
+
 
 static void eject_device(BlockDriverState *bs, int force, Error **errp)
 {
diff --git a/qapi-schema.json b/qapi-schema.json
index b8d66d0..c4b27a3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1121,6 +1121,14 @@
   'data': {'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
 
 ##
+# @SnapshotFail
+#
+#  @failed_file:       string of the filename that failed
+##
+{ 'type': 'SnapshotFail',
+  'data': {'failed_file': 'str' } }
+
+##
 # @blockdev-group-snapshot-sync
 #
 # Generates a synchronous snapshot of a group of one or more block devices,
@@ -1152,6 +1160,23 @@
   'data': { 'dev': [ 'SnapshotDev' ] } }
 
 ##
+# @blockdev-query-group-snapshot-failure
+#
+#
+# Returns: A list of @SnapshotFail, that contains the filenames for all 
failures
+#                 of the last blockdev-group-snapshot-sync command.
+#
+# Notes:
+#        Since there could potentially be more than one file open or drive
+#        failures, the additional command 
'blockdev-query-group-snapshot-failure'
+#        will return a list of all device files that have failed.  This could
+#        include the original filename if the reopen of an original image file
+#        failed.
+#
+##
+{ 'command': 'blockdev-query-group-snapshot-failure', 'returns': [ 
'SnapshotFail' ] }
+
+##
 # @blockdev-snapshot-sync
 #
 # Generates a synchronous snapshot of a block device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2fe1e6e..9a80e08 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -706,6 +706,11 @@ Example:
 EQMP
 
     {
+        .name       = "blockdev-query-group-snapshot-failure",
+        .args_type  = "",
+        .mhandler.cmd_new = 
qmp_marshal_input_blockdev_query_group_snapshot_failure,
+    },
+    {
         .name       = "blockdev-snapshot-sync",
         .args_type  = "device:B,snapshot-file:s,format:s?",
         .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
-- 
1.7.9.rc2.1.g69204