Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command

[Max Reitz]

On 19.10.2015 13:27, Kevin Wolf wrote:
> Am 13.10.2015 um 15:48 hat Alberto Garcia geschrieben:
>> Here's my first attempt at the 'blockdev-del' command.
>>
>> This series goes on top of Max's "BlockBackend and media" v6:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02810.html
>>
>> With Max's code, 'blockdev-add' can now create a BlockDriverState with
>> or without a BlockBackend (depending on whether the 'id' parameter is
>> passed).
>>
>> Therefore, 'blockdev-del' can be used to delete a backend and/or a
>> BDS.
>>
>> The way it works is simple: it receives a single 'device' parameter
>> which can refer to a block backend or a node name.
>>
>>  - If it's a block backend, it will delete it along with its attached
>>    BDS (if any).
>>
>>  - If it's a node name, it will delete the BDS along with the backend
>>    it is attached to (if any).
>>
>>  - If you're wondering whether it's possible to delete the BDS but not
>>    the backend it is attached to, there is already eject or
>>    blockdev-remove-medium for that.
>>
>> If either the backend or the BDS are being used (refcount > 1, or if
>> the BDS has any parents) the command fails.
> 
> I've been thinking a bit about the creation and deletion of
> BlockBackends a bit last week, and honestly it still feels a bit messy
> (or maybe I just don't fully understand it yet). Anyway, the cover
> letter of this series seems to be a good place for trying to write it
> down.
> 
> So we seem to have two criteria to distinguish BDSes:
> 
> 1. Does/Doesn't have a BlockBackend on top.
>    In the future, multiple BlockBackends are possible, too.
> 
> 2. Has/Hasn't been created explicitly by the user.
>    Only if it has, the monitor owns a reference.
> 
> In addition, we also have BlockBackends without a BDS attached. All BBs
> are explicitly created by the user. This leads us to the following
> cases:
> 
> 1. BB without BDS
> 2. BB attached to BDS without monitor reference
> 3. BB attached to BDS with monitor reference
> 4. BDS without BB and without monitor reference
> 5. BDS without BB and with monitor reference
> 
> The question I've been thinking about for each of the cases is: Can we
> create this with blockdev-add today, and what should blockdev-del do?
> Another interesting question is: Can we move between the cases, e.g. by
> adding a monitor reference retroactively, or do we even want to do that?
> 
> Let me start with the first two questions for all cases:
> 
> 1. As far as I know, it's currently not possible to directly create a BB
>    without a BDS (or before Max' series with an empty BDS). You have to
>    create a BB with an opened BDS and then eject that. Oops.
> 
>    blockdev-del is easy: It deletes the BB and nothing else.
> 
> 2. This is the normal case, easy to produce with blockdev-add.
>    The two options for deleting something are removing the BDS while
>    keeping the BB (this is already covered by 'eject') and dropping the
>    BB (probably makes sense to use 'blockdev-del' here). There is no
>    monitor reference to the BDS anyway, so blockdev-dev can't delete
>    that.
> 
>    Dropping the BB automatically also drops the BDS in simple cases. In
>    more complicated cases where the BDS has got more references later,
>    it might still exist. Then the blockdev-del wasn't the exact opposite
>    operation of blockdev-add.
> 
>    Is this a problem for a user/management tool that wants to keep track
>    of which image files are still in use?
> 
> 3. A BDS with a monitor reference can be created (with some patches) by
>    using blockdev-add with a node-name, but no id. Directly attaching it
>    to a BB is tricky today, even though I'm sure you could do it with
>    some clever use of block jobs... With 1. fixed (i.e. you can create
>    an empty BB), insert-medium should do the job.
> 
>    Here we have even more choices for deleting something: Delete the BB,
>    but leave the BDS alone; delete the BDS and leave an empty BB behind
>    (this is different from eject because eject would drop the connection
>    between BB and BDS, but both would continue to exist!); delete both
>    at once.
> 
>    We had two separate blockdev-add commands for the BDS and the BB, so
>    it makes sense to require two separate blockdev-del commands as well.
>    In order to keep blockdev-add/del symmetrical, we would have to make
>    blockdev-del of a node-name delete the BDS and blockdev-del of an id
>    delete the BB.

And keep in mind that in this case the monitor has two references, one
to the BB and one to the BDS, so in case these are the only "external"
references, the BB will have a refcount of 1 and the BDS a refcount of
2. Therefore, without any magic, you'll need two blockdev-del
invocations anyway, one for the BB and one for the BDS.

As Berto said, with patch 2 as it is, either blockdev-del will fail, and
you need to call remove-medium/eject first, which seems fine to me.

Max

> 4. That's the typical bs->file. Created by nested blockdev-add
>    descriptions. blockdev-del errors out, there is no monitor reference
>    to drop, and there is also no BB that the request could be used for.
> 
> 5. Created by a top-level blockdev-add with node-name and no id (like 3.
>    but without attaching it to a BB afterwards). blockdev-del can only
>    mean deleting the BDS.
> 
> If I compare this with the semantics described in the cover letter, I
> must say that I see some problems, especially with case 3.
> 
> I haven't thought about it enough yet, but it seems to me that we can't
> do the BDS/BB aliasing with blockdev-del, but need to interpret
> node-name as BDS and id as BB. Perhaps we also shouldn't use a single
> 'device' option then, but a union of 'node-name' and 'id' (just like the
> same devices were created in blockdev-add).
> 
> Kevin
>

signature.asc
Description: OpenPGP digital signature