1. Patchew
  2. QEMU
  3. [PATCH v3 0/6] block: add block-dirty-bitmap-populate job
  4. View patch

[PATCH v3 2/6] blockdev: combine DriveBackupState and BlockdevBackupState

Eric Blake posted 6 patches 5 years, 7 months ago
Maintainers: Eric Blake <eblake@redhat.com>, Max Reitz <mreitz@redhat.com>, John Snow <jsnow@redhat.com>, Markus Armbruster <armbru@redhat.com>, Kevin Wolf <kwolf@redhat.com>
There is a newer version of this series
[PATCH v3 2/6] blockdev: combine DriveBackupState and BlockdevBackupState
Posted by Eric Blake 5 years, 7 months ago 
From: John Snow <jsnow@redhat.com>

They have the same fields -- rename it BlockJobActionState.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 blockdev.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 72df193ca73b..6d80af903c55 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1655,11 +1655,11 @@ static void external_snapshot_clean(BlkActionState *common)
     aio_context_release(aio_context);
 }

-typedef struct DriveBackupState {
+typedef struct BlockJobActionState {
     BlkActionState common;
     BlockDriverState *bs;
     BlockJob *job;
-} DriveBackupState;
+} BlockJobActionState;

 static BlockJob *do_backup_common(BackupCommon *backup,
                                   BlockDriverState *bs,
@@ -1669,7 +1669,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,

 static void drive_backup_prepare(BlkActionState *common, Error **errp)
 {
-    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
     DriveBackup *backup;
     BlockDriverState *bs;
     BlockDriverState *target_bs;
@@ -1806,7 +1806,7 @@ out:

 static void drive_backup_commit(BlkActionState *common)
 {
-    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
     AioContext *aio_context;

     aio_context = bdrv_get_aio_context(state->bs);
@@ -1820,7 +1820,7 @@ static void drive_backup_commit(BlkActionState *common)

 static void drive_backup_abort(BlkActionState *common)
 {
-    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);

     if (state->job) {
         AioContext *aio_context;
@@ -1836,7 +1836,7 @@ static void drive_backup_abort(BlkActionState *common)

 static void drive_backup_clean(BlkActionState *common)
 {
-    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
     AioContext *aio_context;

     if (!state->bs) {
@@ -1851,15 +1851,9 @@ static void drive_backup_clean(BlkActionState *common)
     aio_context_release(aio_context);
 }

-typedef struct BlockdevBackupState {
-    BlkActionState common;
-    BlockDriverState *bs;
-    BlockJob *job;
-} BlockdevBackupState;
-
 static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
 {
-    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
+    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
     BlockdevBackup *backup;
     BlockDriverState *bs;
     BlockDriverState *target_bs;
@@ -1907,7 +1901,7 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)

 static void blockdev_backup_commit(BlkActionState *common)
 {
-    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
+    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
     AioContext *aio_context;

     aio_context = bdrv_get_aio_context(state->bs);
@@ -1921,7 +1915,7 @@ static void blockdev_backup_commit(BlkActionState *common)

 static void blockdev_backup_abort(BlkActionState *common)
 {
-    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
+    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);

     if (state->job) {
         AioContext *aio_context;
@@ -1937,7 +1931,7 @@ static void blockdev_backup_abort(BlkActionState *common)

 static void blockdev_backup_clean(BlkActionState *common)
 {
-    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
+    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
     AioContext *aio_context;

     if (!state->bs) {
@@ -2209,14 +2203,14 @@ static const BlkActionOps actions[] = {
         .clean = external_snapshot_clean,
     },
     [TRANSACTION_ACTION_KIND_DRIVE_BACKUP] = {
-        .instance_size = sizeof(DriveBackupState),
+        .instance_size = sizeof(BlockJobActionState),
         .prepare = drive_backup_prepare,
         .commit = drive_backup_commit,
         .abort = drive_backup_abort,
         .clean = drive_backup_clean,
     },
     [TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
-        .instance_size = sizeof(BlockdevBackupState),
+        .instance_size = sizeof(BlockJobActionState),
         .prepare = blockdev_backup_prepare,
         .commit = blockdev_backup_commit,
         .abort = blockdev_backup_abort,
-- 
2.27.0
Re: [PATCH v3 2/6] blockdev: combine DriveBackupState and BlockdevBackupState
Posted by Vladimir Sementsov-Ogievskiy 5 years, 7 months ago 
19.06.2020 22:56, Eric Blake wrote:
> From: John Snow <jsnow@redhat.com>
> 
> They have the same fields -- rename it BlockJobActionState.

commit/abort/clean functions are identical after it. I think better to combine them as well here

> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   blockdev.c | 30 ++++++++++++------------------
>   1 file changed, 12 insertions(+), 18 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 72df193ca73b..6d80af903c55 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1655,11 +1655,11 @@ static void external_snapshot_clean(BlkActionState *common)
>       aio_context_release(aio_context);
>   }
> 
> -typedef struct DriveBackupState {
> +typedef struct BlockJobActionState {
>       BlkActionState common;
>       BlockDriverState *bs;
>       BlockJob *job;
> -} DriveBackupState;
> +} BlockJobActionState;
> 
>   static BlockJob *do_backup_common(BackupCommon *backup,
>                                     BlockDriverState *bs,
> @@ -1669,7 +1669,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
> 
>   static void drive_backup_prepare(BlkActionState *common, Error **errp)
>   {
> -    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
> +    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
>       DriveBackup *backup;
>       BlockDriverState *bs;
>       BlockDriverState *target_bs;
> @@ -1806,7 +1806,7 @@ out:
> 
>   static void drive_backup_commit(BlkActionState *common)
>   {
> -    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
> +    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
>       AioContext *aio_context;
> 
>       aio_context = bdrv_get_aio_context(state->bs);
> @@ -1820,7 +1820,7 @@ static void drive_backup_commit(BlkActionState *common)
> 
>   static void drive_backup_abort(BlkActionState *common)
>   {
> -    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
> +    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
> 
>       if (state->job) {
>           AioContext *aio_context;
> @@ -1836,7 +1836,7 @@ static void drive_backup_abort(BlkActionState *common)
> 
>   static void drive_backup_clean(BlkActionState *common)
>   {
> -    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
> +    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
>       AioContext *aio_context;
> 
>       if (!state->bs) {
> @@ -1851,15 +1851,9 @@ static void drive_backup_clean(BlkActionState *common)
>       aio_context_release(aio_context);
>   }
> 
> -typedef struct BlockdevBackupState {
> -    BlkActionState common;
> -    BlockDriverState *bs;
> -    BlockJob *job;
> -} BlockdevBackupState;
> -
>   static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
>   {
> -    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
> +    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
>       BlockdevBackup *backup;
>       BlockDriverState *bs;
>       BlockDriverState *target_bs;
> @@ -1907,7 +1901,7 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
> 
>   static void blockdev_backup_commit(BlkActionState *common)
>   {
> -    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
> +    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
>       AioContext *aio_context;
> 
>       aio_context = bdrv_get_aio_context(state->bs);
> @@ -1921,7 +1915,7 @@ static void blockdev_backup_commit(BlkActionState *common)
> 
>   static void blockdev_backup_abort(BlkActionState *common)
>   {
> -    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
> +    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
> 
>       if (state->job) {
>           AioContext *aio_context;
> @@ -1937,7 +1931,7 @@ static void blockdev_backup_abort(BlkActionState *common)
> 
>   static void blockdev_backup_clean(BlkActionState *common)
>   {
> -    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
> +    BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, common);
>       AioContext *aio_context;
> 
>       if (!state->bs) {
> @@ -2209,14 +2203,14 @@ static const BlkActionOps actions[] = {
>           .clean = external_snapshot_clean,
>       },
>       [TRANSACTION_ACTION_KIND_DRIVE_BACKUP] = {
> -        .instance_size = sizeof(DriveBackupState),
> +        .instance_size = sizeof(BlockJobActionState),
>           .prepare = drive_backup_prepare,
>           .commit = drive_backup_commit,
>           .abort = drive_backup_abort,
>           .clean = drive_backup_clean,
>       },
>       [TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
> -        .instance_size = sizeof(BlockdevBackupState),
> +        .instance_size = sizeof(BlockJobActionState),
>           .prepare = blockdev_backup_prepare,
>           .commit = blockdev_backup_commit,
>           .abort = blockdev_backup_abort,
> 


-- 
Best regards,
Vladimir
Re: [PATCH v3 2/6] blockdev: combine DriveBackupState and BlockdevBackupState
Posted by Eric Blake 5 years, 7 months ago 
On 6/22/20 12:12 AM, Vladimir Sementsov-Ogievskiy wrote:
> 19.06.2020 22:56, Eric Blake wrote:
>> From: John Snow <jsnow@redhat.com>
>>
>> They have the same fields -- rename it BlockJobActionState.
> 
> commit/abort/clean functions are identical after it. I think better to 
> combine them as well here

I'll give it a shot in v4 (may be done as a separate patch for ease of 
review, though).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.