[libvirt] [PATCH 8/8] qemu: enable blockdev support

[Daniel P. Berrangé]

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 :|