| Message ID | 20180925234924.14338-1-jsnow@redhat.com |
|---|---|
| Headers | show |
| Series | dirty-bitmaps: fix QMP command permissions | expand |
26.09.2018 02:49, John Snow wrote: > based on: jsnow/bitmaps staging branch > > This series builds on a previous standalone patch and adjusts > the permission for all (or most) of the QMP bitmap commands. > > John Snow (5): > block/dirty-bitmaps: add user_modifiable status checker > block/dirty-bitmaps: fix merge permissions > block/dirty-bitmaps: allow clear on disabled bitmaps > block/dirty-bitmaps: prohibit enable/disable on locked/frozen bitmaps > block/backup: prohibit backup from using in-use bitmaps > > block/dirty-bitmap.c | 13 +++++--- > blockdev.c | 75 ++++++++++++++++++++------------------------ > include/block/dirty-bitmap.h | 1 + > 3 files changed, 44 insertions(+), 45 deletions(-) > Great! Thank you for clearing this. I contributed a lot to this mess with my qmp-locked( PS: I have a draft patch in my current developments to allow set/reset bits in disabled bitmaps, which is needed to use BdrvDirtyBitmap as a shared named copy-bitmap between fleecing-hook filter and backup job. So "disabled" is actually only for use in bdrv_set_dirty(), to disable automatic bitmap updates on guest writes.
On 09/26/2018 08:19 AM, Vladimir Sementsov-Ogievskiy wrote: > 26.09.2018 02:49, John Snow wrote: >> based on: jsnow/bitmaps staging branch >> >> This series builds on a previous standalone patch and adjusts >> the permission for all (or most) of the QMP bitmap commands. >> >> John Snow (5): >> block/dirty-bitmaps: add user_modifiable status checker >> block/dirty-bitmaps: fix merge permissions >> block/dirty-bitmaps: allow clear on disabled bitmaps >> block/dirty-bitmaps: prohibit enable/disable on locked/frozen bitmaps >> block/backup: prohibit backup from using in-use bitmaps >> >> block/dirty-bitmap.c | 13 +++++--- >> blockdev.c | 75 >> ++++++++++++++++++++------------------------ >> include/block/dirty-bitmap.h | 1 + >> 3 files changed, 44 insertions(+), 45 deletions(-) >> > > Great! Thank you for clearing this. I contributed a lot to this mess > with my qmp-locked( > It happens. I should have caught it too, but I think the ramifications here aren't so bad. > PS: I have a draft patch in my current developments to allow set/reset > bits in disabled bitmaps, which is needed to use BdrvDirtyBitmap as a > shared named copy-bitmap between fleecing-hook filter and backup job. So > "disabled" is actually only for use in bdrv_set_dirty(), to disable > automatic bitmap updates on guest writes. > OK, I'll keep an eye out for the series when it comes. Thanks for your reviews, touched up commit message on #2 to reflect that you had already fixed the problem, and staged to my bitmaps branch: Thanks, applied to my bitmaps tree: https://github.com/jnsnow/qemu/commits/bitmaps https://github.com/jnsnow/qemu.git --js
On 9/25/18 6:49 PM, John Snow wrote: > based on: jsnow/bitmaps staging branch > > This series builds on a previous standalone patch and adjusts > the permission for all (or most) of the QMP bitmap commands. > > John Snow (5): > block/dirty-bitmaps: add user_modifiable status checker > block/dirty-bitmaps: fix merge permissions > block/dirty-bitmaps: allow clear on disabled bitmaps > block/dirty-bitmaps: prohibit enable/disable on locked/frozen bitmaps > block/backup: prohibit backup from using in-use bitmaps > > block/dirty-bitmap.c | 13 +++++--- > blockdev.c | 75 ++++++++++++++++++++------------------------ > include/block/dirty-bitmap.h | 1 + > 3 files changed, 44 insertions(+), 45 deletions(-) Should there be any testsuite coverage of these changes?
On 09/26/2018 10:23 PM, Eric Blake wrote: > On 9/25/18 6:49 PM, John Snow wrote: >> based on: jsnow/bitmaps staging branch >> >> This series builds on a previous standalone patch and adjusts >> the permission for all (or most) of the QMP bitmap commands. >> >> John Snow (5): >> block/dirty-bitmaps: add user_modifiable status checker >> block/dirty-bitmaps: fix merge permissions >> block/dirty-bitmaps: allow clear on disabled bitmaps >> block/dirty-bitmaps: prohibit enable/disable on locked/frozen bitmaps >> block/backup: prohibit backup from using in-use bitmaps >> >> block/dirty-bitmap.c | 13 +++++--- >> blockdev.c | 75 >> ++++++++++++++++++++------------------------ >> include/block/dirty-bitmap.h | 1 + >> 3 files changed, 44 insertions(+), 45 deletions(-) > > Should there be any testsuite coverage of these changes? > Ideally, it's just that 124 is a bit of a mess at the moment. Maybe I'll start a new file for simpler interface testing...