| Message ID | 20180925234924.14338-3-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | dirty-bitmaps: fix QMP command permissions | expand |
26.09.2018 02:49, John Snow wrote: > We wish to prohibit merging to read-only bitmaps and frozen bitmaps, > but "disabled" bitmaps only preclude their recording of live, new > information. It does not prohibit them from manual writes at the behest > of the user, as is the case for merge operations. > > Allow the merge to "disabled" bitmaps, > and prohibit merging to "locked" ones. only the second part is here.. > > Reported-by: Eric Blake <eblake@redhat.com> > Signed-off-by: John Snow <jsnow@redhat.com> > --- > block/dirty-bitmap.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index fc10543ab0..53b7d282c4 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -806,9 +806,9 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, > > qemu_mutex_lock(dest->mutex); > > - if (bdrv_dirty_bitmap_frozen(dest)) { > - error_setg(errp, "Bitmap '%s' is frozen and cannot be modified", > - dest->name); > + if (!bdrv_dirty_bitmap_user_modifiable(dest)) { > + error_setg(errp, "Bitmap '%s' is currently in-use by another" > + " operation and cannot be modified", dest->name); > goto out; > } >
26.09.2018 14:55, Vladimir Sementsov-Ogievskiy wrote: > 26.09.2018 02:49, John Snow wrote: >> We wish to prohibit merging to read-only bitmaps and frozen bitmaps, >> but "disabled" bitmaps only preclude their recording of live, new >> information. It does not prohibit them from manual writes at the behest >> of the user, as is the case for merge operations. >> >> Allow the merge to "disabled" bitmaps, >> and prohibit merging to "locked" ones. > > only the second part is here.. Hm, the first one is in first separate patch? With commit message fixed to only second part, of course: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > >> >> Reported-by: Eric Blake <eblake@redhat.com> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> block/dirty-bitmap.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >> index fc10543ab0..53b7d282c4 100644 >> --- a/block/dirty-bitmap.c >> +++ b/block/dirty-bitmap.c >> @@ -806,9 +806,9 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap >> *dest, const BdrvDirtyBitmap *src, >> qemu_mutex_lock(dest->mutex); >> - if (bdrv_dirty_bitmap_frozen(dest)) { >> - error_setg(errp, "Bitmap '%s' is frozen and cannot be >> modified", >> - dest->name); >> + if (!bdrv_dirty_bitmap_user_modifiable(dest)) { >> + error_setg(errp, "Bitmap '%s' is currently in-use by another" >> + " operation and cannot be modified", dest->name); >> goto out; >> } > >
On 09/26/2018 08:08 AM, Vladimir Sementsov-Ogievskiy wrote: > 26.09.2018 14:55, Vladimir Sementsov-Ogievskiy wrote: >> 26.09.2018 02:49, John Snow wrote: >>> We wish to prohibit merging to read-only bitmaps and frozen bitmaps, >>> but "disabled" bitmaps only preclude their recording of live, new >>> information. It does not prohibit them from manual writes at the behest >>> of the user, as is the case for merge operations. >>> >>> Allow the merge to "disabled" bitmaps, >>> and prohibit merging to "locked" ones. >> >> only the second part is here.. > > Hm, the first one is in first separate patch? With commit message fixed > to only second part, of course: > Ah, yeah, I'll make that clearer. Got lost in the patch reordering. Thanks! > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > >> >>> >>> Reported-by: Eric Blake <eblake@redhat.com> >>> Signed-off-by: John Snow <jsnow@redhat.com> >>> --- >>> block/dirty-bitmap.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >>> index fc10543ab0..53b7d282c4 100644 >>> --- a/block/dirty-bitmap.c >>> +++ b/block/dirty-bitmap.c >>> @@ -806,9 +806,9 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap >>> *dest, const BdrvDirtyBitmap *src, >>> qemu_mutex_lock(dest->mutex); >>> - if (bdrv_dirty_bitmap_frozen(dest)) { >>> - error_setg(errp, "Bitmap '%s' is frozen and cannot be >>> modified", >>> - dest->name); >>> + if (!bdrv_dirty_bitmap_user_modifiable(dest)) { >>> + error_setg(errp, "Bitmap '%s' is currently in-use by another" >>> + " operation and cannot be modified", dest->name); >>> goto out; >>> } >> >> > >
On 09/26/2018 08:08 AM, Vladimir Sementsov-Ogievskiy wrote: > 26.09.2018 14:55, Vladimir Sementsov-Ogievskiy wrote: >> 26.09.2018 02:49, John Snow wrote: >>> We wish to prohibit merging to read-only bitmaps and frozen bitmaps, >>> but "disabled" bitmaps only preclude their recording of live, new >>> information. It does not prohibit them from manual writes at the behest >>> of the user, as is the case for merge operations. >>> >>> Allow the merge to "disabled" bitmaps, >>> and prohibit merging to "locked" ones. >> >> only the second part is here.. > > Hm, the first one is in first separate patch? With commit message fixed > to only second part, of course: > > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Ah, you actually changed it yourself in b3024909c4b62a989261c288c797f86b150c43fb, for the merge transaction series -- it just goes unmentioned. When I rebased on top of the staging branch I lost that change as it was already made. I'll update the commit message. --js >> >>> >>> Reported-by: Eric Blake <eblake@redhat.com> >>> Signed-off-by: John Snow <jsnow@redhat.com> >>> --- >>> block/dirty-bitmap.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >>> index fc10543ab0..53b7d282c4 100644 >>> --- a/block/dirty-bitmap.c >>> +++ b/block/dirty-bitmap.c >>> @@ -806,9 +806,9 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap >>> *dest, const BdrvDirtyBitmap *src, >>> qemu_mutex_lock(dest->mutex); >>> - if (bdrv_dirty_bitmap_frozen(dest)) { >>> - error_setg(errp, "Bitmap '%s' is frozen and cannot be >>> modified", >>> - dest->name); >>> + if (!bdrv_dirty_bitmap_user_modifiable(dest)) { >>> + error_setg(errp, "Bitmap '%s' is currently in-use by another" >>> + " operation and cannot be modified", dest->name); >>> goto out; >>> } >> >> > >
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index fc10543ab0..53b7d282c4 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -806,9 +806,9 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, qemu_mutex_lock(dest->mutex); - if (bdrv_dirty_bitmap_frozen(dest)) { - error_setg(errp, "Bitmap '%s' is frozen and cannot be modified", - dest->name); + if (!bdrv_dirty_bitmap_user_modifiable(dest)) { + error_setg(errp, "Bitmap '%s' is currently in-use by another" + " operation and cannot be modified", dest->name); goto out; }
We wish to prohibit merging to read-only bitmaps and frozen bitmaps, but "disabled" bitmaps only preclude their recording of live, new information. It does not prohibit them from manual writes at the behest of the user, as is the case for merge operations. Allow the merge to "disabled" bitmaps, and prohibit merging to "locked" ones. Reported-by: Eric Blake <eblake@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> --- block/dirty-bitmap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)