[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 21/38] block: Add blk_insert_bs()
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v5 21/38] block: Add blk_insert_bs() |
Date: |
Tue, 22 Sep 2015 17:20:23 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 22.09.2015 16:42, Kevin Wolf wrote:
> Am 18.09.2015 um 17:22 hat Max Reitz geschrieben:
>> This function associates the given BlockDriverState with the given
>> BlockBackend.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> Reviewed-by: Alberto Garcia <address@hidden>
>> ---
>> block/block-backend.c | 16 ++++++++++++++++
>> include/sysemu/block-backend.h | 1 +
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/block/block-backend.c b/block/block-backend.c
>> index 33145f8..652385e 100644
>> --- a/block/block-backend.c
>> +++ b/block/block-backend.c
>> @@ -314,6 +314,22 @@ void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend
>> *blk)
>> }
>>
>> /*
>> + * Associates a new BlockDriverState with @blk.
>> + */
>> +void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
>> +{
>> + if (bs->blk == blk) {
>> + return;
>> + }
>> +
>> + assert(!blk->bs);
>> + assert(!bs->blk);
>
> Why is it useful to allow reconnecting a BDS to a BB it's already
> connected to? I would have expected that we can assert that this is not
> the case.
We can do that, too, there is no use case. It's just that I in principle
don't like making things an error that do make sense and can trivially
be handled gracefully. But I can see people wanting to hit an assertion
as soon as something looks fishy.
So I'm fine either way. I think Eric asked about the same before, so
that makes two against one now. :-)
>> + bdrv_ref(bs);
>> + blk->bs = bs;
>> + bs->blk = blk;
>> +}
>
> My series to remove bdrv_swap() introduces a blk_set_bs() function,
> which looks suspiciously similar to this one, except that it allows
> passing a BB that already had a BDS (it gets unrefed then) and that I
> don't assert that the BDS isn't attached to a BB yet (I should do that).
>
> Do you think that's similar enough to have only one function?
Well, yours looks like blk_remove_bs()+blk_insert_bs() combined. In case
we have called blk_remove_bs() before, it will effectively be the same,
yes. But that difference still bothers me a bit... I'd prefer
implementing blk_set_bs() by calling blk_remove_bs() and then
blk_insert_bs() instead, but since these functions are not available in
your bdrv_swap() series, that would prove rather difficult.
I don't know. Maybe implement it separately for now, and trust that this
series will stay in flight long enough for the bdrv_swap() series to get
merged so I can include a commit cleaning up blk_set_bs() in this series
later on?
Or I can base this series on your bdrv_swap() series. Your call, as I
haven't looked at it yet and thus don't know how long it'll take to get
merged (albeit judging from your series in the past, it won't be too long).
Max
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v5 18/38] block: Make some BB functions fall back to BBRS, (continued)
- [Qemu-devel] [PATCH v5 20/38] block: Prepare remaining BB functions for NULL BDS, Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 21/38] block: Add blk_insert_bs(), Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 22/38] block: Prepare for NULL BDS, Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 23/38] blockdev: Do not create BDS for empty drive, Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 26/38] block: Add blk_remove_bs(), Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 24/38] blockdev: Pull out blockdev option extraction, Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 25/38] blockdev: Allow more options for BB-less BDS tree, Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 27/38] blockdev: Add blockdev-open-tray, Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 28/38] blockdev: Add blockdev-close-tray, Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 29/38] blockdev: Add blockdev-remove-medium, Max Reitz, 2015/09/18
- [Qemu-devel] [PATCH v5 31/38] blockdev: Implement eject with basic operations, Max Reitz, 2015/09/18