diff mbox

[v2,08/16] block: raw-posix image file reopen

Message ID 6316cedacd954f8106aca8a45618088e24c4a757.1347548248.git.jcody@redhat.com
State New
Headers show

Commit Message

Jeff Cody Sept. 13, 2012, 3:49 p.m. UTC
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 <jcody@redhat.com>
---
 block/raw-posix.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

Comments

Jeff Cody Sept. 13, 2012, 4:57 p.m. UTC | #1
On 09/13/2012 12:02 PM, Paolo Bonzini wrote:
> Il 13/09/2012 17:49, Jeff Cody ha scritto:
>> +
>> +    /*
>> +     * If we didn't have BDRV_O_NOCACHE set before, we may not have allocated
>> +     * aligned_buf
>> +     */
>> +    ret = raw_allocate_aligned_buf(&raw_s->aligned_buf,
>> +                                   &raw_s->aligned_buf_size, state->flags);
> 
> Aligned_buf is unused, except for checking if it exists here:
> 
>     if (s->aligned_buf) {
>         if (!qiov_is_aligned(bs, qiov)) {
>             type |= QEMU_AIO_MISALIGNED;
> #ifdef CONFIG_LINUX_AIO
>         } else if (s->use_aio) {
>             return laio_submit(bs, s->aio_ctx, s->fd, sector_num, qiov,
>                                nb_sectors, cb, opaque, type);
> #endif
>         }
>     }
> 
> You can instead check BDRV_O_NOCACHE and kill aligned_buf completely.
> 
> Paolo
>

Hmm - yes, it looks like that is so.  This patch will transform into
checking for BDRV_O_NOCACHE as you suggest, and killing aligned_buf.

Thanks,
Jeff
Kevin Wolf Sept. 14, 2012, 7:32 a.m. UTC | #2
Am 13.09.2012 18:57, schrieb Jeff Cody:
> On 09/13/2012 12:02 PM, Paolo Bonzini wrote:
>> Il 13/09/2012 17:49, Jeff Cody ha scritto:
>>> +
>>> +    /*
>>> +     * If we didn't have BDRV_O_NOCACHE set before, we may not have allocated
>>> +     * aligned_buf
>>> +     */
>>> +    ret = raw_allocate_aligned_buf(&raw_s->aligned_buf,
>>> +                                   &raw_s->aligned_buf_size, state->flags);
>>
>> Aligned_buf is unused, except for checking if it exists here:
>>
>>     if (s->aligned_buf) {
>>         if (!qiov_is_aligned(bs, qiov)) {
>>             type |= QEMU_AIO_MISALIGNED;
>> #ifdef CONFIG_LINUX_AIO
>>         } else if (s->use_aio) {
>>             return laio_submit(bs, s->aio_ctx, s->fd, sector_num, qiov,
>>                                nb_sectors, cb, opaque, type);
>> #endif
>>         }
>>     }
>>
>> You can instead check BDRV_O_NOCACHE and kill aligned_buf completely.
>>
>> Paolo
>>
> 
> Hmm - yes, it looks like that is so.  This patch will transform into
> checking for BDRV_O_NOCACHE as you suggest, and killing aligned_buf.

Sounds good, but please make removing aligned_buf a separate patch.

Kevin
diff mbox

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 47cab9f..2407eeb 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -140,6 +140,19 @@  typedef struct BDRVRawState {
 #endif
 } BDRVRawState;
 
+typedef struct BDRVRawReopenState {
+    BDRVReopenState reopen_state;
+    int fd;
+    int open_flags;
+#ifdef CONFIG_LINUX_AIO
+    int use_aio;
+    void *aio_ctx;
+#endif
+    uint8_t *aligned_buf;
+    unsigned aligned_buf_size;
+    BDRVRawState *stash_s;
+} BDRVRawReopenState;
+
 static int fd_open(BlockDriverState *bs);
 static int64_t raw_getlength(BlockDriverState *bs);
 
@@ -318,6 +331,113 @@  static int raw_open(BlockDriverState *bs, const char *filename, int flags)
     return raw_open_common(bs, filename, flags, 0);
 }
 
+static int raw_reopen_prepare(BDRVReopenState *state, 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;
+
+    raw_parse_flags(state->flags, &raw_s->open_flags);
+    raw_set_aio(&raw_s->aio_ctx, &raw_s->use_aio, state->flags);
+
+    /*
+     * If we didn't have BDRV_O_NOCACHE set before, we may not have allocated
+     * aligned_buf
+     */
+    ret = raw_allocate_aligned_buf(&raw_s->aligned_buf,
+                                   &raw_s->aligned_buf_size, state->flags);
+    if (ret) {
+        goto error;
+    }
+
+    raw_s->fd = -1;
+
+    int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
+#ifdef O_NOATIME
+    fcntl_flags |= O_NOATIME;
+#endif
+    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);
+        if (raw_s->fd >= 0) {
+            ret = fcntl_setfl(raw_s->fd, raw_s->open_flags);
+            if (ret) {
+                qemu_close(raw_s->fd);
+                raw_s->fd = -1;
+            }
+        }
+    }
+
+    /* If we cannot use fctnl, or fcntl failed, fall back to qemu_open() */
+    if (raw_s->fd == -1) {
+        assert(!(raw_s->open_flags & O_CREAT));
+        raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags);
+        if (raw_s->fd == -1) {
+            ret = -1;
+        }
+    }
+error:
+    return ret;
+}
+
+
+static void raw_reopen_commit(BDRVReopenState *state)
+{
+    BDRVRawReopenState *raw_s = state->opaque;
+    BDRVRawState *s = state->bs->opaque;
+
+    if (raw_s->aligned_buf != NULL) {
+        if (s->aligned_buf) {
+            qemu_vfree(s->aligned_buf);
+        }
+        s->aligned_buf      = raw_s->aligned_buf;
+        s->aligned_buf_size = raw_s->aligned_buf_size;
+    }
+
+    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;
+
+    g_free(state->opaque);
+    state->opaque = NULL;
+}
+
+
+static void raw_reopen_abort(BDRVReopenState *state)
+{
+    BDRVRawReopenState *raw_s = state->opaque;
+
+     /* nothing to do if NULL, we didn't get far enough */
+    if (raw_s == NULL) {
+        return;
+    }
+
+    if (raw_s->fd >= 0) {
+        qemu_close(raw_s->fd);
+        raw_s->fd = -1;
+        if (raw_s->aligned_buf != NULL) {
+            qemu_vfree(raw_s->aligned_buf);
+        }
+    }
+    g_free(state->opaque);
+    state->opaque = NULL;
+}
+
+
 /* XXX: use host sector size if necessary with:
 #ifdef DIOCGSECTORSIZE
         {
@@ -770,6 +890,9 @@  static BlockDriver bdrv_file = {
     .instance_size = sizeof(BDRVRawState),
     .bdrv_probe = NULL, /* no probe for protocols */
     .bdrv_file_open = raw_open,
+    .bdrv_reopen_prepare = raw_reopen_prepare,
+    .bdrv_reopen_commit = raw_reopen_commit,
+    .bdrv_reopen_abort = raw_reopen_abort,
     .bdrv_close = raw_close,
     .bdrv_create = raw_create,
     .bdrv_co_discard = raw_co_discard,