cryptospore / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-blockdev-unify-qmp_drive_backup-and-drive-backup-tra.patch

22c213
From 4a03ab2a6cc4974d8d43240d1297b09160818af3 Mon Sep 17 00:00:00 2001
22c213
From: Sergio Lopez Pascual <slp@redhat.com>
22c213
Date: Fri, 7 Feb 2020 11:27:42 +0000
22c213
Subject: [PATCH 09/18] blockdev: unify qmp_drive_backup and drive-backup
22c213
 transaction paths
22c213
22c213
RH-Author: Sergio Lopez Pascual <slp@redhat.com>
22c213
Message-id: <20200207112749.25073-3-slp@redhat.com>
22c213
Patchwork-id: 93755
22c213
O-Subject: [RHEL-AV-8.2.0 qemu-kvm PATCH v2 2/9] blockdev: unify qmp_drive_backup and drive-backup transaction paths
22c213
Bugzilla: 1745606 1746217 1773517 1779036 1782111 1782175 1783965
22c213
RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>
22c213
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
22c213
RH-Acked-by: Max Reitz <mreitz@redhat.com>
22c213
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
22c213
22c213
Issuing a drive-backup from qmp_drive_backup takes a slightly
22c213
different path than when it's issued from a transaction. In the code,
22c213
this is manifested as some redundancy between do_drive_backup() and
22c213
drive_backup_prepare().
22c213
22c213
This change unifies both paths, merging do_drive_backup() and
22c213
drive_backup_prepare(), and changing qmp_drive_backup() to create a
22c213
transaction instead of calling do_backup_common() direcly.
22c213
22c213
As a side-effect, now qmp_drive_backup() is executed inside a drained
22c213
section, as it happens when creating a drive-backup transaction. This
22c213
change is visible from the user's perspective, as the job gets paused
22c213
and immediately resumed before starting the actual work.
22c213
22c213
Also fix tests 141, 185 and 219 to cope with the extra
22c213
JOB_STATUS_CHANGE lines.
22c213
22c213
Signed-off-by: Sergio Lopez <slp@redhat.com>
22c213
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
22c213
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
22c213
(cherry picked from commit 2288ccfac96281c316db942d10e3f921c1373064)
22c213
Signed-off-by: Sergio Lopez <slp@redhat.com>
22c213
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
22c213
---
22c213
 blockdev.c                 | 224 ++++++++++++++++++++-------------------------
22c213
 tests/qemu-iotests/141.out |   2 +
22c213
 tests/qemu-iotests/185.out |   2 +
22c213
 tests/qemu-iotests/219     |   7 +-
22c213
 tests/qemu-iotests/219.out |   8 ++
22c213
 5 files changed, 117 insertions(+), 126 deletions(-)
22c213
22c213
diff --git a/blockdev.c b/blockdev.c
22c213
index 553e315..5e85fc0 100644
22c213
--- a/blockdev.c
22c213
+++ b/blockdev.c
22c213
@@ -1761,39 +1761,128 @@ typedef struct DriveBackupState {
22c213
     BlockJob *job;
22c213
 } DriveBackupState;
22c213
 
22c213
-static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
22c213
-                            Error **errp);
22c213
+static BlockJob *do_backup_common(BackupCommon *backup,
22c213
+                                  BlockDriverState *bs,
22c213
+                                  BlockDriverState *target_bs,
22c213
+                                  AioContext *aio_context,
22c213
+                                  JobTxn *txn, Error **errp);
22c213
 
22c213
 static void drive_backup_prepare(BlkActionState *common, Error **errp)
