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

[Paolo Bonzini]

Il 01/02/2013 10:32, Stefan Hajnoczi ha scritto:
>>> What is the relationship between QBlockImage and its context?  Can I
>>> have two contexts A and B like this:
>>>
>>> qb_image_ref(ctx_a, &qbi);
>>> qb_image_unref(ctx_b, &qbi);
>>>
>>> Is this okay?
>>>
>>   it should be OK if block layer is thread safe in the future,
>> a thread should own a context. But caller may need to make sure
>> every context should call ref/unref as pairs.
> 
> Hmm...I still don't understand the relationship between QBlockImage and
> a context.  Is it safe to use a QBlockImage with context B if it was
> created with context A?  This should be documented.

No, it shouldn't.

> It would be simpler if a QBlockImage is associated with a context.  Then
> you can drop the context argument from functions that operate on a
> QBlockImage.

Yes, indeed.

> Either provide the struct definition with padding and allow the
> application to allocate/free it - then you don't need these helper
> functions.

We have so many structs and fields for the various formats, that our
choices are:

1) drown in a sea of accessors

2) rewrite the whole thing in Vala

3) add padding

I very much prefer (3), but (2) is only half joking.

> Or hide the struct and manage allocation/freeing and field access with
> accessor functions.
> 
> If you try to mix these approaches the ABI will break when the library
> changes.

> Kevin: Does the block layer do short reads/writes?

No, it doesn't.

>>>> +/**
>>>> + * qb_formattype2str: translate libqblock format enum type to a string.
>>>> + *
>>>> + * return a pointer to the string, or NULL if type is not supported, and
>>>> + *  returned pointer must NOT be freed.
>>>> + *
>>>> + * @fmt: the format enum type.
>>>> + */
>>>> +QEMU_DLL_PUBLIC
>>>> +const char *qb_formattype2str(QBlockFormat fmt_type);
>>>
>>> Why does the QBlockFormat enum exist?  Is there an issue with using strings?
>>>
>>   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.

Paolo