From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56290) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2KQM-0007fm-Qz for qemu-devel@nongnu.org; Thu, 25 Jul 2013 08:13:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V2KQL-000380-3z for qemu-devel@nongnu.org; Thu, 25 Jul 2013 08:13:38 -0400 Received: from plane.gmane.org ([80.91.229.3]:60381) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2KQK-00037s-Qp for qemu-devel@nongnu.org; Thu, 25 Jul 2013 08:13:37 -0400 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1V2KQE-0002Uq-JO for qemu-devel@nongnu.org; Thu, 25 Jul 2013 14:13:30 +0200 Received: from net-2-39-8-162.cust.dsl.vodafone.it ([2.39.8.162]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 25 Jul 2013 14:13:30 +0200 Received: from pbonzini by net-2-39-8-162.cust.dsl.vodafone.it with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 25 Jul 2013 14:13:30 +0200 From: Paolo Bonzini Date: Thu, 25 Jul 2013 14:13:09 +0200 Message-ID: References: <1373992168-26043-1-git-send-email-pbonzini@redhat.com> <1373992168-26043-10-git-send-email-pbonzini@redhat.com> <51E9428D.70507@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit In-Reply-To: <51E9428D.70507@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 09/17] block: introduce bdrv_get_block_status API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Il 19/07/2013 15:43, Eric Blake ha scritto: > On 07/16/2013 10:29 AM, Paolo Bonzini wrote: >> For now, bdrv_get_block_status is just another name for >> bdrv_is_allocated. The next patches will add more flags. >> >> This also touches all block drivers with a mostly mechanical >> rename. The sole exception is cow; because it calls >> cow_co_is_allocated from the read code, we keep that function and >> make cow_co_get_block_status a wrapper. >> >> Signed-off-by: Paolo Bonzini --- v1->v2: >> rebase after vmdk changes > > Reviewed-by: Eric Blake > >> >> @@ -370,7 +376,7 @@ static BlockDriver bdrv_cow = { >> >> .bdrv_read = cow_co_read, .bdrv_write = >> cow_co_write, - .bdrv_co_is_allocated = >> cow_co_is_allocated, + .bdrv_co_get_block_status = >> cow_co_get_block_status, > > Is it worth realigning indentation now that you have a longer > name? > >> +++ b/block/qcow.c @@ -395,7 +395,7 @@ static uint64_t >> get_cluster_offset(BlockDriverState *bs, return cluster_offset; >> } >> >> -static int coroutine_fn qcow_co_is_allocated(BlockDriverState >> *bs, +static int64_t coroutine_fn >> qcow_co_get_block_status(BlockDriverState *bs, int64_t >> sector_num, int nb_sectors, int *pnum) > > Is it worth fixing alignment while you touch this? > >> { BDRVQcowState *s = bs->opaque; @@ -896,7 +896,7 @@ static >> BlockDriver bdrv_qcow = { >> >> .bdrv_co_readv = qcow_co_readv, .bdrv_co_writev >> = qcow_co_writev, - .bdrv_co_is_allocated = >> qcow_co_is_allocated, + .bdrv_co_get_block_status = >> qcow_co_get_block_status, > > Another spot for realignment? > >> +++ b/block/qcow2.c @@ -640,7 +640,7 @@ static int >> qcow2_reopen_prepare(BDRVReopenState *state, return 0; } >> >> -static int coroutine_fn qcow2_co_is_allocated(BlockDriverState >> *bs, +static int64_t coroutine_fn >> qcow2_co_get_block_status(BlockDriverState *bs, int64_t >> sector_num, int nb_sectors, int *pnum) > > alignment? > >> { BDRVQcowState *s = bs->opaque; @@ -1784,7 +1784,7 @@ static >> BlockDriver bdrv_qcow2 = { .bdrv_reopen_prepare = >> qcow2_reopen_prepare, .bdrv_create = qcow2_create, >> .bdrv_has_zero_init = bdrv_has_zero_init_1, - >> .bdrv_co_is_allocated = qcow2_co_is_allocated, + >> .bdrv_co_get_block_status = qcow2_co_get_block_status, >> .bdrv_set_key = qcow2_set_key, .bdrv_make_empty = >> qcow2_make_empty, >> > > wow, this is already an alignment mess before your change > >> +++ b/block/qed.c @@ -667,7 +667,7 @@ static void >> qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, >> size_t l } } >> >> -static int coroutine_fn >> bdrv_qed_co_is_allocated(BlockDriverState *bs, +static int64_t >> coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs, >> int64_t sector_num, int nb_sectors, int *pnum) > > alignment > >> +++ b/block/raw-posix.c @@ -1084,7 +1084,7 @@ static int >> raw_create(const char *filename, QEMUOptionParameter *options) * >> 'nb_sectors' is the max value 'pnum' should be set to. If >> nb_sectors goes * beyond the end of the disk image it will be >> clamped. */ -static int coroutine_fn >> raw_co_is_allocated(BlockDriverState *bs, +static int64_t >> coroutine_fn raw_co_get_block_status(BlockDriverState *bs, >> int64_t sector_num, int nb_sectors, int *pnum) > > and again > >> { @@ -1200,7 +1200,7 @@ static BlockDriver bdrv_file = { >> .bdrv_close = raw_close, .bdrv_create = raw_create, >> .bdrv_has_zero_init = bdrv_has_zero_init_1, - >> .bdrv_co_is_allocated = raw_co_is_allocated, + >> .bdrv_co_get_block_status = raw_co_get_block_status, >> >> .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = >> raw_aio_writev, > > here, nothing was aligned, so you actually met status quo :) > > But that raises a question of consistency between drivers... > >> +++ b/block/raw.c @@ -35,11 +35,11 @@ static void >> raw_close(BlockDriverState *bs) { } >> >> -static int coroutine_fn raw_co_is_allocated(BlockDriverState >> *bs, +static int64_t coroutine_fn >> raw_co_get_block_status(BlockDriverState *bs, int64_t >> sector_num, int nb_sectors, int *pnum) > > alignment. You get the picture; I'll quit pointing it out, since > it doesn't affect semantics. Got it, let's make everything aligned in a separate patch if deemed useful. I didn't want to taint "git blame". Paolo -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJR8RZUAAoJEBvWZb6bTYby2OYP/jDW+qofVnm/sEE0x6j3p/JD 9JlK3kBKIJ3IQLFBWWz7/scNNF4Y0mPZ1U1ylU95F/BZzyntURsCPLGIBrScgx43 L5V1auiHV/aNEWnAD2sHDfkumiS0x8nzq6yEu8go2ldZBQR+2m9gn5wfWTmNs3nD OEhg9Fzz7Lka5KwtGDezcjthKV92eBBEQCJAkx0qa/Jf4BrwQo3P4wCn+4nOJoii veQz809EnQ5VAA1GJ6xIq7iRySLAlbeaG1Km+iB/1gDtD46oRoYOOZGjtvzXQWcY NIVihNsGx9PUYD/uhGhKqxoOJ9Z9Wx6qLadFYi50vFrvBbZ31+Fl5IRrkEgNZb/G eWwPmXJIfqoEyQ4MNdJSQuQhBjd0Y3KCvSBOSi+jQR05osyzvHF5iSWL6SshPocN Ck3blVSxjcZtT2ZhwKxcG0Jk1p9YmnaNGOC8yUxxpgMtJCz7U8G/ki1RaK17VVHU yFXnvvyxfbLl6tpQtPg0MI/XLpyO1BQeJD8C1+q5F16naVczxIB+N/ugKvhWKxNo A7Ou9LgiXq09R6hVrh1Qlz65TbSRwP/j7y07fYj74n14IXwoyt1ybi4RjmM6d0Po vA8uRDb7AVxX4XidFlDe5KTQy3ANGFuhX8Du3JwMvc/KqbqK3rFTT+slDdbJISKJ AeuJHNxM3AYyK53EK4C3 =SxhS -----END PGP SIGNATURE-----