On Mon 19 Oct 2015 01:27:45 PM CEST, Kevin Wolf wrote: > 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.
Thanks for the summary! > 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. One single BDS attached to multiple BlockBackends at the same time? What's the use case? > 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 I would say that the rule that we should follow is: if the outcome is not clear for a particular case, then the command fails. We should try not to guess what the user wants. blockdev-del should only delete something when there's only one sane alternative. > 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. I agree. > 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? I would say that if the BDS has additional references then 'blockdev-del' should fail and do nothing (what the current patch does). > 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. This is a tricky case, thanks for pointing it out. I would also go for the conservative option here: if you create a BDS with blockdev-add and then attach it to a BlockBackend, you're adding one additional reference (via blk_insert_bs()). Therefore blockdev-del would fail like in case 2. You need to eject the BDS first in order to decide which one you want to delete. > 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. Correct. > 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. Correct. > 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). Having two separate options could make things more clear, indeed. Berto
