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

Re: [libvirt] [PATCH] qemu: domain I/O asynchronous handling



On 06/14/2011 10:18 AM, Michal Privoznik wrote:
> For virtio disks and interfaces, qemu allows users to enable or disable
> ioeventfd feature. This means, qemu can execute domain code, while
> another thread waits for I/O event. Basically, in some cases it is win,
> in some loss. This feature is available via 'asyncio' attribute in disk
> and interface <driver> element. It accepts 'on' and 'off'. Leaving this
> attribute out defaults to hypervisor decision.
> ---
> this is rework as suggested:
> https://www.redhat.com/archives/libvir-list/2011-May/msg01269.html
> 
>  docs/formatdomain.html.in                          |   34 ++++++++++++-
>  docs/schemas/domain.rng                            |   14 +++++
>  src/conf/domain_conf.c                             |   49 ++++++++++++++++++-
>  src/conf/domain_conf.h                             |   11 ++++
>  src/libvirt_private.syms                           |    2 +
>  src/qemu/qemu_capabilities.c                       |    3 +
>  src/qemu/qemu_capabilities.h                       |    1 +
>  src/qemu/qemu_command.c                            |   23 +++++++++
>  tests/qemuhelptest.c                               |    3 +-
>  .../qemuxml2argv-disk-asyncio.args                 |   11 ++++
>  .../qemuxml2argvdata/qemuxml2argv-disk-asyncio.xml |   51 ++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |    3 +
>  12 files changed, 201 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-asyncio.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-asyncio.xml

ACK.

> +            The optional <code>asyncio</code> attribute allows users to
> +            set <a href='https://patchwork.kernel.org/patch/43390/'>
> +            domain I/O asynchronous handling</a> for disk device.
> +            The default is left to the discretion of the hypervisor.
> +            Accepted values are "on" and "off". Enabling this allows
> +            qemu to execute VM while a separate thread handles I/O.
> +            Typically guests experiencing high system CPU utilization
> +            during I/O will benefit from this. On the other hand,
> +            on overloaded host it could increase guest I/O latency.
> +            <span class="since">Since 0.9.3 (QEMU and KVM only)</span>
> +            <b>In general you should leave this option alone, unless you
> +            are very certain you know what you are doing.</b>

And nice description.

>  
> +static int
> +qemuBuildAsyncIoStr(virBufferPtr buf,
> +                    enum virDomainAsyncIo use,
> +                    virBitmapPtr qemuCaps)
> +{
> +    if (qemuCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_IOEVENTFD)) {
> +        switch (use) {
> +        case VIR_DOMAIN_ASYNC_IO_ON:
> +        case VIR_DOMAIN_ASYNC_IO_OFF:
> +            virBufferAsprintf(buf, ",ioeventfd=%s",
> +                              virDomainAsyncIoTypeToString(use));
> +            break;
> +        default:
> +            /* In other cases (_DEFAULT, _LAST) we don't
> +             * want to add anything */
> +            break;
> +        }
> +    }

I would have avoided the switch statement, and gone with the simpler:

if (use && qemuCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_IOEVENTFD))
    virBufferAsprintf(buf, ",ioeventfd=%s",
                      virDomainAsyncIoTypeToString(use));

which automatically filters out VIR_DOMAIN_ASYNC_IO_DEFAULT, and nothing
in the code should ever be setting the value of 'use' to
VIR_DOMAIN_ASYNC_IO_LAST or greater.  But your approach is correct
as-is, so it's up to you if you want to simplify.

-- 
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]