[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bd
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap() |
Date: |
Fri, 18 Sep 2015 14:24:21 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 18.09.2015 um 13:45 hat Alberto Garcia geschrieben:
> On Thu 17 Sep 2015 03:48:17 PM CEST, Kevin Wolf wrote:
>
> > +static void swap_feature_fields(BlockDriverState *bs_top,
> > + BlockDriverState *bs_new)
> > +{
> > + BlockDriverState tmp;
> > +
> > + bdrv_move_feature_fields(&tmp, bs_top);
> > + bdrv_move_feature_fields(bs_top, bs_new);
> > + bdrv_move_feature_fields(bs_new, &tmp);
> > +
> > + assert(!bs_new->io_limits_enabled);
> > + if (bs_top->io_limits_enabled) {
> > + bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top));
> > + bdrv_io_limits_disable(bs_top);
> > + }
> > +}
>
> I would use 'if (bs_top->throttle_state != NULL)' instead. That pointer
> is set if and only if the BDS has I/O limits set.
>
> bs->io_limits_enabled can be set to false in order to bypass the limits
> temporarily without removing the BDS's throttling settings (e.g in
> bdrv_read_unthrottled()).
>
> I actually think that we can get rid of io_limits_enabled in several
> places (if not completely), but I will take care of that in a separate
> patch.
>
> The only scenario that could cause problems is if bs->throttle_state is
> set but bs->io_limits_enabled is false when swap_feature_fields() is
> called, but I don't think that's possible in this case.
So something like this? (Specifically, is the assertion right?)
assert(!bs_new->throttle_state);
if (bs_top->throttle_state) {
assert(bs_top->io_limits_enabled);
bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top));
bdrv_io_limits_disable(bs_top);
}
If the assertion doesn't hold true, I guess we would need to call
throttle_group_register() directly for the cases where io_limits_enabled
wasn't true.
Kevin
[Qemu-devel] [PATCH 12/16] block: Introduce parents list, Kevin Wolf, 2015/09/17
[Qemu-devel] [PATCH 15/16] block: Add and use bdrv_replace_in_backing_chain(), Kevin Wolf, 2015/09/17
[Qemu-devel] [PATCH 05/16] block: Convert bs->file to BdrvChild, Kevin Wolf, 2015/09/17