Re: [Qemu-devel] [PATCH v2 6/6] block/dirty-bitmaps: move comment block
On 2/18/19 12:39 PM, Vladimir Sementsov-Ogievskiy wrote: > 14.02.2019 2:23, John Snow wrote: >> Simply move the big status enum comment block to above the status >> function, and document it as being deprecated. The whole confusing >> block can get deleted in three releases time. >> >> Signed-off-by: John Snow > > > Reviewed-by: Vladimir Sementsov-Ogievskiy > > But I think, it's OK to remove it now. It mostly unrelated to code now, so, > is it needed? And it is unrelated to deprecation. As I understand, user-seen > information is DirtyBitmapStatus declaration in qapi, and it is deprecated > and to be removed in three releases. > I decided to keep it because I wanted to document the particulars of what the fields meant internally before we finally remove it some future release, in case we need to change this function around to maintain backwards compatibility. It'll get removed at that point in time. --js
Re: [Qemu-devel] [PATCH v2 6/6] block/dirty-bitmaps: move comment block
14.02.2019 2:23, John Snow wrote:
> Simply move the big status enum comment block to above the status
> function, and document it as being deprecated. The whole confusing
> block can get deleted in three releases time.
>
> Signed-off-by: John Snow
Reviewed-by: Vladimir Sementsov-Ogievskiy
But I think, it's OK to remove it now. It mostly unrelated to code now, so,
is it needed? And it is unrelated to deprecation. As I understand, user-seen
information is DirtyBitmapStatus declaration in qapi, and it is deprecated
and to be removed in three releases.
> ---
> block/dirty-bitmap.c | 36 +++-
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 8ab048385a..fc390cae94 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -28,22 +28,6 @@
> #include "block/block_int.h"
> #include "block/blockjob.h"
>
> -/**
> - * A BdrvDirtyBitmap can be in four possible user-visible states:
> - * (1) Active: successor is NULL, and disabled is false: full r/w mode
> - * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
> - * guest writes are dropped, but monitor writes are possible,
> - * through commands like merge and clear.
> - * (3) Frozen: successor is not NULL.
> - * A frozen bitmap cannot be renamed, deleted, cleared, set,
> - * enabled, merged to, etc. A frozen bitmap can only abdicate()
> - * or reclaim().
> - * In this state, the anonymous successor bitmap may be either
> - * Active and recording writes from the guest (e.g. backup
> jobs),
> - * but it can be Disabled and not recording writes.
> - * (4) Locked: Whether Active or Disabled, the user cannot modify this
> bitmap
> - * in any way from the monitor.
> - */
> struct BdrvDirtyBitmap {
> QemuMutex *mutex;
> HBitmap *bitmap;/* Dirty bitmap implementation */
> @@ -205,7 +189,25 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
> return !bitmap->disabled;
> }
>
> -/* Called with BQL taken. */
> +/**
> + * bdrv_dirty_bitmap_status: This API is now deprecated.
> + * Called with BQL taken.
> + *
> + * A BdrvDirtyBitmap can be in four possible user-visible states:
> + * (1) Active: successor is NULL, and disabled is false: full r/w mode
> + * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
> + * guest writes are dropped, but monitor writes are possible,
> + * through commands like merge and clear.
> + * (3) Frozen: successor is not NULL.
> + * A frozen bitmap cannot be renamed, deleted, cleared, set,
> + * enabled, merged to, etc. A frozen bitmap can only abdicate()
> + * or reclaim().
> + * In this state, the anonymous successor bitmap may be either
> + * Active and recording writes from the guest (e.g. backup
> jobs),
> + * but it can be Disabled and not recording writes.
> + * (4) Locked: Whether Active or Disabled, the user cannot modify this
> bitmap
> + * in any way from the monitor.
> + */
> DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
> {
> if (bdrv_dirty_bitmap_has_successor(bitmap)) {
>
--
Best regards,
Vladimir
Re: [Qemu-devel] [PATCH v2 6/6] block/dirty-bitmaps: move comment block
On 2/13/19 5:23 PM, John Snow wrote: > Simply move the big status enum comment block to above the status > function, and document it as being deprecated. The whole confusing > block can get deleted in three releases time. > > Signed-off-by: John Snow > --- > block/dirty-bitmap.c | 36 +++- > 1 file changed, 19 insertions(+), 17 deletions(-) > > -/* Called with BQL taken. */ > +/** > + * bdrv_dirty_bitmap_status: This API is now deprecated. > + * Called with BQL taken. > + * > + * A BdrvDirtyBitmap can be in four possible user-visible states: > + * (1) Active: successor is NULL, and disabled is false: full r/w mode > + * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode, > + * guest writes are dropped, but monitor writes are possible, > + * through commands like merge and clear. > + * (3) Frozen: successor is not NULL. > + * A frozen bitmap cannot be renamed, deleted, cleared, set, > + * enabled, merged to, etc. A frozen bitmap can only abdicate() > + * or reclaim(). > + * In this state, the anonymous successor bitmap may be either > + * Active and recording writes from the guest (e.g. backup > jobs), > + * but it can be Disabled and not recording writes. Pre-existing due to code motion, but you could improve the grammar with s/but/or/ > + * (4) Locked: Whether Active or Disabled, the user cannot modify this > bitmap > + * in any way from the monitor. > + */ Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v2 6/6] block/dirty-bitmaps: move comment block
Simply move the big status enum comment block to above the status
function, and document it as being deprecated. The whole confusing
block can get deleted in three releases time.
Signed-off-by: John Snow
---
block/dirty-bitmap.c | 36 +++-
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 8ab048385a..fc390cae94 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -28,22 +28,6 @@
#include "block/block_int.h"
#include "block/blockjob.h"
-/**
- * A BdrvDirtyBitmap can be in four possible user-visible states:
- * (1) Active: successor is NULL, and disabled is false: full r/w mode
- * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
- * guest writes are dropped, but monitor writes are possible,
- * through commands like merge and clear.
- * (3) Frozen: successor is not NULL.
- * A frozen bitmap cannot be renamed, deleted, cleared, set,
- * enabled, merged to, etc. A frozen bitmap can only abdicate()
- * or reclaim().
- * In this state, the anonymous successor bitmap may be either
- * Active and recording writes from the guest (e.g. backup jobs),
- * but it can be Disabled and not recording writes.
- * (4) Locked: Whether Active or Disabled, the user cannot modify this bitmap
- * in any way from the monitor.
- */
struct BdrvDirtyBitmap {
QemuMutex *mutex;
HBitmap *bitmap;/* Dirty bitmap implementation */
@@ -205,7 +189,25 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
return !bitmap->disabled;
}
-/* Called with BQL taken. */
+/**
+ * bdrv_dirty_bitmap_status: This API is now deprecated.
+ * Called with BQL taken.
+ *
+ * A BdrvDirtyBitmap can be in four possible user-visible states:
+ * (1) Active: successor is NULL, and disabled is false: full r/w mode
+ * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
+ * guest writes are dropped, but monitor writes are possible,
+ * through commands like merge and clear.
+ * (3) Frozen: successor is not NULL.
+ * A frozen bitmap cannot be renamed, deleted, cleared, set,
+ * enabled, merged to, etc. A frozen bitmap can only abdicate()
+ * or reclaim().
+ * In this state, the anonymous successor bitmap may be either
+ * Active and recording writes from the guest (e.g. backup jobs),
+ * but it can be Disabled and not recording writes.
+ * (4) Locked: Whether Active or Disabled, the user cannot modify this bitmap
+ * in any way from the monitor.
+ */
DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
{
if (bdrv_dirty_bitmap_has_successor(bitmap)) {
--
2.17.2
