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

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