Bug 976141 - [RFE] virsh domiftune and attach-interface should support guaranteed minimum bandwidth "floor"
Summary: [RFE] virsh domiftune and attach-interface should support guaranteed minimum ...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Virtualization Tools
Classification: Community
Component: libvirt
Version: unspecified
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
Assignee: Michal Privoznik
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-06-20 03:11 UTC by hongming
Modified: 2016-04-27 01:38 UTC (History)
6 users (show)

Fixed In Version: libvirt-1.2.19
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-09-09 13:20:03 UTC
Embargoed:


Attachments (Terms of Use)

Description hongming 2013-06-20 03:11:56 UTC
Description

The guaranteed minimum bandwidth "floor" has be supported in domain xml since libvirt-1.0.4-1.el7. 
The virsh domiftune and attach-interface support bandwidth setting , they also should support the
"floor"  attribute for complete functionality.

Reference
Bug 837908 - [RFE] Extend libvirt network QoS to support guaranteed minimum bandwidth "floor"

Steps
# rpm -q libvirt
libvirt-1.0.6-1.el7.x86_64

# man virsh

domiftune domain interface-device [[--config] [--live] | [--current]] [--inbound average,peak,burst][--outbound average,peak,burst]
Set or query the domain's network interface's bandwidth parameters.  interface-device can be the interface's target name (<target dev='name'/>), or the MAC address.
If no --inbound or --outbound is specified, this command will query and show the bandwidth settings.Otherwise, it will set the inbound or outbound bandwidth. average,peak,burst is the same as in command attach-interface.


attach-interface domain type source [--target target] [--mac mac] [--script script] [--model model] [--config] [--inbound average,peak,burst] [--outbound average,peak,burst]
Attach a new network interface to the domain.  type can be either network to indicate a physical network device or bridge to indicate a bridge to a device.  source indicates the source device.target allows to indicate the target device in the guest. Names starting with 'vnet' are considered as auto-generated an hence blanked out.  mac allows to specify the MAC address of the network interface.script allows to specify a path to a script handling a bridge instead of the default one.  model allows to specify the model type.  --config indicates the changes will affect the next boot of the domain, for compatibility purposes, --persistent is alias of --config.  inbound and outbound controlthe bandwidth of the interface. peak and burst are optional, so "average,peak", "average,,burst" and "average" are also legal.



Actual result
virsh domiftune and attach-interface lack support for guaranteed minimum bandwidth "floor".


Expect result
 virsh domiftune and attach-interface support guaranteed minimum bandwidth "floor"

Comment 3 Michal Privoznik 2015-09-09 13:20:03 UTC
This is fixed upstream now:

commit 68b2405c6768ec232984222cf950e1ebdefb6b13
Author:     Michal Privoznik <mprivozn>
AuthorDate: Wed Aug 12 10:38:12 2015 +0200
Commit:     Michal Privoznik <mprivozn>
CommitDate: Wed Aug 12 10:57:36 2015 +0200

    cmdAttachInterface: Fully implement @floor support
    
    In my previous commit d7f5c88961b52 I tried to introduce support
    for inbound.floor. But the code change was incomplete. This is
    the change needed to fully enable the feature.
    
    Signed-off-by: Michal Privoznik <mprivozn>

commit f7fba69bd7a6756af75ebfc60da1f8e4e948977e
Author:     Michal Privoznik <mprivozn>
AuthorDate: Wed Aug 12 10:34:39 2015 +0200
Commit:     Michal Privoznik <mprivozn>
CommitDate: Wed Aug 12 10:57:36 2015 +0200

    networkBandwidthGenericChecks: Drop useless check
    
    There's a check right at the beginning of the function that
    shortcuts if the function was called over all NULL arguments.
    However, this was meant just as a fool-proof check so that we
    don't crash if function is used in a bad manner. Anyway, it makes
    Coverity unhappy as it then thinks any of the arguments could be
    NULL. Well, with the current state of the code it can't.
    
    Signed-off-by: Michal Privoznik <mprivozn>

commit ef6b3b625d4af52371542e3de203eaef4001522f
Author:     Michal Privoznik <mprivozn>
AuthorDate: Wed Aug 12 10:25:48 2015 +0200
Commit:     Michal Privoznik <mprivozn>
CommitDate: Wed Aug 12 10:57:36 2015 +0200

    networkBandwidthUpdate: Don't blindly dereference pointers
    
    It may happen that an interface don't have any bandwidth set and
    a new one is to be set. In that case, @ifaceBand will be NULL.
    This will cause troubles later in the code when deciding what to
    do.
    
    Signed-off-by: Michal Privoznik <mprivozn>

commit b044e3257f3da2a326f0b23794619aa121ec0ade
Author:     Michal Privoznik <mprivozn>
AuthorDate: Sat Aug 1 09:47:46 2015 +0200
Commit:     Michal Privoznik <mprivozn>
CommitDate: Tue Aug 11 16:10:32 2015 +0200

    qemu: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR
    
    Well, there are just two places that needs adjustment:
    
    qemuDomainGetInterfaceParameters - to report the @floor
    qemuDomainSetInterfaceParameters - now that the function has been
    fixed, we can allow updating @floor too.
    
    Signed-off-by: Michal Privoznik <mprivozn>

commit d7f5c88961b522165b6a10482170047b494102b1
Author:     Michal Privoznik <mprivozn>
AuthorDate: Sat Aug 1 09:48:04 2015 +0200
Commit:     Michal Privoznik <mprivozn>
CommitDate: Tue Aug 11 16:10:32 2015 +0200

    virsh: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR
    
    We have a function parseRateStr() that parses --inbound and
    --outbound arguments to both attach-interface and domiftune.
    Now that we have all virTypedParams macros needed for QoS,
    lets parse even floor attribute. The extended format for the
    arguments looks like this then:
    
      --inbound average[,peak[,burst[,floor]]]
    
    Signed-off-by: Michal Privoznik <mprivozn>

commit 55d0e859fd54089c85cf7ef5c9158e10ce375914
Author:     Michal Privoznik <mprivozn>
AuthorDate: Sat Aug 1 09:47:07 2015 +0200
Commit:     Michal Privoznik <mprivozn>
CommitDate: Tue Aug 11 16:10:32 2015 +0200

    Introduce VIR_DOMAIN_BANDWIDTH_IN_FLOOR
    
    This macro represents the single missing field we don't expose
    yet within QoS: inbound.floor.
    
    Signed-off-by: Michal Privoznik <mprivozn>

commit 6983d6d2c34499bf2d673d7c9359e7838684ee7f
Author:     Michal Privoznik <mprivozn>
AuthorDate: Sat Aug 1 08:13:54 2015 +0200
Commit:     Michal Privoznik <mprivozn>
CommitDate: Tue Aug 11 16:10:32 2015 +0200

    virsh: Rework parseRateStr
    
    The function is used to parse a tuple delimited by commas into
    virNetDevBandwidth structure. So far only three out of fore
    fields are supported: average, peak and burst. The single missing
    field is floor. Well, the parsing works, but I think we can do
    better. Especially when we will need to parse floor too in very
    close future.
    
    Signed-off-by: Michal Privoznik <mprivozn>

v1.2.18-107-g68b2405


Note You need to log in before you can comment on or make changes to this bug.