Re: [PATCH] qemu: Report warning if QoS is set for vhostuser interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 31, 2018 at 04:47:45PM +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1524230
>
> Because of historical reasons, we are not denying starting a
> domain which has QoS set for unsupported type of device. We do
> report just a warning instead. And even though we perhaps used to
> do so for vhostuser it got lost somewhere. Bring it back.

So, if my blame-fu isn't flawed, then I don't think that vhostuser ever
reported a warning during machine startup, have a look at commits 4a74ccdb92f
and 0bce012d7f0 respectively - there always was a goto cleanup statement.

>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  src/qemu/qemu_command.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6e3ff67660..489e8bc689 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8246,6 +8246,12 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
>          goto cleanup;
>      }
>
> +    if (virDomainNetGetActualBandwidth(net)) {
> +        VIR_WARN("setting bandwidth on interfaces of "
> +                 "type '%s' is not implemented yet",
> +                 virDomainNetTypeToString(VIR_DOMAIN_NET_TYPE_VHOSTUSER));
> +    }
> +

We already do this kind of checking on line 8593 in the same source file. It's
just because of the goto cleanup jump after calling
qemuBuildVhostuserCommandLine that causes the logic to never reach the condition
(Not to mention the code is a mess). I'd suggest moving the already existing
check to the top of the qemuBuildInterfaceCommandLine function instead of
duplicating the code.

Erik

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux