src/qemu/qemu_command.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
https://bugzilla.redhat.com/show_bug.cgi?id=1524230
Signed-off-by: Roland Schulz <schullzroll@gmail.com>
---
src/qemu/qemu_command.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ff9589f593..284c2709fc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8244,6 +8244,8 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
virQEMUCapsPtr qemuCaps,
unsigned int bootindex)
{
+ virNetDevBandwidthPtr actualBandwidth = virDomainNetGetActualBandwidth(net);
+ virDomainNetType actualType = virDomainNetGetActualType(net);
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
char *chardev = NULL;
char *netdev = NULL;
@@ -8257,6 +8259,19 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
goto cleanup;
}
+ /* Set bandwidth or warn if requested and not supported. */
+ if (actualBandwidth) {
+ if (virNetDevSupportBandwidth(actualType)) {
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
+ !virDomainNetTypeSharesHostView(net)) < 0)
+ goto cleanup;
+ } else {
+ VIR_WARN("setting bandwidth on interfaces of "
+ "type '%s' is not implemented yet",
+ virDomainNetTypeToString(actualType));
+ }
+ }
+
switch ((virDomainChrType)net->data.vhostuser->type) {
case VIR_DOMAIN_CHR_TYPE_UNIX:
if (!(chardev = qemuBuildChrChardevStr(logManager, secManager,
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Sep 10, 2018 at 16:30:59 +0200, Roland Schulz wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1524230 Please describe your change in the commit message. A bugzilla may not give enough reasoning for it. > > Signed-off-by: Roland Schulz <schullzroll@gmail.com> > --- > src/qemu/qemu_command.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index ff9589f593..284c2709fc 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -8244,6 +8244,8 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, > virQEMUCapsPtr qemuCaps, > unsigned int bootindex) > { > + virNetDevBandwidthPtr actualBandwidth = virDomainNetGetActualBandwidth(net); > + virDomainNetType actualType = virDomainNetGetActualType(net); > virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); > char *chardev = NULL; > char *netdev = NULL; > @@ -8257,6 +8259,19 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, > goto cleanup; > } > > + /* Set bandwidth or warn if requested and not supported. */ > + if (actualBandwidth) { > + if (virNetDevSupportBandwidth(actualType)) { > + if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, > + !virDomainNetTypeSharesHostView(net)) < 0) > + goto cleanup; This is a very convoluted dead branch. qemuBuildVhostuserCommandLine gets called only when actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER and virNetDevSupportBandwidth returns false for that value. > + } else { > + VIR_WARN("setting bandwidth on interfaces of " > + "type '%s' is not implemented yet", > + virDomainNetTypeToString(actualType)); Reporting a warning is almost pointless. It only gets logged but the user does not get notified. Is this a hard failure where we can error out? > + } > + } > + > switch ((virDomainChrType)net->data.vhostuser->type) { > case VIR_DOMAIN_CHR_TYPE_UNIX: > if (!(chardev = qemuBuildChrChardevStr(logManager, secManager, > -- > 2.17.1 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 09/10/2018 05:06 PM, Peter Krempa wrote: > On Mon, Sep 10, 2018 at 16:30:59 +0200, Roland Schulz wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=1524230 > > Please describe your change in the commit message. A bugzilla may not > give enough reasoning for it. > >> >> Signed-off-by: Roland Schulz <schullzroll@gmail.com> >> --- >> src/qemu/qemu_command.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index ff9589f593..284c2709fc 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -8244,6 +8244,8 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, >> virQEMUCapsPtr qemuCaps, >> unsigned int bootindex) >> { >> + virNetDevBandwidthPtr actualBandwidth = virDomainNetGetActualBandwidth(net); >> + virDomainNetType actualType = virDomainNetGetActualType(net); >> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); >> char *chardev = NULL; >> char *netdev = NULL; >> @@ -8257,6 +8259,19 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, >> goto cleanup; >> } >> >> + /* Set bandwidth or warn if requested and not supported. */ >> + if (actualBandwidth) { >> + if (virNetDevSupportBandwidth(actualType)) { >> + if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, >> + !virDomainNetTypeSharesHostView(net)) < 0) >> + goto cleanup; > > This is a very convoluted dead branch. > > qemuBuildVhostuserCommandLine gets called only when > actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER and virNetDevSupportBandwidth > returns false for that value. > >> + } else { >> + VIR_WARN("setting bandwidth on interfaces of " >> + "type '%s' is not implemented yet", >> + virDomainNetTypeToString(actualType)); > > Reporting a warning is almost pointless. It only gets logged but the > user does not get notified. Is this a hard failure where we can error > out? I did send a patch for that quite a while ago: https://www.redhat.com/archives/libvir-list/2015-January/msg00171.html and it was decided to just warn users instead of throwing and error and denying starting such domain. I don't mind revisiting that decision though. But we have backward compatibility to bear in mind. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 11, 2018 at 09:10:43 +0200, Michal Privoznik wrote: > On 09/10/2018 05:06 PM, Peter Krempa wrote: > > On Mon, Sep 10, 2018 at 16:30:59 +0200, Roland Schulz wrote: > >> https://bugzilla.redhat.com/show_bug.cgi?id=1524230 > > > > Please describe your change in the commit message. A bugzilla may not > > give enough reasoning for it. > > > >> > >> Signed-off-by: Roland Schulz <schullzroll@gmail.com> > >> --- > >> src/qemu/qemu_command.c | 15 +++++++++++++++ > >> 1 file changed, 15 insertions(+) > >> > >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > >> index ff9589f593..284c2709fc 100644 > >> --- a/src/qemu/qemu_command.c > >> +++ b/src/qemu/qemu_command.c > >> @@ -8244,6 +8244,8 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, > >> virQEMUCapsPtr qemuCaps, > >> unsigned int bootindex) > >> { > >> + virNetDevBandwidthPtr actualBandwidth = virDomainNetGetActualBandwidth(net); > >> + virDomainNetType actualType = virDomainNetGetActualType(net); > >> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); > >> char *chardev = NULL; > >> char *netdev = NULL; > >> @@ -8257,6 +8259,19 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, > >> goto cleanup; > >> } > >> > >> + /* Set bandwidth or warn if requested and not supported. */ > >> + if (actualBandwidth) { > >> + if (virNetDevSupportBandwidth(actualType)) { > >> + if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, > >> + !virDomainNetTypeSharesHostView(net)) < 0) > >> + goto cleanup; > > > > This is a very convoluted dead branch. > > > > qemuBuildVhostuserCommandLine gets called only when > > actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER and virNetDevSupportBandwidth > > returns false for that value. > > > >> + } else { > >> + VIR_WARN("setting bandwidth on interfaces of " > >> + "type '%s' is not implemented yet", > >> + virDomainNetTypeToString(actualType)); > > > > Reporting a warning is almost pointless. It only gets logged but the > > user does not get notified. Is this a hard failure where we can error > > out? > > I did send a patch for that quite a while ago: > > https://www.redhat.com/archives/libvir-list/2015-January/msg00171.html > > and it was decided to just warn users instead of throwing and error and > denying starting such domain. That is the stuff that should have been in the commit message in the first place. > I don't mind revisiting that decision though. But we have backward > compatibility to bear in mind. Actually I don't mind it being a warning if it is justified e.g. by not wanting to break existing configs. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.