qapi: add dirty bitmap status
PATCH 1/1 qapi: add dirty bitmap status
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)
block.c +12 -1
@@ -3104,6 +3104,17 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)return !(bitmap->disabled || bitmap->successor); } +DirtyBitmapStatus bdrv_dirty_bitmap_status (BdrvDirtyBitmap *bitmap) +{ +if (bdrv_dirty_bitmap_frozen (bitmap)) { +return DIRTY_BITMAP_STATUS_FROZEN ; + }else if (!bdrv_dirty_bitmap_enabled (bitmap)) { +return DIRTY_BITMAP_STATUS_DISABLED ; + }else { +return DIRTY_BITMAP_STATUS_ACTIVE ; + } +} +/** * Create a successor bitmap destined to replace this bitmap after an operation. * Requires that the bitmap is not frozen and has no successor.
@@ -3244,7 +3255,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) info->granularity =bdrv_dirty_bitmap_granularity (bm); info->has_name = !!bm->name; info->name =g_strdup (bm->name); - info->frozen =bdrv_dirty_bitmap_frozen (bm); + info->status =bdrv_dirty_bitmap_status (bm); entry->value = info; *plist = entry; plist = &entry->next;
include/block/block.h +1 -0
@@ -474,6 +474,7 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);uint32_t bdrv_dirty_bitmap_granularity (BdrvDirtyBitmap *bitmap);bool bdrv_dirty_bitmap_enabled (BdrvDirtyBitmap *bitmap);bool bdrv_dirty_bitmap_frozen (BdrvDirtyBitmap *bitmap); +DirtyBitmapStatus bdrv_dirty_bitmap_status (BdrvDirtyBitmap *bitmap);int bdrv_get_dirty (BlockDriverState *bs,BdrvDirtyBitmap *bitmap,int64_t sector);void bdrv_set_dirty_bitmap (BdrvDirtyBitmap *bitmap,int64_t cur_sector,int nr_sectors);
qapi/block-core.json +21 -2
@@ -326,6 +326,25 @@ 'data': 'bool', '*offset': 'int' } } ## +# @DirtyBitmapStatus: +# +# An enumeration of possible states that a dirty bitmap can report to the user. +# +# @frozen: The bitmap is currently in-use by a backup operation or block job, +# and is immutable. +# +# @disabled: The bitmap is currently in-use by an internal operation and is +# read-only. It can still be deleted. +# +# @active: The bitmap is actively monitoring for new writes, and can be cleared, +# deleted, or used for backup operations. +# +# Since: 2.4 +## +{ 'enum': 'DirtyBitmapStatus', + 'data': ['active', 'disabled', 'frozen'] } + +## # @BlockDirtyInfo: # # Block dirty bitmap information.
@@ -336,13 +355,13 @@ # # @granularity: granularity of the dirty bitmap in bytes (since 1.4) # -# @frozen: whether the dirty bitmap is frozen (Since 2.4) +# @status: current status of the dirty bitmap (since 2.4) # # Since: 1.3 ## { 'struct': 'BlockDirtyInfo', 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32', - 'frozen': 'bool'} } + 'status': 'DirtyBitmapStatus'} } ## # @BlockInfo:
Signature · 1 line
2.1.0
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 10 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
Quoted 6 lines
Quoted 1 line
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Good thing you mentioned it!
:)
Quoted 10 lines
Quoted 7 lines
Suggested-by Eric Blake <[email protected]> Signed-off-by: JohnSnow <[email protected]> ---
block.c| 13
++++++++++++-include/block/block.h| 1 +qapi/block-core.json| 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Reviewed-by Eric Blake <[email protected]>
Thanks!
Quoted 25 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)
Quoted 2 lines
Reviewed-by Eric Blake <[email protected]>
I'm not actually sure whose tree this should go in. Markus's, perhaps?
("ping")
Quoted 26 lines · John Snow
Quoted 25 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)
Quoted 6 lines
Quoted 2 lines
Reviewed-by Eric Blake <[email protected]>I'm not actually sure whose tree this should go in. Markus's, perhaps?
("ping")
I guess the case for "Block layer core" (Kevin) is at least as strong as the case for "QAPI" (me). Kevin, what do you think?
Quoted 31 lines · Markus Armbruster
Quoted 29 lines · John Snow
Quoted 27 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Reviewed-by Eric Blake <[email protected]>
Quoted 7 lines
Quoted 3 lines
I'm not actually sure whose tree this should go in. Markus's, perhaps?
("ping")
I guess the case for "Block layer core" (Kevin) is at least as strong as the case for "QAPI" (me). Kevin, what do you think?
His silence says "Markus, can you please do it? I discovered today that I don't care about this patch."
--js
Am 21.05.2015 um 23:48 hat John Snow geschrieben:
Quoted 34 lines · Markus Armbruster
Quoted 31 lines
Quoted 29 lines · John Snow
Quoted 27 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Reviewed-by Eric Blake <[email protected]>
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
I'm not actually sure whose tree this should go in. Markus's, perhaps?
("ping")
I guess the case for "Block layer core" (Kevin) is at least as strong as the case for "QAPI" (me). Kevin, what do you think?
I think bdrv_query_dirty_bitmaps() really belongs into block/qapi.c, which is yours anyway. So it's either you as the QAPI maintainer or you as the block submaintainer.
But if you think otherwise, I can consider it.
Quoted 2 lines
His silence says "Markus, can you please do it? I discovered today that I don't care about this patch."
I'm sorry, John, but you didn't CC me, you didn't CC qemu-block, you didn't CC anyone. I only had a chance to know about it since Wednesday when Markus forwarded it, and I'm not sitting there waiting for new patch emails because I'm bored. Rest assured, I have enough of them.
And then the forwarded email didn't even quote the patch any more, so I couldn't just give a quick reply, but had to find the full email thread in a different folder.
If you want to have patches applied quickly, make it easy for the maintainers. You did the exact opposite, so you have no reason to complain.
Kevin
Quoted 37 lines · Kevin Wolf
Am 21.05.2015 um 23:48 hat John Snow geschrieben:
Quoted 36 lines · Markus Armbruster
Quoted 33 lines
Quoted 31 lines · John Snow
Quoted 25 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Quoted 2 lines
Reviewed-by Eric Blake <[email protected]>I'm not actually sure whose tree this should go in. Markus's, perhaps?
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
("ping")
I guess the case for "Block layer core" (Kevin) is at least as strong as the case for "QAPI" (me). Kevin, what do you think?
I think
bdrv_query_dirty_bitmaps()really belongs intoblock/qapi.c, which is yours anyway. So it's either you as the QAPI maintainer or you as the block submaintainer.
s/the block submaintainer/the newly minted block submaintainer/
Quoted 10 lines
But if you think otherwise, I can consider it.
Quoted 2 lines
His silence says "Markus, can you please do it? I discovered today that I don't care about this patch."
I'm sorry, John, but you didn't CC me, you didn't CC qemu-block, you didn't CC anyone. I only had a chance to know about it since Wednesday when Markus forwarded it, and I'm not sitting there waiting for new patch emails because I'm bored. Rest assured, I have enough of them.
Quoted 7 lines
And then the forwarded email didn't even quote the patch any more, so I couldn't just give a quick reply, but had to find the full email thread in a different folder.
If you want to have patches applied quickly, make it easy for the maintainers. You did the exact opposite, so you have no reason to complain.
On the other hand, his "complaining" made me smile, which I appreciate :)
Don't worry, John, I'll take it through my tree.
Am 22.05.2015 um 10:31 hat Markus Armbruster geschrieben:
Quoted 64 lines
Quoted 37 lines · Kevin Wolf
Am 21.05.2015 um 23:48 hat John Snow geschrieben:
Quoted 36 lines · Markus Armbruster
Quoted 33 lines
Quoted 31 lines · John Snow
Quoted 25 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Quoted 2 lines
Reviewed-by Eric Blake <[email protected]>I'm not actually sure whose tree this should go in. Markus's, perhaps?
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
("ping")
I guess the case for "Block layer core" (Kevin) is at least as strong as the case for "QAPI" (me). Kevin, what do you think?
I think
bdrv_query_dirty_bitmaps()really belongs intoblock/qapi.c, which is yours anyway. So it's either you as the QAPI maintainer or you as the block submaintainer.s/the block submaintainer/the newly minted block submaintainer/
Quoted 14 lines
But if you think otherwise, I can consider it.
Quoted 2 lines
His silence says "Markus, can you please do it? I discovered today that I don't care about this patch."
I'm sorry, John, but you didn't CC me, you didn't CC qemu-block, you didn't CC anyone. I only had a chance to know about it since Wednesday when Markus forwarded it, and I'm not sitting there waiting for new patch emails because I'm bored. Rest assured, I have enough of them.
And then the forwarded email didn't even quote the patch any more, so I couldn't just give a quick reply, but had to find the full email thread in a different folder.
Quoted 5 lines
Quoted 3 lines
If you want to have patches applied quickly, make it easy for the maintainers. You did the exact opposite, so you have no reason to complain.
On the other hand, his "complaining" made me smile, which I appreciate :)
Drom secht mr's jô em Guada. ;-)
I'm sorry if my reply reads a bit too harsh, it's not meant like that. In fact, the way John phrased it made me smile, too - but that doesn't change that it is a reproach for me, and looking at the timestamp I didn't feel that it was entirely fair.
Kevin
Quoted 71 lines · Kevin Wolf
Am 22.05.2015 um 10:31 hat Markus Armbruster geschrieben:
Quoted 64 lines
Quoted 37 lines · Kevin Wolf
Am 21.05.2015 um 23:48 hat John Snow geschrieben:
Quoted 36 lines · Markus Armbruster
Quoted 33 lines
Quoted 31 lines · John Snow
Quoted 25 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Quoted 2 lines
Reviewed-by Eric Blake <[email protected]>I'm not actually sure whose tree this should go in. Markus's, perhaps?
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
("ping")
I guess the case for "Block layer core" (Kevin) is at least as strong as the case for "QAPI" (me). Kevin, what do you think?
I think
bdrv_query_dirty_bitmaps()really belongs intoblock/qapi.c, which is yours anyway. So it's either you as the QAPI maintainer or you as the block submaintainer.s/the block submaintainer/the newly minted block submaintainer/
Quoted 14 lines
But if you think otherwise, I can consider it.
Quoted 2 lines
His silence says "Markus, can you please do it? I discovered today that I don't care about this patch."
I'm sorry, John, but you didn't CC me, you didn't CC qemu-block, you didn't CC anyone. I only had a chance to know about it since Wednesday when Markus forwarded it, and I'm not sitting there waiting for new patch emails because I'm bored. Rest assured, I have enough of them.
And then the forwarded email didn't even quote the patch any more, so I couldn't just give a quick reply, but had to find the full email thread in a different folder.
Quoted 5 lines
Quoted 3 lines
If you want to have patches applied quickly, make it easy for the maintainers. You did the exact opposite, so you have no reason to complain.
On the other hand, his "complaining" made me smile, which I appreciate :)
Quoted 9 lines
Drom secht mr's jô em Guada. ;-)
I'm sorry if my reply reads a bit too harsh, it's not meant like that. In fact, the way John phrased it made me smile, too - but that doesn't change that it is a reproach for me, and looking at the timestamp I didn't feel that it was entirely fair.
Kevin
Yes, sorry again. I will try to choose my jokes a little more carefully in the future. I want to make people laugh, but not at the expense of anyone's integrity.
--John Snow
Quoted 60 lines · Kevin Wolf
Am 21.05.2015 um 23:48 hat John Snow geschrieben:
Quoted 34 lines · Markus Armbruster
Quoted 31 lines
Quoted 29 lines · John Snow
Quoted 27 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Reviewed-by Eric Blake <[email protected]>Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
I'm not actually sure whose tree this should go in. Markus's, perhaps?
("ping")
I guess the case for "Block layer core" (Kevin) is at least as strong as the case for "QAPI" (me). Kevin, what do you think?
I think
bdrv_query_dirty_bitmaps()really belongs intoblock/qapi.c, which is yours anyway. So it's either you as the QAPI maintainer or you as the block submaintainer.But if you think otherwise, I can consider it.
Quoted 2 lines
His silence says "Markus, can you please do it? I discovered today that I don't care about this patch."
I'm sorry, John, but you didn't CC me, you didn't CC qemu-block, you didn't CC anyone. I only had a chance to know about it since Wednesday when Markus forwarded it, and I'm not sitting there waiting for new patch emails because I'm bored. Rest assured, I have enough of them.
And then the forwarded email didn't even quote the patch any more, so I couldn't just give a quick reply, but had to find the full email thread in a different folder.
Quoted 6 lines
If you want to have patches applied quickly, make it easy for the maintainers. You did the exact opposite, so you have no reason to complain.
Kevin
Sorry, I didn't mean it to come across that way. I wasn't complaining, I just figured that it wasn't on your radar and decided to ping Markus again.
My apologies for making it seem like I was being critical of your response times, that wasn't my intent. I figured it got lost in the shuffle and just wanted to prod Markus to take it into his QAPI tree.
This patch isn't /that/ important, so I promise I wasn't being impatient, just a miss on being funny.
--js
Markus Armbruster
Reviewed-by Eric BlakeQuoted 25 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)
Quoted 1 line
Reviewed-by Eric Blake <[email protected]>
Patch does two things:
1. Convert status from bool frozen to enum.
2. Add new status 'disabled'.
I would've done this separately, but it's no big deal. But I think we should spell it out in the commit message.
What about:
qapi: add dirty bitmap status
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Management applications may need to know why certain bitmaps are unavailable for various commands, e.g. busy in another operation, busy being migrated, etc.
Right now, all we offer is BlockDirtyInfo's boolean member 'frozen'. Instead of adding more booleans, replace it by an enumeration member 'status' with values 'active' and 'frozen'. Then add new value 'disabled'.
Incompatible change. Fine because the changed part hasn't been released so far.
Quoted 53 lines · Markus Armbruster
Quoted 25 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Quoted 1 line
Reviewed-by Eric Blake <[email protected]>Patch does two things:
1. Convert status from bool frozen to enum.
2. Add new status 'disabled'.I would've done this separately, but it's no big deal. But I think we should spell it out in the commit message.
What about:
qapi: add dirty bitmap status
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Management applications may need to know why certain bitmaps are unavailable for various commands, e.g. busy in another operation, busy being migrated, etc.
Right now, all we offer is BlockDirtyInfo's boolean member 'frozen'. Instead of adding more booleans, replace it by an enumeration member 'status' with values 'active' and 'frozen'. Then add new value 'disabled'.
Quoted 9 lines
Incompatible change. Fine because the changed part hasn't been released so far.
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>Reviewed-by Eric Blake <[email protected]>[Commit message tweaked]Signed-off-by Markus Armbruster <[email protected]>
This is OK by me, Markus. I'll assume that you are OK with or have already made these changes locally, so I won't resend.
Thank you,
--John
Markus Armbruster
AppliedQuoted 64 lines · John Snow
Quoted 53 lines · Markus Armbruster
Quoted 25 lines · Eric Blake
Quoted 9 lines · John Snow
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands
(e.g. busy in another operation, busy being migrated, etc.)
Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all).
Quoted 9 lines
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>--- block.c | 13 ++++++++++++- include/block/block.h | 1 + qapi/block-core.json | 23 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-)Quoted 1 line
Reviewed-by Eric Blake <[email protected]>Patch does two things:
1. Convert status from bool frozen to enum.
2. Add new status 'disabled'.I would've done this separately, but it's no big deal. But I think we should spell it out in the commit message.
What about:
qapi: add dirty bitmap status
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches.
Management applications may need to know why certain bitmaps are unavailable for various commands, e.g. busy in another operation, busy being migrated, etc.
Right now, all we offer is BlockDirtyInfo's boolean member 'frozen'. Instead of adding more booleans, replace it by an enumeration member 'status' with values 'active' and 'frozen'. Then add new value 'disabled'.
Quoted 9 lines
Incompatible change. Fine because the changed part hasn't been released so far.
Suggested-by Eric Blake <[email protected]>Signed-off-by John Snow <[email protected]>Reviewed-by Eric Blake <[email protected]>[Commit message tweaked]Signed-off-by Markus Armbruster <[email protected]>
Quoted 2 lines
This is OK by me, Markus. I'll assume that you are OK with or have already made these changes locally, so I won't resend.
Applied to my block-next branch, thanks!