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