From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39100) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFqGf-00047q-Tn for qemu-devel@nongnu.org; Wed, 13 Mar 2013 14:19:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UFqGV-0006CD-R5 for qemu-devel@nongnu.org; Wed, 13 Mar 2013 14:19:13 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:42782) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFqGV-0006BB-MJ for qemu-devel@nongnu.org; Wed, 13 Mar 2013 14:19:03 -0400 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 13 Mar 2013 14:19:01 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id AA9DD6E8047 for ; Wed, 13 Mar 2013 14:18:45 -0400 (EDT) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r2DIIllQ282092 for ; Wed, 13 Mar 2013 14:18:47 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r2DIIlX1029771 for ; Wed, 13 Mar 2013 15:18:47 -0300 Message-ID: <5140C306.4040703@linux.vnet.ibm.com> Date: Wed, 13 Mar 2013 13:18:46 -0500 From: Joel Schopp MIME-Version: 1.0 References: <1363128232-3686-1-git-send-email-jschopp@linux.vnet.ibm.com> <51406DF4.8050008@redhat.com> In-Reply-To: <51406DF4.8050008@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3] Move File operations to qemu-file.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, Stefan Berger >> -util-obj-y = util/ qobject/ qapi/ trace/ >> +util-obj-y = util/ qobject/ qapi/ trace/ qemu-file.o > > Please either move it to util/ (and the include file to > include/qemu/file.h), or leave it in common-obj-y. I prefer the former, > since as a rule of thumb util-obj-y includes code that should be easy to > unit-test. I like that suggestion. I'll move qemu-file.c to the util directory. I'd rather not move the include file as it seems to be fine where it is and there are about 6000 places it is included that would have to be updated as well. Moving the unmodified file locations doesn't gain anything functional. >> + >> +/** >> + * Yield until a file descriptor becomes readable >> + * >> + * Note that this function clobbers the handlers for the file descriptor. >> + */ >> +static void coroutine_fn yield_until_fd_readable(int fd) >> +{ >> + FDYieldUntilData data; >> + >> + assert(qemu_in_coroutine()); >> + data.co = qemu_coroutine_self(); >> + data.fd = fd; >> + qemu_set_fd_handler(fd, fd_coroutine_enter, NULL, &data); >> + qemu_coroutine_yield(); >> +} > > Coroutines are not part of libqemuutil.a, so you should be getting > missing symbols here; I'm surprised that the patch successfully passes > "make check" though I may be missing something. It does pass make check. I'd rather move all the file related stuff out of migration. > > Everything from here down to qemu_fopen_bdrv (included) should remain in > savevm.c: I like these broken out better, but I don't actually need them so I'll go ahead and leave them where they are for now on your suggestion. > >> +static int block_put_buffer(void *opaque, const uint8_t *buf, >> + int64_t pos, int size) >> +{ >> + bdrv_save_vmstate(opaque, buf, pos, size); >> + return size; >> +} >> + >> +static int block_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size) >> +{ >> + return bdrv_load_vmstate(opaque, buf, pos, size); >> +} >> + >> +static int bdrv_fclose(void *opaque) >> +{ >> + return bdrv_flush(opaque); >> +} >> + >> +static const QEMUFileOps bdrv_read_ops = { >> + .get_buffer = block_get_buffer, >> + .close = bdrv_fclose >> +}; >> + >> +static const QEMUFileOps bdrv_write_ops = { >> + .put_buffer = block_put_buffer, >> + .close = bdrv_fclose >> +}; >> + >> +QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable) >> +{ >> + if (is_writable) >> + return qemu_fopen_ops(bs, &bdrv_write_ops); >> + return qemu_fopen_ops(bs, &bdrv_read_ops); >> +}