[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: add 208 nbd-server +
From: |
Stefano Panella |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: add 208 nbd-server + blockdev-snapshot-sync test case |
Date: |
Wed, 7 Mar 2018 16:43:12 +0000 |
On Wed, Mar 7, 2018 at 4:16 PM, Stefan Hajnoczi <address@hidden> wrote:
>
> On Wed, Mar 7, 2018 at 1:57 PM, Stefano Panella <address@hidden>
wrote:
> > On Wed, Mar 7, 2018 at 10:55 AM, Stefan Hajnoczi <address@hidden>
wrote:
> >>
> >> On Tue, Mar 6, 2018 at 11:25 PM, Stefano Panella <address@hidden>
> >> wrote:
> >> > I have applied this patch and when I run the following qmp commands
I I
> >> > do
> >> > not see the crash anymore but there is still something wrong because
> >> > only
> >> > /root/a is opened from qemu. It looks like nbd-server-stop is also
> >> > getting
> >> > rid of the nodes added with blockdev-snapshot-sync, therfore is than
not
> >> > possible to do blockdev-del on /root/d because node-name is not found
> >>
> >> Nodes are reference counted. If nothing holds a refcount then the
> >> node is freed.
> > Thanks, that explains the behaviour
> >>
> >> The blockdev-add command holds a reference to the node. The node will
> >> stay alive until blockdev-del, which releases that reference.
> >>
> >> blockdev-snapshot-sync does not hold a reference. Therefore snapshot
> >> nodes are freed once nothing is using them anymore. When the snapshot
> >> node is created, the users of the parent node are updated to point to
> >> the snapshot node instead. This is why the NBD server switches to the
> >> snapshot mode after blockdev-snapshot-sync.
> >>
> >> This is why the snapshot nodes disappear after the NBD server is
> >> stopped while /root/a stays alive.
> >>
> >> I'm not sure if the current blockdev-snapshot-sync behavior is useful.
> >> Perhaps the presence of the "snapshot-node-name" argument should cause
> >> the snapshot node to be treated as monitor-owned, just like
> >> blockdev-add. This would introduce leaks for existing QMP clients
> >> though, so it may be necessary to add yet another argument for this
> >> behavior.
> > that would be nice, I mean to add an extra parameter so it is added to
the
> > monitor
> >>
> >> Anyway, I hope this explains the current behavior. I don't see a
> >> problem with it, but it's something the API users need to be aware of.
> >>
> > Yes, I was not aware of that behaviour, the problem is that many
examples
> > refer
> > to having a device associated with the blockdev-add'd node therefore we
do
> > not
> > see this problem.
> >> If it is a problem for your use case, please explain what you are
trying
> >> to do.
> >>
> > It is not strictly a problem for my usecase but it would be nice to
have the
> > extra param to
> > blockdev-snapshot-sync. That would also fix the problem of running
multiple
> > snap-sync
> > after blockdev-add but before there is any user.
>
> Max Reitz mentioned that the 'blockdev-snapshot' command is preferred
> over 'blockdev-snapshot-sync'. 'blockdev-snapshot-sync' is a legacy
> command that implicitly creates the snapshot node.
>
> The difference is that 'blockdev-snapshot' requires that the user
> first creates the snapshot file (e.g. using qemu-img create), then
> uses 'blockdev-add' to add the snapshot node, and finally uses
> 'blockdev-snapshot' to install the snapshot node.
>
> When 'blockdev-snapshot' is used, the user must delete snapshot nodes
> using 'blockdev-del' since they created using 'blockdev-add'.
>
That is a very usefull info, I was not aware that blockdev-snapshot-sync
was not
recommended. I will try to run some examples with blockdev-snapshot.
In case I want to achieve
A <- B
and I do:
blockdev_add A
create external snapshot with qemu-img B with A as backing image
blockdev_add B
blockdev_snapshot B -> A
What do I need to do to delete A and B?
is it fine to just call blockdev_del B ?
or should I call blockdev_del A as well?
Thanks again for your help!!!
> Stefan
Re: [Qemu-devel] [PATCH 2/2] iotests: add 208 nbd-server + blockdev-snapshot-sync test case, Eric Blake, 2018/03/09
Re: [Qemu-devel] [PATCH 2/2] iotests: add 208 nbd-server + blockdev-snapshot-sync test case, Max Reitz, 2018/03/12
Re: [Qemu-devel] [PATCH 0/2] block: fix nbd-server-stop crash after blockdev-snapshot-sync, Eric Blake, 2018/03/07