[libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)

Peter Krempa posted 2 patches 4 years, 7 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1568378355.git.pkrempa@redhat.com
src/qemu/qemu_capabilities.c                  |   5 +
src/qemu/qemu_capabilities.h                  |   1 +
.../caps_4.1.0.x86_64.xml                     |   2 +
.../controller-virtio-scsi.x86_64-latest.args |  35 +++--
.../disk-aio.x86_64-latest.args               |  19 ++-
...-backing-chains-noindex.x86_64-latest.args | 145 +++++++++++++++---
.../disk-cache.x86_64-latest.args             |  50 ++++--
.../disk-cdrom-network.x86_64-latest.args     |  32 ++--
.../disk-cdrom-tray.x86_64-latest.args        |  24 ++-
.../disk-cdrom.x86_64-latest.args             |  21 +--
.../disk-copy_on_read.x86_64-latest.args      |  19 ++-
.../disk-detect-zeroes.x86_64-latest.args     |  17 +-
.../disk-error-policy.x86_64-latest.args      |  30 ++--
.../disk-floppy-q35-2_11.x86_64-latest.args   |  14 +-
.../disk-floppy-q35-2_9.x86_64-latest.args    |  14 +-
.../disk-floppy.x86_64-latest.args            |  21 ++-
.../disk-network-gluster.x86_64-latest.args   |  32 ++--
.../disk-network-iscsi.x86_64-latest.args     |  58 ++++---
.../disk-network-nbd.x86_64-latest.args       |  41 +++--
.../disk-network-rbd.x86_64-latest.args       |  67 +++++---
.../disk-network-sheepdog.x86_64-latest.args  |  16 +-
...isk-network-source-auth.x86_64-latest.args |  30 ++--
.../disk-network-tlsx509.x86_64-latest.args   |  64 +++++---
.../disk-readonly-disk.x86_64-latest.args     |  14 +-
.../disk-scsi-device-auto.x86_64-latest.args  |  14 +-
.../disk-scsi.x86_64-latest.args              |  35 +++--
.../disk-shared.x86_64-latest.args            |  36 +++--
...irtio-scsi-reservations.x86_64-latest.args |  20 ++-
.../floppy-drive-fat.x86_64-latest.args       |   7 +-
...egl-headless-rendernode.x86_64-latest.args |   7 +-
.../graphics-egl-headless.x86_64-latest.args  |   7 +-
...threads-virtio-scsi-pci.x86_64-latest.args |  25 ++-
...y-hotplug-nvdimm-access.x86_64-latest.args |   7 +-
...ry-hotplug-nvdimm-align.x86_64-latest.args |   7 +-
...ry-hotplug-nvdimm-label.x86_64-latest.args |   7 +-
...ory-hotplug-nvdimm-pmem.x86_64-latest.args |   7 +-
...hotplug-nvdimm-readonly.x86_64-latest.args |   7 +-
.../memory-hotplug-nvdimm.x86_64-latest.args  |   7 +-
...eo-bochs-display-device.x86_64-latest.args |  10 +-
...virtio-non-transitional.x86_64-latest.args |   7 +-
.../virtio-transitional.x86_64-latest.args    |   7 +-
.../x86_64-pc-graphics.x86_64-latest.args     |   8 +-
.../x86_64-pc-headless.x86_64-latest.args     |   8 +-
.../x86_64-q35-graphics.x86_64-latest.args    |   8 +-
.../x86_64-q35-headless.x86_64-latest.args    |   8 +-
45 files changed, 714 insertions(+), 306 deletions(-)
[libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
Posted by Peter Krempa 4 years, 7 months ago
To my knowledge, everything in libvirt is now prepared to fully use
-blockdev way to configure disks in qemu.

There is one known qemu bug though: Internal snapshots don't work with
-blockdev:

https://bugzilla.redhat.com/show_bug.cgi?id=1658981

Since I can't in good faith ask for merging this patchset yet I'd like
to give it some more testing I'm suggesting that we push it and revert
it during freeze or add a capability check once qemu is fixed.

Any other ideas?

Peter Krempa (2):
  qemu: caps: Add capability for dynamic 'auto-read-only' support for
    files
  qemu: enable blockdev support

 src/qemu/qemu_capabilities.c                  |   5 +
 src/qemu/qemu_capabilities.h                  |   1 +
 .../caps_4.1.0.x86_64.xml                     |   2 +
 .../controller-virtio-scsi.x86_64-latest.args |  35 +++--
 .../disk-aio.x86_64-latest.args               |  19 ++-
 ...-backing-chains-noindex.x86_64-latest.args | 145 +++++++++++++++---
 .../disk-cache.x86_64-latest.args             |  50 ++++--
 .../disk-cdrom-network.x86_64-latest.args     |  32 ++--
 .../disk-cdrom-tray.x86_64-latest.args        |  24 ++-
 .../disk-cdrom.x86_64-latest.args             |  21 +--
 .../disk-copy_on_read.x86_64-latest.args      |  19 ++-
 .../disk-detect-zeroes.x86_64-latest.args     |  17 +-
 .../disk-error-policy.x86_64-latest.args      |  30 ++--
 .../disk-floppy-q35-2_11.x86_64-latest.args   |  14 +-
 .../disk-floppy-q35-2_9.x86_64-latest.args    |  14 +-
 .../disk-floppy.x86_64-latest.args            |  21 ++-
 .../disk-network-gluster.x86_64-latest.args   |  32 ++--
 .../disk-network-iscsi.x86_64-latest.args     |  58 ++++---
 .../disk-network-nbd.x86_64-latest.args       |  41 +++--
 .../disk-network-rbd.x86_64-latest.args       |  67 +++++---
 .../disk-network-sheepdog.x86_64-latest.args  |  16 +-
 ...isk-network-source-auth.x86_64-latest.args |  30 ++--
 .../disk-network-tlsx509.x86_64-latest.args   |  64 +++++---
 .../disk-readonly-disk.x86_64-latest.args     |  14 +-
 .../disk-scsi-device-auto.x86_64-latest.args  |  14 +-
 .../disk-scsi.x86_64-latest.args              |  35 +++--
 .../disk-shared.x86_64-latest.args            |  36 +++--
 ...irtio-scsi-reservations.x86_64-latest.args |  20 ++-
 .../floppy-drive-fat.x86_64-latest.args       |   7 +-
 ...egl-headless-rendernode.x86_64-latest.args |   7 +-
 .../graphics-egl-headless.x86_64-latest.args  |   7 +-
 ...threads-virtio-scsi-pci.x86_64-latest.args |  25 ++-
 ...y-hotplug-nvdimm-access.x86_64-latest.args |   7 +-
 ...ry-hotplug-nvdimm-align.x86_64-latest.args |   7 +-
 ...ry-hotplug-nvdimm-label.x86_64-latest.args |   7 +-
 ...ory-hotplug-nvdimm-pmem.x86_64-latest.args |   7 +-
 ...hotplug-nvdimm-readonly.x86_64-latest.args |   7 +-
 .../memory-hotplug-nvdimm.x86_64-latest.args  |   7 +-
 ...eo-bochs-display-device.x86_64-latest.args |  10 +-
 ...virtio-non-transitional.x86_64-latest.args |   7 +-
 .../virtio-transitional.x86_64-latest.args    |   7 +-
 .../x86_64-pc-graphics.x86_64-latest.args     |   8 +-
 .../x86_64-pc-headless.x86_64-latest.args     |   8 +-
 .../x86_64-q35-graphics.x86_64-latest.args    |   8 +-
 .../x86_64-q35-headless.x86_64-latest.args    |   8 +-
 45 files changed, 714 insertions(+), 306 deletions(-)

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
Posted by Daniel P. Berrangé 4 years, 7 months ago
On Fri, Sep 13, 2019 at 02:43:53PM +0200, Peter Krempa wrote:
> To my knowledge, everything in libvirt is now prepared to fully use
> -blockdev way to configure disks in qemu.
> 
> There is one known qemu bug though: Internal snapshots don't work with
> -blockdev:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1658981

What are the chances of that bug actually getting fixed any time in the
forseeable future ?

savevm has been a pain for a long time since it blocks QEMU and we've
been waiting for it to be fixedd by QEMU devs for at least 7-8 years
now, and I don't see anyone from QEMU block maintainers commenting on
this new BZ to indicate they're going to fix this latest problem.

> Since I can't in good faith ask for merging this patchset yet I'd like
> to give it some more testing I'm suggesting that we push it and revert
> it during freeze or add a capability check once qemu is fixed.

I'd only be in favour of enabling it if there is some clear time frame
on whicht hat bug above is likely to be fixed. I'm guesing it is pretty
unlikely to be fixed before our freeze, so I thinking speculatively
enabling it is premature.

> Any other ideas?

We already have the option you added to allow capabilities to be
force enabled for people to test with, though admittedly that
is going to get fairly limited usage.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
Posted by Peter Krempa 4 years, 7 months ago
(ccing Kevin)

On Mon, Sep 16, 2019 at 10:14:44 +0100, Daniel Berrange wrote:
> On Fri, Sep 13, 2019 at 02:43:53PM +0200, Peter Krempa wrote:
> > To my knowledge, everything in libvirt is now prepared to fully use
> > -blockdev way to configure disks in qemu.
> > 
> > There is one known qemu bug though: Internal snapshots don't work with
> > -blockdev:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1658981
> 
> What are the chances of that bug actually getting fixed any time in the
> forseeable future ?
> 
> savevm has been a pain for a long time since it blocks QEMU and we've
> been waiting for it to be fixedd by QEMU devs for at least 7-8 years
> now, and I don't see anyone from QEMU block maintainers commenting on
> this new BZ to indicate they're going to fix this latest problem.
> 
> > Since I can't in good faith ask for merging this patchset yet I'd like
> > to give it some more testing I'm suggesting that we push it and revert
> > it during freeze or add a capability check once qemu is fixed.
> 
> I'd only be in favour of enabling it if there is some clear time frame
> on whicht hat bug above is likely to be fixed. I'm guesing it is pretty
> unlikely to be fixed before our freeze, so I thinking speculatively
> enabling it is premature.

I'm not sure. Kevin, any idea when we can expect the fix?

Otherwise the only option to enable blockdev would be to just enable it
and blame qemu for internal snapshots not working.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
Posted by Kevin Wolf 4 years, 7 months ago
Am 16.09.2019 um 11:32 hat Peter Krempa geschrieben:
> (ccing Kevin)
> 
> On Mon, Sep 16, 2019 at 10:14:44 +0100, Daniel Berrange wrote:
> > On Fri, Sep 13, 2019 at 02:43:53PM +0200, Peter Krempa wrote:
> > > To my knowledge, everything in libvirt is now prepared to fully use
> > > -blockdev way to configure disks in qemu.
> > > 
> > > There is one known qemu bug though: Internal snapshots don't work with
> > > -blockdev:
> > > 
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1658981
> > 
> > What are the chances of that bug actually getting fixed any time in the
> > forseeable future ?
> > 
> > savevm has been a pain for a long time since it blocks QEMU and we've
> > been waiting for it to be fixedd by QEMU devs for at least 7-8 years
> > now, and I don't see anyone from QEMU block maintainers commenting on
> > this new BZ to indicate they're going to fix this latest problem.
> > 
> > > Since I can't in good faith ask for merging this patchset yet I'd like
> > > to give it some more testing I'm suggesting that we push it and revert
> > > it during freeze or add a capability check once qemu is fixed.
> > 
> > I'd only be in favour of enabling it if there is some clear time frame
> > on whicht hat bug above is likely to be fixed. I'm guesing it is pretty
> > unlikely to be fixed before our freeze, so I thinking speculatively
> > enabling it is premature.
> 
> I'm not sure. Kevin, any idea when we can expect the fix?
> 
> Otherwise the only option to enable blockdev would be to just enable it
> and blame qemu for internal snapshots not working.

The main problem with implementing a fix was that it's unclear what the
desired result even is. After some discussion with Max, I think we'll
try it with restricting snapshotting to everything that is directly
attached to a BlockBackend and monitor-owned nodes that don't have any
parents.

Kevin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
Posted by Peter Krempa 4 years, 7 months ago
On Mon, Sep 16, 2019 at 16:29:01 +0200, Kevin Wolf wrote:
> Am 16.09.2019 um 11:32 hat Peter Krempa geschrieben:

[...]

> > > I'd only be in favour of enabling it if there is some clear time frame
> > > on whicht hat bug above is likely to be fixed. I'm guesing it is pretty
> > > unlikely to be fixed before our freeze, so I thinking speculatively
> > > enabling it is premature.
> > 
> > I'm not sure. Kevin, any idea when we can expect the fix?
> > 
> > Otherwise the only option to enable blockdev would be to just enable it
> > and blame qemu for internal snapshots not working.
> 
> The main problem with implementing a fix was that it's unclear what the
> desired result even is. After some discussion with Max, I think we'll
> try it with restricting snapshotting to everything that is directly
> attached to a BlockBackend and monitor-owned nodes that don't have any
> parents.

Yes that is what I would expect as well, and possibly also consistent
with a 'drive_add with if=none' being used but not the corresponding
-device. (thus no frontend).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
Posted by Martin Kletzander 4 years, 7 months ago
On Fri, Sep 13, 2019 at 02:43:53PM +0200, Peter Krempa wrote:
>To my knowledge, everything in libvirt is now prepared to fully use
>-blockdev way to configure disks in qemu.
>
>There is one known qemu bug though: Internal snapshots don't work with
>-blockdev:
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1658981
>
>Since I can't in good faith ask for merging this patchset yet I'd like
>to give it some more testing I'm suggesting that we push it and revert
>it during freeze or add a capability check once qemu is fixed.
>

I am very much in favour of getting some testing before releasing such big
changes.  Any way of not including this in release tarballs is fine, but we
should also not completely give up on non-blockdev approach until we do actually
release it, if only because by having this in for the whole development period
only to disable it for release would mean that we are releasing something we did
not test at all for a whole cycle.

Yeah, I know the above does not give a clear answer or solution, but that is
because I am ambivalent about it.  But I'm not against having this in git to
test it.

Since the change itself is fairly trivial you can consider it
Reviewed-but-not-tested-by: me =)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
Posted by Peter Krempa 4 years, 7 months ago
On Fri, Sep 13, 2019 at 23:35:47 +0200, Martin Kletzander wrote:
> On Fri, Sep 13, 2019 at 02:43:53PM +0200, Peter Krempa wrote:
> > To my knowledge, everything in libvirt is now prepared to fully use
> > -blockdev way to configure disks in qemu.
> > 
> > There is one known qemu bug though: Internal snapshots don't work with
> > -blockdev:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1658981
> > 
> > Since I can't in good faith ask for merging this patchset yet I'd like
> > to give it some more testing I'm suggesting that we push it and revert
> > it during freeze or add a capability check once qemu is fixed.
> > 
> 
> I am very much in favour of getting some testing before releasing such big
> changes.  Any way of not including this in release tarballs is fine, but we
> should also not completely give up on non-blockdev approach until we do actually
> release it, if only because by having this in for the whole development period
> only to disable it for release would mean that we are releasing something we did
> not test at all for a whole cycle.

