Re: [Qemu-devel] [PATCH V17 07/10] libqblock: libqblock API design and type defines

[Paolo Bonzini]

Il 05/02/2013 16:00, Wenchao Xia ha scritto:
>>> I guess you mean "It is not safe to use a QBlockImage with context B if
>>> it was created with context A" rather than "No, it shouldn't be
>>> documented"? :)
>>
>> Indeed. :)
>>
>   About the qbcontext, there are two choices:
> 1 discard of qbcontext, adding *error to get error message
> for every API. Need to confirm how to make it work
> with glib's aio-context.
> 2 keep qbcontext, every thread use it to store its thread
> specific data, such as errors, and it is easy to package
> glib's multi-thread contents inside.
>   Which way do you like better?

I agree with Stefan on this.

>>>>>>    These fmt/enum code will always exist in an application to handler
>>>>>> different format, the choice is whether libqblock handle it for
>>>>>> the application. This is more a choice, my idea do it for user. What
>>>>>> is your idea?
>>>>>
>>>>> Always use the strings ("qcow2", "raw", etc).  strcmp() on a 4 or 5-byte
>>>>> string is very cheap and eliminates the need to convert between strings
>>>>> and enums.  Dropping the enum also means one less thing to update when a
>>>>> new image format is added.
>>>>
>>>> Hmm, I've never seen discriminated records with strings.  When working
>>>> on the API with Wenchao, I tried to give it a QAPI flavor.
>>>
>>> My thinking is that converting back and forth between string and enum is
>>> a chore.  If we can reduce boilerplate the library becomes more pleasant
>>> to use.
>>
>> I hope/think in most case the user won't really need to deal with
>> format-specific options...
>
> Exposing the strings to user make libqblock thiner and easier

Not much because you have anyway to convert the option union to
QemuOption/QemuOptionParameter inside libqblock.  The only difference is
whether you use == or strcmp.

> , while
> using enum instead of string, resulting a nicer API.

Yes, I prefer the enum too.

> The best thing I
> can image is let qemu block layer use enum also to eliminate the
> converting later, but effort may be not little. However, enum style
> stands in the right direction, problem is the implemention, but it can
> be hided inside, so I think it is OK.
> 
> 
>>> I guess it's nice to keep the structs QAPI compatible in case we want to
>>> convert to QAPI later.
>>
>> Yeah, or at least share idioms.
>>
>   Any special requirement in struct define to make it compatiable with
> QAPI?

Not looking for strict compatibility, just having similar idioms is
fine.  What you're doing now is okay.

Paolo