On Thu, Nov 21, 2019 at 18:32:11 +0000, Daniel Berrange wrote: > On Mon, Nov 18, 2019 at 06:02:08PM +0100, Peter Krempa wrote: > > Now that all pieces are in place (hopefully) let's enable -blockdev. > > > > We base the capability on presence of the fix for 'auto-read-only' on > > files so that blockdev works properly, mandate that qemu supports > > explicit SCSI id strings to avoid ABI regression and that the fix for > > 'savevm' is present so that internal snapshots work. > > IIUC, once we enable this, we are fully committed to blockdev > hereafter.... > > > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > > index 6f23400f95..b5fa0fba7e 100644 > > --- a/src/qemu/qemu_capabilities.c > > +++ b/src/qemu/qemu_capabilities.c > > @@ -4476,13 +4476,15 @@ virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps) > > virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW); > > } > > > > - /* To avoid guest ABI regression, blockdev shall be enabled only when > > - * we are able to pass the custom 'device_id' for SCSI disks and cdroms. */ > > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID)) > > - virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV); > > - > > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES)) > > virQEMUCapsSet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES); > > + > > + /* To avoid guest ABI regression, blockdev shall be enabled only when > > + * we are able to pass the custom 'device_id' for SCSI disks and cdroms. */ > > + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC) && > > + virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID) && > > + virQEMUCapsGet(qemuCaps, QEMU_CAPS_SAVEVM_MONITOR_NODES)) > > .... we must *not* add more capabilities in future to this list of three > that we're checking, as that would cause us to regress in use of blockdev I think we have plenty qemucapabilitiestests and 'latest' xml2argv tests which would catch us turning it off. I can possibly add as follow up a bunch of the test files based on 4.2 capabilities so that we have historical reference if the requirements would change. Luckily blockdev would not cause an ABI regression and theoretically we could enable it e.g. during migration as well. But I chose to go the trditional way. > > > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_BLOCKDEV); > > I don't think that's a problem really - we'll just treat anything else we > find as a normal bug & make an effort to fix it as possible. We dont want > to continually turn off blockdev over & over every time we find a new bug. Definitely. This is a known set of qemu bugs which need to be fixed so that libvirt can use it. As of libvirt's implementation, all features which we supported with pre-blockdev should [*] work on blockdev as well. There is only one caveat and that is if a VM has a SD card as disk, blockdev is disabled for such a VM (sd-cards can't be hotplugged). This is because some non-x86 boards in qemu have sd-card which can't be instantiated via -device. Also qemu's documentation is terrible in this regard so I didn't pursue fixing this yet. [*] I tried my best to test as much as possible and I also got some help from Red Hat's QE and a few colleagues using it prior to being enabled in doing so. I'm very thankful for this as I identified a handful of corner cases which weren't treated properly.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