22c213
 {
22c213
     DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
22c213
-    BlockDriverState *bs;
22c213
     DriveBackup *backup;
22c213
+    BlockDriverState *bs;
22c213
+    BlockDriverState *target_bs;
22c213
+    BlockDriverState *source = NULL;
22c213
     AioContext *aio_context;
22c213
+    QDict *options;
22c213
     Error *local_err = NULL;
22c213
+    int flags;
22c213
+    int64_t size;
22c213
+    bool set_backing_hd = false;
22c213
 
22c213
     assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
22c213
     backup = common->action->u.drive_backup.data;
22c213
 
22c213
+    if (!backup->has_mode) {
22c213
+        backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
22c213
+    }
22c213
+
22c213
     bs = bdrv_lookup_bs(backup->device, backup->device, errp);
22c213
     if (!bs) {
22c213
         return;
22c213
     }
22c213
 
22c213
+    if (!bs->drv) {
22c213
+        error_setg(errp, "Device has no medium");
22c213
+        return;
22c213
+    }
22c213
+
22c213
     aio_context = bdrv_get_aio_context(bs);
22c213
     aio_context_acquire(aio_context);
22c213
 
22c213
     /* Paired with .clean() */
22c213
     bdrv_drained_begin(bs);
22c213
 
22c213
-    state->bs = bs;
22c213
+    if (!backup->has_format) {
22c213
+        backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
22c213
+                         NULL : (char *) bs->drv->format_name;
22c213
+    }
22c213
+
22c213
+    /* Early check to avoid creating target */
22c213
+    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
22c213
+        goto out;
22c213
+    }
22c213
+
22c213
+    flags = bs->open_flags | BDRV_O_RDWR;
22c213
+
22c213
+    /*
22c213
+     * See if we have a backing HD we can use to create our new image
22c213
+     * on top of.
22c213
+     */
22c213
+    if (backup->sync == MIRROR_SYNC_MODE_TOP) {
22c213
+        source = backing_bs(bs);
22c213
+        if (!source) {
22c213
+            backup->sync = MIRROR_SYNC_MODE_FULL;
22c213
+        }
22c213
+    }
22c213
+    if (backup->sync == MIRROR_SYNC_MODE_NONE) {
22c213
+        source = bs;
22c213
+        flags |= BDRV_O_NO_BACKING;
22c213
+        set_backing_hd = true;
22c213
+    }
22c213
+
22c213
+    size = bdrv_getlength(bs);
22c213
+    if (size < 0) {
22c213
+        error_setg_errno(errp, -size, "bdrv_getlength failed");
22c213
+        goto out;
22c213
+    }
22c213
+
22c213
+    if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
22c213
+        assert(backup->format);
22c213
+        if (source) {
22c213
+            bdrv_refresh_filename(source);
22c213
+            bdrv_img_create(backup->target, backup->format, source->filename,
22c213
+                            source->drv->format_name, NULL,
22c213
+                            size, flags, false, &local_err);
22c213
+        } else {
22c213
+            bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL,
22c213
+                            size, flags, false, &local_err);
22c213
+        }
22c213
+    }
22c213
 
22c213
-    state->job = do_drive_backup(backup, common->block_job_txn, &local_err);
22c213
     if (local_err) {
22c213
         error_propagate(errp, local_err);
22c213
         goto out;
22c213
     }
22c213
 
22c213
+    options = qdict_new();
22c213
+    qdict_put_str(options, "discard", "unmap");
22c213
+    qdict_put_str(options, "detect-zeroes", "unmap");
22c213
+    if (backup->format) {
22c213
+        qdict_put_str(options, "driver", backup->format);
22c213
+    }
22c213
+
22c213
+    target_bs = bdrv_open(backup->target, NULL, options, flags, errp);
22c213
+    if (!target_bs) {
22c213
+        goto out;
22c213
+    }
22c213
+
22c213
+    if (set_backing_hd) {
22c213
+        bdrv_set_backing_hd(target_bs, source, &local_err);
22c213
+        if (local_err) {
22c213
+            goto unref;
22c213
+        }
22c213
+    }
22c213
+
22c213
+    state->bs = bs;
22c213
+
22c213
+    state->job = do_backup_common(qapi_DriveBackup_base(backup),
22c213
+                                  bs, target_bs, aio_context,
22c213
+                                  common->block_job_txn, errp);
22c213
+
22c213
+unref:
22c213
+    bdrv_unref(target_bs);
22c213
 out:
22c213
     aio_context_release(aio_context);
22c213
 }
22c213
@@ -3587,126 +3676,13 @@ static BlockJob *do_backup_common(BackupCommon *backup,
22c213
     return job;
22c213
 }
22c213
 
