qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Blue Swirl
Subject: Re: [Qemu-devel] [qemu-devel] [PATCH V2 0/3] [RFC] libqblock draft code v2
Date: Sat, 11 Aug 2012 12:18:11 +0000

On Fri, Aug 10, 2012 at 8:04 AM, Wenchao Xia <address@hidden> 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 <address@hidden>
>> 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).

>
>   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
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]