Re: [Qemu-devel] [PATCH 05/11] snapshot: add paired functions for internal snapshot id and name

[Stefan Hajnoczi <stefanha@gmail.com>]

On Thu, Jun 13, 2013 at 01:33:29PM +0800, Wenchao Xia wrote:
> 于 2013-6-11 17:14, Stefan Hajnoczi 写道:
> >On Sat, Jun 08, 2013 at 02:58:01PM +0800, Wenchao Xia wrote:
> >>+
> >>+/* Following function generate id string, used by block driver, such as qcow2.
> >>+   Since no better place to go, place the funtion here for now. */
> >>+void snapshot_id_string_generate(int id, char *id_str, int id_str_size)
> >>+{
> >>+    snprintf(id_str, id_str_size, "%d", id);
> >>+}
> >
> >Since the caller has to manage id, this function doesn't really abstract
> >anything.  I would keep the snprintf() inline, there's only one caller.
> >
>   Its purpose is move the id rules from one implemention(qcow2) into
> generic. If not, it would be a question why snapshot_name_wellformed()
> could make sure name not conflict with ID, then reader find the answer
> in qcow2...
>   I think at least a document is needed about how should implemention
> under ./block generate snapshot ID.

I see your point.  Maybe keep the function.  I was not sure because the
caller still has the int id and has to increment it.  Therefore it
doesn't fully handle id generation.

Stefan