| Message ID | 20170823134242.12080-3-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08/23/2017 08:42 AM, Fam Zheng wrote: > These two conditions corresponds to mirror job's source and target, s/corresponds to/correspond to a/ [can touch up on pull request] > which need to be allowed as they are part of the non-shared storage > migration workflow: failing to inactivate either will result in a > failure during migration completion. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/block-backend.c | 21 ++++++++++++++++----- > include/sysemu/block-backend.h | 1 + > 2 files changed, 17 insertions(+), 5 deletions(-) > > > - return false; > + /* Inactivating means no more write to the image can be done, even if it's s/write/writes/ > + * guest invisible change. For block job BBs that satisfy this, we can just reads awkwardly. Maybe 'even if it's changes invisible to the guest'? But I can leave your wording if I don't get confirmation. > + * allow it. This is the case for mirror job source, which is required by > + * libvirt non-shared block migration. */ > + if (!(blk->perm & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED))) { > + return true; > + } > + > + return blk->force_allow_inactivate; > }
On Wed, 08/23 09:36, Eric Blake wrote: > On 08/23/2017 08:42 AM, Fam Zheng wrote: > > These two conditions corresponds to mirror job's source and target, > > s/corresponds to/correspond to a/ > > [can touch up on pull request] > > > which need to be allowed as they are part of the non-shared storage > > migration workflow: failing to inactivate either will result in a > > failure during migration completion. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > block/block-backend.c | 21 ++++++++++++++++----- > > include/sysemu/block-backend.h | 1 + > > 2 files changed, 17 insertions(+), 5 deletions(-) > > > > > > - return false; > > + /* Inactivating means no more write to the image can be done, even if it's > > s/write/writes/ > > > + * guest invisible change. For block job BBs that satisfy this, we can just > > reads awkwardly. Maybe 'even if it's changes invisible to the guest'? > But I can leave your wording if I don't get confirmation. Not sure I understand the grammar of "it's changes" but yours must be better. Feel free to update it. Thanks, Fam > > > + * allow it. This is the case for mirror job source, which is required by > > + * libvirt non-shared block migration. */ > > + if (!(blk->perm & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED))) { > > + return true; > > + } > > + > > + return blk->force_allow_inactivate; > > } > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org >
On 08/23/2017 09:47 AM, Fam Zheng wrote: > On Wed, 08/23 09:36, Eric Blake wrote: >> On 08/23/2017 08:42 AM, Fam Zheng wrote: >>> These two conditions corresponds to mirror job's source and target, >> >> s/corresponds to/correspond to a/ >> >> [can touch up on pull request] >> >>> which need to be allowed as they are part of the non-shared storage >>> migration workflow: failing to inactivate either will result in a >>> failure during migration completion. >>> >>> Signed-off-by: Fam Zheng <famz@redhat.com> >>> --- >>> block/block-backend.c | 21 ++++++++++++++++----- >>> include/sysemu/block-backend.h | 1 + >>> 2 files changed, 17 insertions(+), 5 deletions(-) >>> >>> >>> - return false; >>> + /* Inactivating means no more write to the image can be done, even if it's >> >> s/write/writes/ >> >>> + * guest invisible change. For block job BBs that satisfy this, we can just >> >> reads awkwardly. Maybe 'even if it's changes invisible to the guest'? >> But I can leave your wording if I don't get confirmation. > > Not sure I understand the grammar of "it's changes" but yours must be better. > Feel free to update it. Thanks, If it's confusing, we can still do better. I settled on: no more writes to the image can be done, even if those writes would be changes invisible to the guest.
diff --git a/block/block-backend.c b/block/block-backend.c index a3984d2bec..48f5356657 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -70,6 +70,7 @@ struct BlockBackend { int quiesce_counter; VMChangeStateEntry *vmsh; + bool force_allow_inactivate; }; typedef struct BlockBackendAIOCB { @@ -192,17 +193,27 @@ static void blk_root_activate(BdrvChild *child, Error **errp) } } +void blk_set_force_allow_inactivate(BlockBackend *blk) +{ + blk->force_allow_inactivate = true; +} + static bool blk_can_inactivate(BlockBackend *blk) { - /* Only inactivate BlockBackends for guest devices (which are inactive at - * this point because the VM is stopped) and unattached monitor-owned - * BlockBackends. If there is still any other user like a block job, then - * we simply can't inactivate the image. */ + /* If it is a guest device, inactivate is ok. */ if (blk->dev || blk_name(blk)[0]) { return true; } - return false; + /* Inactivating means no more write to the image can be done, even if it's + * guest invisible change. For block job BBs that satisfy this, we can just + * allow it. This is the case for mirror job source, which is required by + * libvirt non-shared block migration. */ + if (!(blk->perm & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED))) { + return true; + } + + return blk->force_allow_inactivate; } static int blk_root_inactivate(BdrvChild *child) diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 4a3730596b..aadc733daf 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -241,5 +241,6 @@ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg); void blk_io_limits_disable(BlockBackend *blk); void blk_io_limits_enable(BlockBackend *blk, const char *group); void blk_io_limits_update_group(BlockBackend *blk, const char *group); +void blk_set_force_allow_inactivate(BlockBackend *blk); #endif
These two conditions corresponds to mirror job's source and target, which need to be allowed as they are part of the non-shared storage migration workflow: failing to inactivate either will result in a failure during migration completion. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/block-backend.c | 21 ++++++++++++++++----- include/sysemu/block-backend.h | 1 + 2 files changed, 17 insertions(+), 5 deletions(-)