22c213
-static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
22c213
-                                 Error **errp)
22c213
-{
22c213
-    BlockDriverState *bs;
22c213
-    BlockDriverState *target_bs;
22c213
-    BlockDriverState *source = NULL;
22c213
-    BlockJob *job = NULL;
22c213
-    AioContext *aio_context;
22c213
-    QDict *options;
22c213
-    Error *local_err = NULL;
22c213
-    int flags;
22c213
-    int64_t size;
22c213
-    bool set_backing_hd = false;
22c213
-
22c213
-    if (!backup->has_mode) {
22c213
-        backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
22c213
-    }
22c213
-
22c213
-    bs = bdrv_lookup_bs(backup->device, backup->device, errp);
22c213
-    if (!bs) {
22c213
-        return NULL;
22c213
-    }
22c213
-
22c213
-    if (!bs->drv) {
22c213
-        error_setg(errp, "Device has no medium");
22c213
-        return NULL;
22c213
-    }
22c213
-
22c213
-    aio_context = bdrv_get_aio_context(bs);
22c213
-    aio_context_acquire(aio_context);
22c213
-
22c213
-    if (!backup->has_format) {
22c213
-        backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
22c213
-                         NULL : (char *) bs->drv->format_name;
22c213
-    }
22c213
-
22c213
-    /* Early check to avoid creating target */
22c213
-    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
22c213
-        goto out;
22c213
-    }
22c213
-
22c213
-    flags = bs->open_flags | BDRV_O_RDWR;
22c213
-
22c213
-    /*
22c213
-     * See if we have a backing HD we can use to create our new image
22c213
-     * on top of.
22c213
-     */
22c213
-    if (backup->sync == MIRROR_SYNC_MODE_TOP) {
22c213
-        source = backing_bs(bs);
22c213
-        if (!source) {
22c213
-            backup->sync = MIRROR_SYNC_MODE_FULL;
22c213
-        }
22c213
-    }
22c213
-    if (backup->sync == MIRROR_SYNC_MODE_NONE) {
22c213
-        source = bs;
22c213
-        flags |= BDRV_O_NO_BACKING;
22c213
-        set_backing_hd = true;
22c213
-    }
22c213
-
22c213
-    size = bdrv_getlength(bs);
22c213
-    if (size < 0) {
22c213
-        error_setg_errno(errp, -size, "bdrv_getlength failed");
22c213
-        goto out;
22c213
-    }
22c213
-
22c213
-    if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
22c213
-        assert(backup->format);
22c213
-        if (source) {
22c213
-            bdrv_refresh_filename(source);
22c213
-            bdrv_img_create(backup->target, backup->format, source->filename,
22c213
-                            source->drv->format_name, NULL,
22c213
-                            size, flags, false, &local_err);
22c213
-        } else {
22c213
-            bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL,
22c213
-                            size, flags, false, &local_err);
22c213
-        }
22c213
-    }
22c213
-
22c213
-    if (local_err) {
22c213
-        error_propagate(errp, local_err);
22c213
-        goto out;
22c213
-    }
22c213
-
22c213
-    options = qdict_new();
22c213
-    qdict_put_str(options, "discard", "unmap");
22c213
-    qdict_put_str(options, "detect-zeroes", "unmap");
22c213
-    if (backup->format) {
22c213
-        qdict_put_str(options, "driver", backup->format);
22c213
-    }
22c213
-
22c213
-    target_bs = bdrv_open(backup->target, NULL, options, flags, errp);
22c213
-    if (!target_bs) {
22c213
-        goto out;
22c213
-    }
22c213
-
22c213
-    if (set_backing_hd) {
22c213
-        bdrv_set_backing_hd(target_bs, source, &local_err);
22c213
-        if (local_err) {
22c213
-            goto unref;
22c213
-        }
22c213
-    }
22c213
-
22c213
-    job = do_backup_common(qapi_DriveBackup_base(backup),
22c213
-                           bs, target_bs, aio_context, txn, errp);
22c213
-
22c213
-unref:
22c213
-    bdrv_unref(target_bs);
22c213
-out:
22c213
-    aio_context_release(aio_context);
22c213
-    return job;
22c213
-}
22c213
-
22c213
-void qmp_drive_backup(DriveBackup *arg, Error **errp)
22c213
+void qmp_drive_backup(DriveBackup *backup, Error **errp)
22c213
 {
22c213
-
22c213
-    BlockJob *job;
22c213
-    job = do_drive_backup(arg, NULL, errp);
22c213
-    if (job) {
22c213
-        job_start(&job->job);
22c213
-    }
22c213
+    TransactionAction action = {
22c213
+        .type = TRANSACTION_ACTION_KIND_DRIVE_BACKUP,
22c213
+        .u.drive_backup.data = backup,
22c213
+    };
22c213
+    blockdev_do_action(&action, errp);
22c213
 }
22c213
 
22c213
 BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
22c213
diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out
22c213
index 3645675..263b680 100644
22c213
--- a/tests/qemu-iotests/141.out
22c213
+++ b/tests/qemu-iotests/141.out
22c213
@@ -13,6 +13,8 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/m.
22c213
 Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT
22c213
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
22c213
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}
22c213
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "paused", "id": "job0"}}
22c213
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}
22c213
 {'execute': 'blockdev-del', 'arguments': {'node-name': 'drv0'}}
