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
