Re: [Qemu-devel] [qemu-devel] [PATCH V2 0/3] [RFC] libqblock draft code v2

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2012-8-11 20:18, Blue Swirl 写道:
> On Fri, Aug 10, 2012 at 8:04 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>     Thanks for your review, sorry I have forgot some fixing you
>> mentioned before, will correct them this time.
>>
>> 于 2012-8-10 1:12, Blue Swirl 写道:
>>
>>> On Thu, Aug 9, 2012 at 10:10 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> wrote:
>>>>
>>>>     This patch intrudce libqblock API, libqblock-test is used as a test
>>>> case.
>>>>
>>>> V2:
>>>>     Using struct_size and [xxx]_new [xxx]_free to keep ABI.
>>>>     Using embbed structure to class format options in image creating.
>>>>     Format specific options were brought to surface.
>>>>     Image format was changed to enum interger instead of string.
>>>>     Some API were renamed.
>>>>     Internel error with errno was saved and with an API caller can get it.
>>>>     ALL flags used were defined in libqblock.h.
>>>>
>>>> Something need discuss:
>>>>     Embbed structure or union could make the model more friendly, but that
>>>> make ABI more difficult, because we need to check every embbed
>>>> structure's
>>>> size and guess compiler's memory arrangement. This means #pragma pack(4)
>>>> or struct_size, offset_next in every structure. Any better way to solve
>>>> it?
>>>> or make every structure a plain one?
>>>
>>>
>>> I'd still use accessor functions instead of structure passing, it
>>> would avoid these problems.
>>>
>>    Do you mean some function like :
>>    CreateOption_Filename_Set(const char *filename)
>>    CreateOption_Format_Set(const char *filename)
>
> Something like this:
> int qb_create_cow(struct QBlockState *qbs, const char *filename,
> size_t virt_size, const char *backing_file, int backing_flag);
> etc. for rest of the formats.
>
> Alternatively, we could have more generic versions like you describe,
> or even more generic still:
>
> void qb_set_property(struct QBlockState *qbs, const char *prop_name,
> const char *prop_value);
>
> so the create sequence (ignoring error handling) would be:
> s = qb_init();
> qb_set_property(s, "filename", "c:\\\\autoexec.bat");
> qb_set_property(s, "format", "cow");
> qb_set_property(s, "virt_size", "10GB");
> //  use defaults for backing_file and backing_flag
> qb_create(s);
>
> Likewise for info structure:
> char *qb_get_property(struct QBlockState *qbs, const char *prop_name);
>
> foo = qb_get_property(s, "format");
> foo = qb_get_property(s, "encrypted");
> foo = qb_get_property(s, "num_backing_files");
> foo = qb_get_property(s, "virt_size");
>
> This would be helpful for the client to display parameters without
> much understanding of their contents:
> char **qb_list_properties(struct QBlockState *qbs); /* returns array
> of property names available for this file, use get_property to
> retrieve their contents */
>
> But the clients can't be completely ignorant of all formats available,
> for example a second UI dialog needs to be added for formats with
> backing files, otherwise it won't be able to access some files at all.
> Maybe by adding type descriptions for each property (type for
> "filename" is "path", for others "string", "bool", "enum" etc).
>
   Thanks. This seems heavy document is needed for that no structure
can indicate what options sub format have, user can only get that info
from returns or documents. I am not sure if this is good, because it
looks more like a object oriented API that C.


>>
>>    It can solve the issue, with a cost of more small APIs in header
>> files that user should use. Not sure if there is a good way to make
>> it more friendly as an object language:
>>    "oc.filename = name;" automatically call CreateOption_Filename_Set,
>> API CreateOption_Filename_Set is invisible to user.
>>
>>
>>
>>> Packing can even introduce a new set of problems since we don't
>>> control the CFLAGS of the client of the library.
>>
>>
>>    indeed, I tried to handle the difference in function qboo_adjust_o2n,
>> found that structure member alignment is hard to deal.
>>
>>
>>>>     AIO is missing, need a prototype.
>>>>
>>>> Wenchao Xia (3):
>>>>     adding libqblock
>>>>     libqblock API
>>>>     libqblock test case
>>>>
>>>>    Makefile         |    3 +
>>>>    block.c          |    2 +-
>>>>    block.h          |    1 +
>>>>    libqblock-test.c |  197 ++++++++++++++++
>>>>    libqblock.c      |  670
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    libqblock.h      |  447 ++++++++++++++++++++++++++++++++++++
>>>>    6 files changed, 1319 insertions(+), 1 deletions(-)
>>>>    create mode 100644 libqblock-test.c
>>>>    create mode 100644 libqblock.c
>>>>    create mode 100644 libqblock.h
>>>>
>>>>
>>>>
>>>
>>
>>
>> --
>> Best Regards
>>
>> Wenchao Xia
>>
>


-- 
Best Regards

Wenchao Xia