[Qemu-devel] [PATCH v3 0/5] dirty-bitmaps: fix QMP command permissions

John Snow posted 5 patches 5 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
block/dirty-bitmap.c         | 13 +++++---
blockdev.c                   | 75 ++++++++++++++++++++------------------------
include/block/dirty-bitmap.h |  1 +
3 files changed, 44 insertions(+), 45 deletions(-)
[Qemu-devel] [PATCH v3 0/5] dirty-bitmaps: fix QMP command permissions
Posted by John Snow 5 years, 7 months ago
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(-)

-- 
2.14.4


Re: [Qemu-devel] [PATCH v3 0/5] dirty-bitmaps: fix QMP command permissions
Posted by Vladimir Sementsov-Ogievskiy 5 years, 7 months ago
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.

-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH v3 0/5] dirty-bitmaps: fix QMP command permissions
Posted by John Snow 5 years, 7 months ago

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

Re: [Qemu-devel] [PATCH v3 0/5] dirty-bitmaps: fix QMP command permissions
Posted by Eric Blake 5 years, 7 months ago
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?

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

Re: [Qemu-devel] [PATCH v3 0/5] dirty-bitmaps: fix QMP command permissions
Posted by John Snow 5 years, 7 months ago

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...