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

[Kevin Wolf]

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.

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