From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57471) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brn0e-0005eE-32 for qemu-devel@nongnu.org; Wed, 05 Oct 2016 10:17:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brn0Y-0005W7-QY for qemu-devel@nongnu.org; Wed, 05 Oct 2016 10:17:22 -0400 Date: Wed, 5 Oct 2016 16:17:08 +0200 From: Kevin Wolf Message-ID: <20161005141708.GD1657@noname.str.redhat.com> References: <1475272849-19990-1-git-send-email-jsnow@redhat.com> <1475272849-19990-6-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1475272849-19990-6-git-send-email-jsnow@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 05/11] blockjobs: split interface into public/private List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: qemu-block@nongnu.org, vsementsov@virtuozzo.com, famz@redhat.com, stefanha@redhat.com, jcody@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org Am 01.10.2016 um 00:00 hat John Snow geschrieben: > To make it a little more obvious which functions are intended to be > public interface and which are intended to be for use only by jobs > themselves, split the interface into "public" and "private" files. > > Convert blockjobs (e.g. block/backup) to using the private interface. > Leave blockdev and others on the public interface. > > Give up and let qemu-img use the internal interface, though it doesn't > strictly need to be using it. > > As a side-effect, hide the BlockJob and BlockJobDriver implementation > from most of the QEMU codebase. > > Signed-off-by: John Snow > --- a/block/replication.c > +++ b/block/replication.c > @@ -15,7 +15,7 @@ > #include "qemu/osdep.h" > #include "qemu-common.h" > #include "block/nbd.h" > -#include "block/blockjob.h" > +#include "block/blockjob_int.h" > #include "block/block_int.h" > #include "block/block_backup.h" > #include "sysemu/block-backend.h" This one is wrong. replication.c is a block job user, not part of the implementation. After removing this hunk, the result still compiles. > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -7,16 +7,15 @@ > #include "qemu/coroutine.h" > #include "block/accounting.h" > #include "block/dirty-bitmap.h" > +#include "block/blockjob.h" > #include "qapi/qmp/qobject.h" > #include "qapi-types.h" > #include "qemu/hbitmap.h" Hm... This header file doesn't need any of the definitions from blockjob.h, so I guess .c files should include it explicitly rather than getting it from here. That would be a separate patch, though. > /* block.c */ > typedef struct BlockDriver BlockDriver; > -typedef struct BlockJob BlockJob; > typedef struct BdrvChild BdrvChild; > typedef struct BdrvChildRole BdrvChildRole; > -typedef struct BlockJobTxn BlockJobTxn; > > typedef struct BlockDriverInfo { > /* in bytes, 0 if irrelevant */ > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -37,7 +37,7 @@ > #include "sysemu/sysemu.h" > #include "sysemu/block-backend.h" > #include "block/block_int.h" > -#include "block/blockjob.h" > +#include "block/blockjob_int.h" > #include "block/qapi.h" > #include "crypto/init.h" > #include "trace/control.h" qemu-img doesn't compile without including blockjob_int.h, but that's just a sign that the split isn't completely right yet. Maybe it needs to use the pulic function block_job_query() to get the information it takes directly from the BlockJob struct. Kevin