qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: block-dirty-bitmap-add fails with "Operation not supported" in 4.2 r


From: Kevin Wolf
Subject: Re: block-dirty-bitmap-add fails with "Operation not supported" in 4.2 rc5 (worked in 4.1)
Date: Fri, 13 Dec 2019 10:46:07 +0100
User-agent: Mutt/1.12.1 (2019-06-15)

Am 13.12.2019 um 00:12 hat Nir Soffer geschrieben:
> On Thu, Dec 12, 2019 at 11:34 PM Eric Blake <address@hidden> wrote:
> >
> > On 12/12/19 3:04 PM, John Snow wrote:
> > >
> > >
> > > On 12/12/19 1:32 PM, Nir Soffer wrote:
> > >> On Thu, Dec 12, 2019 at 2:04 PM Nir Soffer <address@hidden> wrote:
> > >>>
> > >>> We have a test for full backup flow testing that we can consume the
> > >>> data using our nbd client.
> > >>>
> > >>> The test[0] is starting a full backup flow, based on Eric examples
> > >>> from [1] and [2].
> > >>
> >
> > Note that  my examples in [1] and [2] were written without -blockdev,
> > and included significant contortions to determine the proper node names
> > rather than relying on block names.  Meanwhile, the current code now
> > checked in to upstream libvirt with Peter's edits to my code relies on
> > -blockdev, and does not suffer from the problems caused by drive names.
> >
> > >> Looking at qemu-iotests/256, I switch the order of the actions in the
> > >> transaction
> > >> from:
> > >>
> > >> actions = [
> > >>      {"type": 'blockdev-backup", "data": ...},
> > >>      { "type": "block-dirty-bitmap-add", "data": ...},
> > >> ]
> > >>
> > >> To:
> > >>
> > >>   actions = [
> > >>      { "type": "block-dirty-bitmap-add", "data": ...},
> > >>      {"type": 'blockdev-backup", "data": ...},
> > >> ]
> > >>
> > >> And now the the tests pass with 4.2 rc5.
> > >>
> > >> I'm not sure why it was ok to add the bitmap after starting the block
> > >> job before and now it is not, but it makes sense to me.
> > >>
> > >
> > > This ... might have something to do with filter nodes, I bet.
> >
> > Makes sense as the explanation to me as well.
> >
> > >
> > > ide0-hd0 is the name of a device[*], not the name of a block graph node.
> > > When you use such names for "node" or "node-name" parameters, they
> > > (often) resolve to the root of the graph attached to the device.[**]
> > >
> > > The problem is that the root node of the graph can change, especially
> > > during the runtime of a block job, where filters might be added above
> > > the existing graph.
> > >
> > > Before blockdev-backup prepares itself, "ide0-hd0" has a qcow2 node at
> > > the root of its tree. This node can be used to store persistent bitmaps.
> > >
> > > After blockdev-backup prepares, "ide0-hd0" now has a filter node at its
> > > root -- it no longer implicitly refers to the qcow2 node.
> >
> > And that filter node is new behavior to qemu 4.2, which my original
> > tests did not exercise.
> >
> > >
> > > And that node, you may be surprised to learn, does not support adding
> > > persistent dirty bitmaps. Oops.
> > >
> > > What I recommend is using blockdev configuration top-to-bottom:
> >
> > And that's what Peter's libvirt patches do.
> >
> > >
> > > - Either on the CLI by using -blockdev, or
> > > - At runtime using QMP, see the "create_target" function in iotest 256
> > > for how I create a two-node qcow2 storage graph.
> > >
> > > Using blockdev, you can give the nodes meaningful IDs that never change
> > > out from under you. Then, I believe that your transaction should work in
> > > either order.
> >
> > You'll notice even in my libvirt emails that I used $node rather than
> > $device in my steps, by using query-block to scrape out the generated
> > node name rather than the device name.  Looking at your trace...
> >
> > > 13:55:11 DEBUG   (MainThread) [qmp] Received return: {}
> > > 13:55:11 DEBUG   (MainThread) [qmp] Sending {'execute': 'query-block'}
> > > 13:55:11 DEBUG   (MainThread) [qmp] Received return: [{'io-status':
> > > 'ok', 'device': 'ide0-hd0', 'locked': False, 'removable': False,
> > > 'inserted': {'iops_rd': 0, 'detect_zeroes': 'off', 'image':
> > > {'virtual-size': 5368709120, 'filename':
> > > '/var/tmp/imageio-storage/file-512-ext4-mount/file', 'cluster-size':
> > > 65536, 'format': 'qcow2', 'actual-size': 860160, 'format-specific':
> > > {'type': 'qcow2', 'data': {'compat': '1.1', 'lazy-refcounts': False,
> > > 'refcount-bits': 16, 'corrupt': False}}, 'dirty-flag': False},
> > > 'iops_wr': 0, 'ro': False, 'node-name': '#block126',
> >
> > you had an assigned node name #block126...
> >
> > > 13:55:11 DEBUG   (MainThread) [qmp] Sending {'execute': 'transaction',
> > > 'arguments': {'actions': [{'type': 'blockdev-backup', 'data':
> > > {'device': 'ide0-hd0', 'job-id': 'backup-sda', 'sync': 'none',
> > > 'target': 'backup-sda'}}, {'type': 'block-dirty-bitmap-add', 'data':
> > > {'name': 'check1', 'node': 'ide0-hd0', 'persistent': True}}]}}
> >
> > so try using #block126 instead of 'ide0-hd0' if the transaction remains
> > in this order.  But yes, better yet is to match what libvirt will do and
> > use -blockdev everywhere (upstream libvirt will refuse to use bitmaps
> > with any qemu older than 4.2).
> 
> Thank you for the detailed answer!
> 
> I'm sure blockdev is the greatest thing since sliced bread but it
> looks like overkill for our use case.

If you care about more than the top layer, you need to control the block
graph. And -blockdev is the best way to be sure that the graph looks
what you want it to look, and to assign node names everywhere.

> I'm happy to use the node name generated by qemu.

You're not supposed to. That's why they contain a random element and are
unpredictable. If you do this against all warnings, don't blame us for
any pain you'll incur. The assumption in QEMU is that if you use node
names, you take responsibility to manage the graph.

If you absolutely can't see yourself using -blockdev for now, you can
still set explicit node names with -drive at least.

Kevin




reply via email to

[Prev in Thread] Current Thread [Next in Thread]