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

[Kevin Wolf]

Am 22.10.2015 um 13:08 hat Alberto Garcia geschrieben:
> >> I'm currently thinking about d), which tries to maintain the symmetry
> >> with blockdev-add:
> >> 
> >> - blockdev-del would have two parameters, 'id' and 'node-name', and only
> >>   one of them can be set, so you must choose whether you want to delete
> >>   a backend or a BDS.
> >> 
> >> - blockdev-add can either create a backend with a BDS, or a BDS alone,
> >>   so:
> >> 
> >>   - If you created a backend and you try to delete it you can do it
> >>     (along with its BDS) as long as neither the backend nor the BDS are
> >>     being used (no extra references, no parents). This means that the
> >>     operation will fail if there's a BDS that has been created
> >>     separately and manually attached to the the backend.
> >> 
> >>   - If you created a BDS alone and you try to delete it you can do it as
> >>     long as no one else is using it. This would delete the BDS and only
> >>     the BDS (because that's what you create with blockdev-add). If it's
> >>     currently attached to a backend then the operation fails.
> >
> > So this is essentially c) with the modification that no implicit eject
> > happens. Either both BB and BDS go away because the BDS is only
> > referenced by the BB or you get an error.
> 
> Right, I would say you always get an error.
> 
> I'm currently extending the set of tests (I expect to send the updated
> series later today or tomorrow) and most are quite straightforward and
> hopefully helpful to prevent surprises in the future. It's also an
> interesting exercise to test the BlockBackend series by Max.

Awesome. As you know, I love test cases.

> But there's this case that is not so obvious. It involves the new
> 'blockdev-snapshot' command I'm working on:
> 
>   - blockdev-add id=drive0 node-name=node0 file=hd0.qcow2
>   - qemu-img create -f qcow2 -b hd0.qcow2 overlay0.qcow2
>   - blockdev-add node-name=overlay0 file=overlay0.qcow2
>   - blockdev-snapshot node=hd0 overlay=overlay0
> 
> At this point you have drive0 with overlay0 inserted, and hd0 as its
> backing image. All these operation will fail:
> 
>   - blockdev-del id=drive0         because overlay0 has two references
>                                    (monitor and block backend)
>   - blockdev-del node=overlay0     for the same reason
>   - blockdev-del node=hd0          because it's a backing image
> 
> In order to delete all this you need to:
> 
>   - eject device=drive0            overlay0 has one reference left
>   - blockdev-del id=drive0
>   - blockdev-del node=overlay0     this deletes hd0 as well
> 
> Does this make sense, or do we need to rethink the semantics a bit more?

Well, it's consistent with what you described above.

The confusing part might be that you could blockdev-del id=drive0
originally, but after taking the snapshot it doesn't work any more. The
only way I can see to remove this effect is that you always need to
eject the BDS first, even if its only reference is from the BB that is
going to be deleted.

I guess that would be even clearer rules, but of course it also means
that it's a bit more cumbersome to use. If it helps avoiding bugs in
management tools, it might be worth it.

But again, I'd like to hear a libvirt opinion from Eric here.

Kevin