block/dirty-bitmap.c | 13 +++++--- blockdev.c | 75 +++++++++++++++++++----------------------- include/block/dirty-bitmap.h | 1 + migration/block-dirty-bitmap.c | 10 ++---- nbd/server.c | 4 +-- 5 files changed, 48 insertions(+), 55 deletions(-)
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. V4: - Replace "in-use" with "in use" - Replace "user_modifiable" version with "user_locked" - Remove more usages of frozen-and-or-locked in NBD. John Snow (6): block/dirty-bitmaps: add user_locked 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 nbd: forbid use of frozen bitmaps block/dirty-bitmap.c | 13 +++++--- blockdev.c | 75 +++++++++++++++++++----------------------- include/block/dirty-bitmap.h | 1 + migration/block-dirty-bitmap.c | 10 ++---- nbd/server.c | 4 +-- 5 files changed, 48 insertions(+), 55 deletions(-) -- 2.14.4
On 10/02/2018 07:02 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. > > V4: > - Replace "in-use" with "in use" > - Replace "user_modifiable" version with "user_locked" > - Remove more usages of frozen-and-or-locked in NBD. > > John Snow (6): > block/dirty-bitmaps: add user_locked 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 > nbd: forbid use of frozen bitmaps > > block/dirty-bitmap.c | 13 +++++--- > blockdev.c | 75 +++++++++++++++++++----------------------- > include/block/dirty-bitmap.h | 1 + > migration/block-dirty-bitmap.c | 10 ++---- > nbd/server.c | 4 +-- > 5 files changed, 48 insertions(+), 55 deletions(-) > Planning on submitting tests for 3.1, but with the merge queue opened up again I'm pretty keen on getting some stable commit IDs for these so far. --js
On 10/2/18 6:08 PM, John Snow wrote: >> John Snow (6): >> block/dirty-bitmaps: add user_locked 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 >> nbd: forbid use of frozen bitmaps >> > > Planning on submitting tests for 3.1, but with the merge queue opened up > again I'm pretty keen on getting some stable commit IDs for these so far. Food for thought when writing tests: I've just noticed with my progress on libvirt code that for error-recovery purposes, if we decide to make x-block-dirty-bitmap-disable fail on a disabled bitmap (or enable on an enabled map) that it would be nice to have a distinct error code; or even better to just be a silent no-op. That's because when libvirt has the situation: bitmap1 (disabled), bitmap2 (enabled) and wants to delete the point in time between those two, it will call: enable bitmap1 merge bitmap2 into bitmap1 remove bitmap2 Removing a bitmap is not part of 'transaction', and there is no pressing reason for it to be (other than convenience) - which means this is at least 2 commands (if the first two are a transaction) or 3 (if all are done independently). But because it is not a transaction, libvirt can conceivably succeed at reenabling bitmap1 but then fail because the guest shut down before merging bitmap2 into 1. When the guest is restarted, there is no real data loss (bitmap1 and 2 are now both active, which is a little bit less efficient for guest writes than having just one active), so the user can attempt to have libvirt retry the deletion sequence. If asking for bitmap1 to be enabled fails (because it is already enabled), then libvirt will have to add workaround code (either to detect that the error iss different than other failures, or to pre-probe whether the bitmap is already enabled); whereas if the request is a no-op, things are easier from libvirt's point of view. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 10/2/18 6:02 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. > > V4: > - Replace "in-use" with "in use" > - Replace "user_modifiable" version with "user_locked" > - Remove more usages of frozen-and-or-locked in NBD. > > John Snow (6): > block/dirty-bitmaps: add user_locked 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 > nbd: forbid use of frozen bitmaps Just now noticing that our docs are slightly out of sync with these changes: # @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. This state is also reachable when using x-block-dirty-bitmap-disable, which is not an internal operation. We probably also ought to document that this state is a prereq to x-nbd-server-add-bitmap, since... # # @active: The bitmap is actively monitoring for new writes, and can be cleared, # deleted, or used for backup operations. ...active is not valid for that usage. # # @locked: The bitmap is currently in-use by some operation and can not be # cleared, deleted, or used for backup operations. (Since 2.12) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 10/17/2018 02:24 PM, Eric Blake wrote: > On 10/2/18 6:02 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. >> >> V4: >> - Replace "in-use" with "in use" >> - Replace "user_modifiable" version with "user_locked" >> - Remove more usages of frozen-and-or-locked in NBD. >> >> John Snow (6): >> block/dirty-bitmaps: add user_locked 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 >> nbd: forbid use of frozen bitmaps > > Just now noticing that our docs are slightly out of sync with these > changes: > > # @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. > > This state is also reachable when using x-block-dirty-bitmap-disable, > which is not an internal operation. We probably also ought to document > that this state is a prereq to x-nbd-server-add-bitmap, since... > > # > # @active: The bitmap is actively monitoring for new writes, and can be > cleared, > # deleted, or used for backup operations. > > ...active is not valid for that usage. > > # > # @locked: The bitmap is currently in-use by some operation and can not be > # cleared, deleted, or used for backup operations. (Since 2.12) > > Ah, you're right. The rationale on disabled there is a bit wrong. I think active is still correct, but the pull workflow has some extra criteria. I'll give a shot at touching this up. --js
On 10/02/2018 07:02 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. > > V4: > - Replace "in-use" with "in use" > - Replace "user_modifiable" version with "user_locked" > - Remove more usages of frozen-and-or-locked in NBD. > > John Snow (6): > block/dirty-bitmaps: add user_locked 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 > nbd: forbid use of frozen bitmaps > > block/dirty-bitmap.c | 13 +++++--- > blockdev.c | 75 +++++++++++++++++++----------------------- > include/block/dirty-bitmap.h | 1 + > migration/block-dirty-bitmap.c | 10 ++---- > nbd/server.c | 4 +-- > 5 files changed, 48 insertions(+), 55 deletions(-) > Thanks, applied to my bitmaps tree: [with edits suggested by Vladimir]; https://github.com/jnsnow/qemu/commits/bitmaps https://github.com/jnsnow/qemu.git --js
© 2016 - 2024 Red Hat, Inc.