On 2/18/20 6:01 AM, Max Reitz wrote:


Is it worth an assert(role) somewhere now that you've converted all
callers to pass at least one role?

Well, as the commit message states, block_job_add_bdrv() in blockjob.c
still passes BdrvChildRole=0 to bdrv_root_attach_child().  So it depends
on what function we’re looking at.

I suppose we could add such an assertion to bdrv_attach_child() because
we could expect all BDSs to pass some role for their children.

OTOH, maybe a BDS has a legitimate reason not to: Maybe it just wants to
take some permissions on some BDS without having any real relationship
to it.  Right now, some block jobs do that, well, except they do so
through the back door of adding the child BDS to the block job object
(which then passes no child role).  So maybe I’d actually rather not add
such an assertion anywhere.

Fair enough - you have more knowledge of which callers remain that still have a legitimate reason to not request a role.

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


Reply via email to