[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.11 3/5] block: Guard against NULL bs->drv

From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH for-2.11 3/5] block: Guard against NULL bs->drv
Date: Tue, 14 Nov 2017 16:36:32 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

On 2017-11-10 22:46, Eric Blake wrote:
> 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 <address@hidden>
>> Buglink: https://bugs.launchpad.net/qemu/+bug/1728660
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> +++ 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?

Maybe? :-)

Isn't the hidden disk usually a qcow2 file?  As such I guess there can
be corruptions in it that make the qcow2 driver eject it (even though
qemu isn't writing to it).


> But what you have is equally safe, so
> Reviewed-by: Eric Blake <address@hidden>

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]