At this point it would be more like half of a cycle. Also once we enable
it anyways (even for some future-qemu-only) the testing given will
decrease by time into the almost-bitrot region.

To facilitate some deliberate testing I've added the possibility to
control the qemu capability bits from the qemu namespace:

https://libvirt.org/drvqemu.html#xmlnsfeatures

That allows developers and uses who wish to experiment to deliberately
disable any feature. (Obviously resulting breakage may be considered
unsupported)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
Posted by Martin Kletzander 4 years, 7 months ago
On Sat, Sep 14, 2019 at 10:55:59AM +0200, Peter Krempa wrote:
>On Fri, Sep 13, 2019 at 23:35:47 +0200, Martin Kletzander wrote:
>> On Fri, Sep 13, 2019 at 02:43:53PM +0200, Peter Krempa wrote:
>> > To my knowledge, everything in libvirt is now prepared to fully use
>> > -blockdev way to configure disks in qemu.
>> >
>> > There is one known qemu bug though: Internal snapshots don't work with
>> > -blockdev:
>> >
>> > https://bugzilla.redhat.com/show_bug.cgi?id=1658981
>> >
>> > Since I can't in good faith ask for merging this patchset yet I'd like
>> > to give it some more testing I'm suggesting that we push it and revert
>> > it during freeze or add a capability check once qemu is fixed.
>> >
>>
>> I am very much in favour of getting some testing before releasing such big
>> changes.  Any way of not including this in release tarballs is fine, but we
>> should also not completely give up on non-blockdev approach until we do actually
>> release it, if only because by having this in for the whole development period
>> only to disable it for release would mean that we are releasing something we did
>> not test at all for a whole cycle.
>
>At this point it would be more like half of a cycle. Also once we enable
>it anyways (even for some future-qemu-only) the testing given will
>decrease by time into the almost-bitrot region.
>
>To facilitate some deliberate testing I've added the possibility to
>control the qemu capability bits from the qemu namespace:
>
>https://libvirt.org/drvqemu.html#xmlnsfeatures
>

Still, without enabling it, how many people will actually enable it for their
domains to try it out before we release it.  Maybe when setting it we could do
something along the lines of:

    if (virRandom() < .5 || we_are_in_tests) {
        if (!we_are_in_tests) {
            VIR_INFO("You are a lucky winner! This domain (%s) "
                     "will now use blockdev if possible!",
                     vm->def->name);
        }
        virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
    }


And then revert this particular part before releasing.  Or is that way too
crazy?  I mean the `.5` constant can be changed, but it is closest to gradual
testing as you can get with projects like libvirt.

>That allows developers and uses who wish to experiment to deliberately
>disable any feature. (Obviously resulting breakage may be considered
>unsupported)

does that mark the domain as tainted?  Probably no because it is already visible
from the XML, right?  I'm asking purely from curiosity.

Have a nice weekend!

>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list