[libvirt] [RFC PATCH] Use disable-modern=on for disk device='lun'

[Daniel P. Berrange]

On Tue, Sep 20, 2016 at 02:21:49PM +0200, Ján Tomko wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1365823
> 
> For virtio-blk, scsi=on has been deprecated in virtio-1,
> so <disk type='block' device='lun'> no longer works with
> with a virtio-blk-pci device with machine types newer than 2.7:
> https://bugzilla.redhat.com/show_bug.cgi?id=1245453
> http://git.qemu.org/?p=qemu.git;a=commitdiff;h=9a4c0e22
> 
> Add disable-modern=on on the QEMU command line to prolong
> the suffering for a while longer.
> ---
> The alternative would be using the QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY
> capability to report an error, instead of waiting until QEMU fails with
> an error, since it's the first commit when the following error should
> be reachable via libvirt-generated command line:
> http://git.qemu.org/?p=qemu.git;a=commitdiff;h=efb8206c

My preference would be for us to come to some conclusion of the
way we will represent disable-modern/disable-legacy in the
XML before we try to fix this device=lun problem.

In general I'm not a fan of silently changing configs behind
the users back. IOW, if we have XML for setting disable-modern
explicitly, then I would not want to see us silently setting
disable-modern ourselves. Leave it upto the app to decide to
set it if they really want to continue using virtio-blk instead
of virtio-scsi.

>  src/qemu/qemu_command.c                            | 12 +++++
>  .../qemuxml2argv-virtio-lun-legacy.args            | 30 +++++++++++++
>  .../qemuxml2argv-virtio-lun-legacy.xml             | 52 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  3 ++
>  4 files changed, 97 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-lun-legacy.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-lun-legacy.xml
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 051a0bc..03dc20e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2075,6 +2075,18 @@ qemuBuildDriveDevStr(const virDomainDef *def,
>              virBufferAddLit(&opt, "virtio-blk-pci");
>              if (disk->iothread)
>                  virBufferAsprintf(&opt, ",iothread=iothread%u", disk->iothread);
> +
> +            /*
> +             * SCSI command passthrough was deprecated in virtio 1.0,
> +             * see https://bugzilla.redhat.com/show_bug.cgi?id=1365823
> +             */
> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SCSI) &&
> +                disk->device == VIR_DOMAIN_DISK_DEVICE_LUN &&
> +                virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY)) {
> +                VIR_WARN("lun type devices are deprecated for virtio-blk-pci devices, "
> +                         "consider using disks on a virtio-scsi controller");

Adding warnings like this is fairly pointless since they've invisible to the
person who actually needs to see them (the app developer), and annoying to
the person who does see them (the host administrator).

> +                virBufferAddLit(&opt, ",disable-modern=on");
> +            }
>          }

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|