On 09/18/2012 05:20 PM, Eric Blake wrote: > On 09/18/2012 12:53 PM, Jeff Cody wrote: >> This is derived from the Supriya Kannery's reopen patches. >> >> This contains the raw-posix driver changes for the bdrv_reopen_* >> functions. All changes are staged into a temporary scratch buffer >> during the prepare() stage, and copied over to the live structure >> during commit(). Upon abort(), all changes are abandoned, and the >> live structures are unmodified. >> >> The _prepare() will create an extra fd - either by means of a dup, >> if possible, or opening a new fd if not (for instance, access >> control changes). Upon _commit(), the original fd is closed and >> the new fd is used. Upon _abort(), the duplicate/new fd is closed. >> >> Signed-off-by: Jeff Cody <jc...@redhat.com> >> --- >> block/raw-posix.c | 99 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 99 insertions(+) >> >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index 6bf5480..edd1eca 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -138,6 +138,15 @@ typedef struct BDRVRawState { >> #endif >> } BDRVRawState; >> >> +typedef struct BDRVRawReopenState { >> + int fd; >> + int open_flags; >> +#ifdef CONFIG_LINUX_AIO >> + int use_aio; >> + void *aio_ctx; >> +#endif > > These members are conditional... > >> +static int raw_reopen_prepare(BDRVReopenState *state, >> + BlockReopenQueue *queue, Error **errp) >> +{ >> + BDRVRawState *s; >> + BDRVRawReopenState *raw_s; >> + int ret = 0; >> + >> + assert(state != NULL); >> + assert(state->bs != NULL); >> + >> + s = state->bs->opaque; >> + >> + state->opaque = g_malloc0(sizeof(BDRVRawReopenState)); >> + raw_s = state->opaque; >> + raw_s->use_aio = s->use_aio; >> + raw_s->aio_ctx = s->aio_ctx; > > ...but you are unconditionally assigning into them. This will introduce > compile failures on other platforms.
Argh. Thanks. > >> + if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & >> ~fcntl_flags)) { >> + /* dup the original fd */ >> + /* TODO: use qemu fcntl wrapper */ >> + raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0); > > F_DUPFD_CLOEXEC is not defined everywhere yet; you still need to address > your TODO. > >> + >> + /* If we cannot use fctnl, or fcntl failed, fall back to qemu_open() */ > > s/fctnl/fcntl/ > >> +static void raw_reopen_commit(BDRVReopenState *state) >> +{ >> + BDRVRawReopenState *raw_s = state->opaque; >> + BDRVRawState *s = state->bs->opaque; >> + >> + s->open_flags = raw_s->open_flags; >> + >> + qemu_close(s->fd); >> + s->fd = raw_s->fd; >> + s->use_aio = raw_s->use_aio; >> + s->aio_ctx = raw_s->aio_ctx; > > Again, more unconditional use of conditional members. >