Re: [PATCH v2 06/15] block: Fix crash on block_resize on inactive node

2025-02-03 Thread Stefan Hajnoczi
On Thu, Jan 30, 2025 at 06:12:37PM +0100, Kevin Wolf wrote:
> In order for block_resize to fail gracefully on an inactive node instead
> of crashing with an assertion failure in bdrv_co_write_req_prepare()
> (called from bdrv_co_truncate()), we need to check for inactive nodes
> also when they are attached as a root node and make sure that
> BLK_PERM_RESIZE isn't among the permissions allowed for inactive nodes.
> To this effect, don't enumerate the permissions that are incompatible
> with inactive nodes any more, but allow only BLK_PERM_CONSISTENT_READ
> for them.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 7 +++
>  block/block-backend.c | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [PATCH v2 06/15] block: Fix crash on block_resize on inactive node

2025-01-30 Thread Eric Blake
On Thu, Jan 30, 2025 at 06:12:37PM +0100, Kevin Wolf wrote:
> In order for block_resize to fail gracefully on an inactive node instead
> of crashing with an assertion failure in bdrv_co_write_req_prepare()
> (called from bdrv_co_truncate()), we need to check for inactive nodes
> also when they are attached as a root node and make sure that
> BLK_PERM_RESIZE isn't among the permissions allowed for inactive nodes.
> To this effect, don't enumerate the permissions that are incompatible
> with inactive nodes any more, but allow only BLK_PERM_CONSISTENT_READ
> for them.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 7 +++
>  block/block-backend.c | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




[PATCH v2 06/15] block: Fix crash on block_resize on inactive node

2025-01-30 Thread Kevin Wolf
In order for block_resize to fail gracefully on an inactive node instead
of crashing with an assertion failure in bdrv_co_write_req_prepare()
(called from bdrv_co_truncate()), we need to check for inactive nodes
also when they are attached as a root node and make sure that
BLK_PERM_RESIZE isn't among the permissions allowed for inactive nodes.
To this effect, don't enumerate the permissions that are incompatible
with inactive nodes any more, but allow only BLK_PERM_CONSISTENT_READ
for them.

Signed-off-by: Kevin Wolf 
---
 block.c   | 7 +++
 block/block-backend.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index be6b71c314..fa6198bca3 100644
--- a/block.c
+++ b/block.c
@@ -3077,6 +3077,13 @@ bdrv_attach_child_common(BlockDriverState *child_bs,
 assert(child_class->get_parent_desc);
 GLOBAL_STATE_CODE();
 
+if (bdrv_is_inactive(child_bs) && (perm & ~BLK_PERM_CONSISTENT_READ)) {
+g_autofree char *perm_names = bdrv_perm_names(perm);
+error_setg(errp, "Permission '%s' unavailable on inactive node",
+   perm_names);
+return NULL;
+}
+
 new_child = g_new(BdrvChild, 1);
 *new_child = (BdrvChild) {
 .bs = NULL,
diff --git a/block/block-backend.c b/block/block-backend.c
index d093f01f89..cc6f58ae78 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -253,7 +253,7 @@ static bool blk_can_inactivate(BlockBackend *blk)
  * guest.  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))) {
+if (!(blk->perm & ~BLK_PERM_CONSISTENT_READ)) {
 return true;
 }
 
-- 
2.48.1