[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command |
Date: |
Mon, 19 Oct 2015 16:15:20 +0200 |
User-agent: |
Notmuch/0.13.2 (http://notmuchmail.org) Emacs/23.2.1 (i486-pc-linux-gnu) |
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
- [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Alberto Garcia, 2015/10/13
- [Qemu-block] [PATCH 1/3] block: Add blk_get_refcnt(), Alberto Garcia, 2015/10/13
- [Qemu-block] [PATCH 2/3] block: Add 'blockdev-del' QMP command, Alberto Garcia, 2015/10/13
- [Qemu-block] [PATCH 3/3] iotests: Add test for the blockdev-del command, Alberto Garcia, 2015/10/13
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Kevin Wolf, 2015/10/19
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command,
Alberto Garcia <=
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Kevin Wolf, 2015/10/19
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Alberto Garcia, 2015/10/20
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Kevin Wolf, 2015/10/22
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Alberto Garcia, 2015/10/22
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Kevin Wolf, 2015/10/22
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Alberto Garcia, 2015/10/22
- Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Kevin Wolf, 2015/10/22
Re: [Qemu-block] [PATCH 0/3] Add 'blockdev-del' command, Max Reitz, 2015/10/19
Re: [Qemu-block] [Qemu-devel] [PATCH 0/3] Add 'blockdev-del' command, Markus Armbruster, 2015/10/21