Re: [Qemu-devel] [PATCH] rbd: Detect rbd image resizes and propagate them

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2758 bytes --]

On 09/15/2017 07:33 AM, Kevin Wolf wrote:
> Am 13.09.2017 um 18:44 hat Adam Wolfe Gordon geschrieben:
>> Register a watcher with rbd so that we get notified when an image is
>> resized. Propagate resizes to parent block devices so that guest devices
>> get resized without user intervention.
>>
>> Signed-off-by: Adam Wolfe Gordon <awg@digitalocean.com>
>> ---

> 
> There's more wrong about this than just making assumptions about the
> graph layout above our own node (which would be wrong enough already).
> 
> Other assumptions that you are making here and that don't hold true
> generally are that there is only one parent node (no reason why the
> image couldn't be used by two different users) and that we always
> inherit from a parent. If this node was created explicitly by the user
> with its own -blockdev (or -drive) option, it doesn't inherit options
> from any of its parents.
> 
> Basically, a block driver shouldn't care about its users and it can't
> access them in a clean way. This is intentional.
> 
>> +    if (parent == NULL) {
>> +        error_report("bs %s does not have parent", bdrv_get_device_or_node_name(bs));
>> +        return;
>> +    }
>> +
>> +    /* Force parents to re-read our size. */
>> +    was_variable_length = bs->drv->has_variable_length;
>> +    bs->drv->has_variable_length = true;
>> +    new_parent_len = bdrv_getlength(parent);
>> +    if (new_parent_len < 0) {
>> +        error_report("getlength failed on parent %s", bdrv_get_device_or_node_name(parent));
>> +        bs->drv->has_variable_length = was_variable_length;
>> +        return;
>> +    }
>> +    bs->drv->has_variable_length = was_variable_length;
>> +
>> +    /* Notify block backends that that we have resized.
>> +       Copied from bdrv_parent_cb_resize. */
>> +    QLIST_FOREACH(c, &parent->parents, next_parent) {
>> +        if (c->role->resize) {
>> +            c->role->resize(c);
>> +        }
>> +    }
> 
> This is the right approach that you should use consistently instead.

Also, we need to fix existing bugs in block.c first; right now,
bdrv_truncate() forgets that refresh_total_sectors() can fail, and then
calls bdrv_dirty_bitmap_truncate() even though that will act on the
wrong length.  If we are going to allow devices to recognize that they
have been externally resized outside of qemu's control, then we need to
make sure that we react to size updates consistently.

I'm trying to fix that [1], but it may interact with what you are trying
to do here.

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03776.html

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]