On 11/10/2017 02:31 PM, Max Reitz wrote: > We currently do not guard everywhere against a NULL bs->drv where we > should be doing so. Most of the places fixed here just do not care > about that case at all. > > Some care implicitly, e.g. through a prior function call to > bdrv_getlength() which would always fail for an ejected BDS. Add an > assert there to make it more obvious. > > Other places seem to care, but do so insufficiently: Freeing clusters in > a qcow2 image is an error-free operation, but it may leave the image in > an unusable state anyway. Giving qcow2_free_clusters() an error code is > not really viable, it is much easier to note that bs->drv may be NULL > even after a successful driver call. This concerns bdrv_co_flush(), and > the way the check is added to bdrv_co_pdiscard() (in every iteration > instead of only once). > > Finally, some places employ at least an assert(bs->drv); somewhere, that > may be reasonable (such as in the reopen code), but in > bdrv_has_zero_init(), it is definitely not. Returning 0 there in case > of an ejected BDS saves us much headache instead. > > Reported-by: R. Nageswara Sastry <nasas...@in.ibm.com> > Buglink: https://bugs.launchpad.net/qemu/+bug/1728660 > Signed-off-by: Max Reitz <mre...@redhat.com> > ---
> +++ b/block/replication.c
>
> + if (!s->hidden_disk->bs->drv) {
> + error_setg(errp, "Hidden disk %s is ejected",
> + s->hidden_disk->bs->node_name);
> + return;
> + }
How would the hidden disk ever be ejected? Could this be an assert instead?
But what you have is equally safe, so
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
