[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 3/7] block/qapi: Move 'aio' option to file drive
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH 3/7] block/qapi: Move 'aio' option to file driver |
Date: |
Thu, 22 Sep 2016 08:18:40 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 09/22/2016 05:25 AM, Kevin Wolf wrote:
>>>
>>> +static bool get_aio_option(QemuOpts *opts, int flags, Error **errp)
>>> +{
>>> + const char *aio = qemu_opt_get(opts, "aio");
>>> + if (!aio) {
>>> + return !!(flags & BDRV_O_NATIVE_AIO);
>>> + } else if (!strcmp(aio, "native")) {
>>> + return true;
>>> + } else if (!strcmp(aio, "threads")) {
>>> + return false;
>>> + }
>>> +
>>> + error_setg(errp, "invalid aio option");
>>> + return false;
>>> +}
>>
>> Is there somewhere common to share this, to avoid duplication?
>
> I don't know where I would put it. This is a driver-specific option, so
> it doesn't belong in the generic block layer. It's just that two drivers
> happen to provide the same option currently. If we add another backend
> to raw-posix, raw-win32 wouldn't get the new option, so maybe leaving
> them separate is the best anyway.
>
Fair enough...
> I guess I could do something like this to make the "duplicated" code
> look somewhat smaller, or at least condensed into a single statement:
>
> BlockdevAioOptions aio =
> qapi_enum_parse(BlockdevAioOptions_lookup,
> qemu_opt_get(opts, "aio"),
> BLOCKDEV_AIO_OPTIONS__MAX,
> (flags & BDRV_O_NATIVE_AIO) ?
> BLOCKDEV_AIO_OPTIONS_NATIVE :
> BLOCKDEV_AIO_OPTIONS_THREADS);
> s->use_linux_aio = (aio == BLOCKDEV_AIO_OPTIONS_NATIVE);
>
> Would you consider this an improvement?
Yes, that looks nicer, because it's not hand-rolling the parse. If, in
the future, we do diverge and add a mode in posix that is not available
in win32, that just means we would have two separate QAPI enums, but
keep the qapi_enum_parse() in place for no additional if/else branches.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature