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

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

[Wenchao Xia]

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

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

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

Packing can even introduce a new set of problems since we don't
control the CFLAGS of the client of the library.

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

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

[Wenchao Xia]

    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)

   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

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

[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).

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

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

[Wenchao Xia]

于 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

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

[Blue Swirl]

On Mon, Aug 13, 2012 at 11:27 AM, Wenchao Xia
<xiawenc@linux.vnet.ibm.com> wrote:
> 于 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.

This approach may be a bit over-engineered, but it may be simpler to
tie to an UI.

What do you think of the simple version then:
int qb_create_cow(struct QBlockState *qbs, const char *filename,
size_t virt_size, const char *backing_file, int backing_flag);

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

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

[Wenchao Xia]

于 2012-8-14 4:00, Blue Swirl 写道:
> On Mon, Aug 13, 2012 at 11:27 AM, Wenchao Xia
> <xiawenc@linux.vnet.ibm.com> wrote:
>> 于 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.
>
> This approach may be a bit over-engineered, but it may be simpler to
> tie to an UI.
>
> What do you think of the simple version then:
> int qb_create_cow(struct QBlockState *qbs, const char *filename,
> size_t virt_size, const char *backing_file, int backing_flag);
>
   it is hard to extend more options, if for every format a API is 
needed, then
int qb_create_cow(struct QBlockState *qbs, struct QBlockOptionFmtCow *op)
seems better, while keep struct QBlockOptionFmtCow a plain struct
without embbed struct(using pointer instead).

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


-- 
Best Regards

Wenchao Xia

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

[Blue Swirl]

On Tue, Aug 14, 2012 at 8:52 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> 于 2012-8-14 4:00, Blue Swirl 写道:
>
>> On Mon, Aug 13, 2012 at 11:27 AM, Wenchao Xia
>> <xiawenc@linux.vnet.ibm.com> wrote:
>>>
>>> 于 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.
>>
>>
>> This approach may be a bit over-engineered, but it may be simpler to
>> tie to an UI.
>>
>> What do you think of the simple version then:
>> int qb_create_cow(struct QBlockState *qbs, const char *filename,
>> size_t virt_size, const char *backing_file, int backing_flag);
>>
>   it is hard to extend more options, if for every format a API is needed,
> then
> int qb_create_cow(struct QBlockState *qbs, struct QBlockOptionFmtCow *op)
> seems better, while keep struct QBlockOptionFmtCow a plain struct
> without embbed struct(using pointer instead).

OK,  practically there isn't much difference and if a lot of
parameters are needed, a structure is needed anyway.

>
>
>>>
>>>
>>>
>>>>>
>>>>>     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
>>>
>>
>
>
> --
> Best Regards
>
> Wenchao Xia
>