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

Re: [libvirt] [PATCHv2 2/2] Add txmode attribute to interface XML for virtio backend



On 02/09/2011 10:14 AM, Laine Stump wrote:
> The resulting difference, according to the qemu developer who added
> the option is:
> 
>     bh makes tx more asynchronous and reduces latency, but potentially
>     causes more processor bandwidth contention since the cpu doing the
>     tx isn't necessarily the cpu where the guest generated the
>     packets.

This sentence (or something derived from it, using our names of
iothread|timer) should be in docs/formatdomain.html.in.

> 
> Changes from v1:
> 
> 1) add error log / abort domain startup if option isn't supported by qemu
> 
> 2) change attribute name from tx_alg to txmode
> 
> 3) change attribute values from bh|timer to iothread|timer

All seem reasonable.

> 
> (The difference in length between full patch and delta diff was small
> enough that I decided to just resend the full patch.)
> 
> 
>  src/conf/domain_conf.c       |   26 +++++++++++++++++++++++++-
>  src/conf/domain_conf.h       |   11 +++++++++++
>  src/qemu/qemu_capabilities.c |    3 +++
>  src/qemu/qemu_capabilities.h |    1 +
>  src/qemu/qemu_command.c      |   27 +++++++++++++++++++++++++++
>  5 files changed, 67 insertions(+), 1 deletions(-)

Incomplete - need a v3 (or at least a follow-up patch) that touches
docs/schemas/domain.rng, adds at least one test of the new attribute in
tests/qemuxml2argvdata/, and documents the issue.

> @@ -1367,6 +1377,7 @@ VIR_ENUM_DECL(virDomainFS)
>  VIR_ENUM_DECL(virDomainFSAccessMode)
>  VIR_ENUM_DECL(virDomainNet)
>  VIR_ENUM_DECL(virDomainNetBackend)
> +VIR_ENUM_DECL(virDomainNetVirtioTxMode)

Also missing two new entries in src/libvirt_private.syms.

> @@ -1104,6 +1105,8 @@ qemuCapsParseDeviceStr(const char *str, unsigned long long *flags)
>          if (strstr(str, "pci-assign.bootindex"))
>              *flags |= QEMUD_CMD_FLAG_PCI_BOOTINDEX;
>      }
> +    if (strstr(str, "virtio-net-pci.tx="))
> +        *flags |= QEMUD_CMD_FLAG_VIRTIO_TX_ALG;

It's a race between your patch and Jirka's bitmap cleanup :)

Is this something that needs to go in 0.8.8, or should we wait until
post-release?

Looks better, but I don't feel comfortable giving this ack without a
completion of the patch series.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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