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: Wenchao Xia
Subject: Re: [Qemu-devel] [qemu-devel] [PATCH V2 0/3] [RFC] libqblock draft code v2
Date: Mon, 13 Aug 2012 19:27:50 +0800
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120713 Thunderbird/14.0

于 2012-8-11 20:18, Blue Swirl 写道:
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).

  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




reply via email to

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