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
> + 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.
> diff --git a/tests/qemuxml2argvdata/aarch64-os-firmware-efi.aarch64-latest.args b/tests/qemuxml2argvdata/aarch64-os-firmware-efi.aarch64-latest.args
> index 3914b6bf6e..ed6e513f3c 100644
> --- a/tests/qemuxml2argvdata/aarch64-os-firmware-efi.aarch64-latest.args
> +++ b/tests/qemuxml2argvdata/aarch64-os-firmware-efi.aarch64-latest.args
> @@ -12,12 +12,20 @@ QEMU_AUDIO_DRV=none \
> -S \
> -object secret,id=masterKey0,format=raw,\
> file=/tmp/lib/domain--1-aarch64test/master-key.aes \
> --machine virt-4.0,accel=tcg,usb=off,dump-guest-core=off,gic-version=2 \
> +-blockdev '{"driver":"file","filename":"/usr/share/AAVMF/AAVMF_CODE.fd",\
> +"node-name":"libvirt-pflash0-storage","auto-read-only":true,\
> +"discard":"unmap"}' \
> +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,\
> +"driver":"raw","file":"libvirt-pflash0-storage"}' \
> +-blockdev '{"driver":"file",\
> +"filename":"/var/lib/libvirt/qemu/nvram/aarch64test_VARS.fd",\
> +"node-name":"libvirt-pflash1-storage","auto-read-only":true,\
> +"discard":"unmap"}' \
> +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,\
> +"driver":"raw","file":"libvirt-pflash1-storage"}' \
> +-machine virt-4.0,accel=tcg,usb=off,dump-guest-core=off,gic-version=2,\
> +pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \
Ah ha, this is where testing for the pflash syntax arrives that I
commented was missing in the previous patch series.
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
