Re: [PATCH qemu-server 1/3] drive: add helper to detect VirtIO driver ISOs with known issues

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Nicolas Frey <n.frey@proxmox.com> writes:

> the wiki lists version ranges of the VirtIO driver ISO for Windows that
> are known to cause issues. these helpers should be the single source of
> truth in code for checking these known issues.
>
> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
> ---
>  src/PVE/QemuServer/Drive.pm | 61 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>
> diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
> index 17c46f36..9839bb48 100644
> --- a/src/PVE/QemuServer/Drive.pm
> +++ b/src/PVE/QemuServer/Drive.pm
> @@ -13,6 +13,7 @@ use PVE::Storage;
>  use PVE::Storage::Common;
>  use PVE::JSONSchema qw(get_standard_option);
>  
> +use PVE::QemuServer::Helpers;
>  use PVE::QemuServer::Monitor qw(qsd_qmp_peer vm_qmp_peer);
>  
>  use base qw(Exporter);
> @@ -766,6 +767,66 @@ sub drive_is_cdrom {
>      return $drive && $drive->{media} && ($drive->{media} eq 'cdrom');
>  }
>  
> +# version ranges of the VirtIO driver ISO for Windows that are known to cause issues, see
> +# https://pve.proxmox.com/wiki/Windows_VirtIO_Drivers#Known_Issues
> +my $virtio_win_issues = [
> +    { from => [0, 1, 215], to => [0, 1, 262] }, { from => [0, 1, 285], to => [0, 1, 285] },
> +];

Since the warning is always the same now, it might make sense (or be
cleaner) to have an explicit list of all affected versions, e.g.
0.1.{215, 217, 221, 225, 229, 240, 248, 262, 285},

No strong opinion on this, definitively not a blocker.

> +
> +# returns the version of the VirtIO driver ISO referenced by $volid if it is known to cause
> +# issues, undef otherwise
> +sub virtio_win_iso_issue {
> +    my ($volid) = @_;
> +
> +    my @version = ($volid // '') =~ m/virtio-win[_-](\d+)\.(\d+)\.(\d+)/i;
> +    return if !@version;
> +
> +    my $cmp_with = sub {
> +        my ($other) = @_;
> +
> +        return PVE::QemuServer::Helpers::version_cmp(map { ($version[$_], $other->[$_]) }
> +            0 .. 2);
> +    };
> +
> +    for my $issue ($virtio_win_issues->@*) {
> +        return join('.', @version)
> +            if $cmp_with->($issue->{from}) >= 0 && $cmp_with->($issue->{to}) <= 0;
> +    }
> +
> +    return;
> +}
> +
> +# warn if the CD-ROM drive $opt contains a VirtIO driver ISO that is known to cause issues
> +sub warn_about_virtio_win_issues {
> +    my ($opt, $volid) = @_;
> +
> +    my $version = virtio_win_iso_issue($volid);
> +    return if !defined($version);
> +
> +    log_warn("$opt: version $version of the VirtIO drivers for Windows is known to cause issues,"
> +        . " see https://pve.proxmox.com/wiki/Windows_VirtIO_Drivers#Known_Issues");
> +
> +    return;
> +}
> +
> +# warn about every CD-ROM drive in $conf that contains a problematic VirtIO driver ISO
> +sub warn_about_virtio_win_issues_in_config {
> +    my ($conf) = @_;
> +
> +    return if !PVE::QemuServer::Helpers::windows_version($conf->{ostype});
> +
> +    for my $opt (valid_drive_names()) {
> +        next if !defined($conf->{$opt});
> +
> +        my $drive = eval { parse_drive($opt, $conf->{$opt}) };
> +        next if !$drive || !drive_is_cdrom($drive, 1);
> +
> +        warn_about_virtio_win_issues($opt, $drive->{file});
> +    }
> +
> +    return;
> +}
> +
>  sub parse_drive_interface {
>      my ($key) = @_;

-- 
Maximiliano