qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH V7 05/14] block: add snapshot info query functio


From: Wenchao Xia
Subject: Re: [Qemu-devel] [PATCH V7 05/14] block: add snapshot info query function bdrv_query_snapshot_infolist()
Date: Fri, 01 Mar 2013 09:41:41 +0800
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130215 Thunderbird/17.0.3

于 2013-2-28 23:41, Stefan Hajnoczi 写道:
On Thu, Feb 28, 2013 at 09:53:53AM +0800, Wenchao Xia wrote:
于 2013-2-27 22:26, Markus Armbruster 写道:
Wenchao Xia <address@hidden> writes:

    This patch add function bdrv_query_snapshot_infolist(), which will

adds

return snapshot info of an image in qmp object format. The implementation
code are based on the code moved from qemu-img.c with modification to fit more

implementation is based

for qmp based block layer API.
    To help filter out snapshot info not needed, a call back function is
added in bdrv_query_snapshot_infolist().
    bdrv_can_read_snapshot() should be called before call this function,
to avoid got *errp set unexpectedly.

"avoid getting"

Not sure about "should".  Couldn't a caller safely call this with a
non-snapshot bs argument, and simply deal with the error?

   yes, but in some case this error should be avoided. For eg, in
bdrv_query_image_info(), if a image is not inserted, or it can't
take snapshot, other info should also be returned without error,
so if bdrv_can_read_snapshot() is called before, this kind of
error is avoided, and caller can check if there is other
error. I guess it should be documented as "in some case,
bdrv_can_read_snapshot() should be called."

Is the problem that we cannot test what an Error instance means?  So the
caller cannot test the Error to find out whether the medium is not
inserted.

It's cleaner to keep a -errno return value *plus* have an Error*
argument.  That way QEMU callers can distinguish the error meaning and
we don't have to create a separate function that gets called first to
test very specific conditions that aren't meaningful by themselves.

Stefan

  This is also a solution, but let me check if the function below
can return correct error value now.


--
Best Regards

Wenchao Xia




reply via email to

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