22c213
 {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is used as backing hd of 'NODE_NAME'"}}
22c213
 {'execute': 'block-job-cancel', 'arguments': {'device': 'job0'}}
22c213
diff --git a/tests/qemu-iotests/185.out b/tests/qemu-iotests/185.out
22c213
index 8379ac5..9a3b657 100644
22c213
--- a/tests/qemu-iotests/185.out
22c213
+++ b/tests/qemu-iotests/185.out
22c213
@@ -65,6 +65,8 @@ Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 l
22c213
 Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 lazy_refcounts=off refcount_bits=16
22c213
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "disk"}}
22c213
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "disk"}}
22c213
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "paused", "id": "disk"}}
22c213
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "disk"}}
22c213
 {"return": {}}
22c213
 { 'execute': 'quit' }
22c213
 {"return": {}}
22c213
diff --git a/tests/qemu-iotests/219 b/tests/qemu-iotests/219
22c213
index e0c5166..655f54d 100755
22c213
--- a/tests/qemu-iotests/219
22c213
+++ b/tests/qemu-iotests/219
22c213
@@ -63,7 +63,7 @@ def test_pause_resume(vm):
22c213
                 # logged immediately
22c213
                 iotests.log(vm.qmp('query-jobs'))
22c213
 
22c213
-def test_job_lifecycle(vm, job, job_args, has_ready=False):
22c213
+def test_job_lifecycle(vm, job, job_args, has_ready=False, is_mirror=False):
22c213
     global img_size
22c213
 
22c213
     iotests.log('')
22c213
@@ -135,6 +135,9 @@ def test_job_lifecycle(vm, job, job_args, has_ready=False):
22c213
     iotests.log('Waiting for PENDING state...')
22c213
     iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
22c213
     iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
22c213
+    if is_mirror:
22c213
+        iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
22c213
+        iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
22c213
 
22c213
     if not job_args.get('auto-finalize', True):
22c213
         # PENDING state:
22c213
@@ -218,7 +221,7 @@ with iotests.FilePath('disk.img') as disk_path, \
22c213
 
22c213
     for auto_finalize in [True, False]:
22c213
         for auto_dismiss in [True, False]:
22c213
-            test_job_lifecycle(vm, 'drive-backup', job_args={
22c213
+            test_job_lifecycle(vm, 'drive-backup', is_mirror=True, job_args={
22c213
                 'device': 'drive0-node',
22c213
                 'target': copy_path,
22c213
                 'sync': 'full',
22c213
diff --git a/tests/qemu-iotests/219.out b/tests/qemu-iotests/219.out
22c213
index 8ebd3fe..0ea5d0b 100644
22c213
--- a/tests/qemu-iotests/219.out
22c213
+++ b/tests/qemu-iotests/219.out
22c213
@@ -135,6 +135,8 @@ Pause/resume in RUNNING
22c213
 {"return": {}}
22c213
 
22c213
 Waiting for PENDING state...
22c213
+{"data": {"id": "job0", "status": "paused"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
+{"data": {"id": "job0", "status": "running"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "waiting"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "pending"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "concluded"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
@@ -186,6 +188,8 @@ Pause/resume in RUNNING
22c213
 {"return": {}}
22c213
 
22c213
 Waiting for PENDING state...
22c213
+{"data": {"id": "job0", "status": "paused"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
+{"data": {"id": "job0", "status": "running"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "waiting"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "pending"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "concluded"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
@@ -245,6 +249,8 @@ Pause/resume in RUNNING
22c213
 {"return": {}}
22c213
 
22c213
 Waiting for PENDING state...
22c213
+{"data": {"id": "job0", "status": "paused"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
+{"data": {"id": "job0", "status": "running"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "waiting"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "pending"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"return": [{"current-progress": 4194304, "id": "job0", "status": "pending", "total-progress": 4194304, "type": "backup"}]}
22c213
@@ -304,6 +310,8 @@ Pause/resume in RUNNING
22c213
 {"return": {}}
22c213
 
22c213
 Waiting for PENDING state...
22c213
+{"data": {"id": "job0", "status": "paused"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
+{"data": {"id": "job0", "status": "running"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "waiting"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"data": {"id": "job0", "status": "pending"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
22c213
 {"return": [{"current-progress": 4194304, "id": "job0", "status": "pending", "total-progress": 4194304, "type": "backup"}]}
22c213
-- 
22c213
1.8.3.1
22c213