[v3,14/19] block: vmdk image file reopen

Submitted by Jeff Cody on Sept. 18, 2012, 6:53 p.m.

Details

Message ID 8a055b1732489a90b50fcf941691ef2a14bb4514.1347993885.git.jcody@redhat.com
State New
Headers show

Commit Message

Jeff Cody Sept. 18, 2012, 6:53 p.m.
This patch supports reopen for VMDK image files.  VMDK extents are added
to the existing reopen queue, so that the transactional model of reopen
is maintained with multiple image files.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vmdk.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

Kevin Wolf Sept. 20, 2012, 2:12 p.m.
Am 18.09.2012 20:53, schrieb Jeff Cody:
> This patch supports reopen for VMDK image files.  VMDK extents are added
> to the existing reopen queue, so that the transactional model of reopen
> is maintained with multiple image files.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/vmdk.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index bba4c61..f2e861b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -300,6 +300,40 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
>      return 1;
>  }
>  
> +/* Queue extents, if any, for reopen() */
> +static int vmdk_reopen_prepare(BDRVReopenState *state,
> +                               BlockReopenQueue *queue, Error **errp)
> +{
> +    BDRVVmdkState *s;
> +    int ret = -1;
> +    int i;
> +    VmdkExtent *e;
> +
> +    assert(state != NULL);
> +    assert(state->bs != NULL);
> +
> +    if (queue == NULL) {
> +        error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> +                 "No reopen queue for VMDK extents");
> +        goto exit;
> +    }
> +
> +    s = state->bs->opaque;
> +
> +    assert(s != NULL);
> +
> +    for (i = 0; i < s->num_extents; i++) {
> +        e = &s->extents[i];
> +        if (e->file != state->bs->file) {
> +            bdrv_reopen_queue(queue, e->file, state->flags);

This is wrong, you can't abort the transaction if you do it like this.

VMDK needs to separately pass through each of prepare/commit/abort to
all extents, it can't use the all-in-one function.

Kevin
Jeff Cody Sept. 20, 2012, 2:49 p.m.
On 09/20/2012 10:12 AM, Kevin Wolf wrote:
> Am 18.09.2012 20:53, schrieb Jeff Cody:
>> This patch supports reopen for VMDK image files.  VMDK extents are added
>> to the existing reopen queue, so that the transactional model of reopen
>> is maintained with multiple image files.
>>
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>> ---
>>  block/vmdk.c | 35 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index bba4c61..f2e861b 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -300,6 +300,40 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
>>      return 1;
>>  }
>>  
>> +/* Queue extents, if any, for reopen() */
>> +static int vmdk_reopen_prepare(BDRVReopenState *state,
>> +                               BlockReopenQueue *queue, Error **errp)
>> +{
>> +    BDRVVmdkState *s;
>> +    int ret = -1;
>> +    int i;
>> +    VmdkExtent *e;
>> +
>> +    assert(state != NULL);
>> +    assert(state->bs != NULL);
>> +
>> +    if (queue == NULL) {
>> +        error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> +                 "No reopen queue for VMDK extents");
>> +        goto exit;
>> +    }
>> +
>> +    s = state->bs->opaque;
>> +
>> +    assert(s != NULL);
>> +
>> +    for (i = 0; i < s->num_extents; i++) {
>> +        e = &s->extents[i];
>> +        if (e->file != state->bs->file) {
>> +            bdrv_reopen_queue(queue, e->file, state->flags);
> 
> This is wrong, you can't abort the transaction if you do it like this.
> 
> VMDK needs to separately pass through each of prepare/commit/abort to
> all extents, it can't use the all-in-one function.
> 
> Kevin
> 
(Follow up from conversation on IRC)

This is actually correct, because it is adding the extents to the queue
in the prepare(). This will cause the extents to go through the same
transactional prepare/commit/abort cycle as the rest of the images in
the queue.

Jeff

Patch hide | download patch | download mbox

diff --git a/block/vmdk.c b/block/vmdk.c
index bba4c61..f2e861b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -300,6 +300,40 @@  static int vmdk_is_cid_valid(BlockDriverState *bs)
     return 1;
 }
 
+/* Queue extents, if any, for reopen() */
+static int vmdk_reopen_prepare(BDRVReopenState *state,
+                               BlockReopenQueue *queue, Error **errp)
+{
+    BDRVVmdkState *s;
+    int ret = -1;
+    int i;
+    VmdkExtent *e;
+
+    assert(state != NULL);
+    assert(state->bs != NULL);
+
+    if (queue == NULL) {
+        error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+                 "No reopen queue for VMDK extents");
+        goto exit;
+    }
+
+    s = state->bs->opaque;
+
+    assert(s != NULL);
+
+    for (i = 0; i < s->num_extents; i++) {
+        e = &s->extents[i];
+        if (e->file != state->bs->file) {
+            bdrv_reopen_queue(queue, e->file, state->flags);
+        }
+    }
+    ret = 0;
+
+exit:
+    return ret;
+}
+
 static int vmdk_parent_open(BlockDriverState *bs)
 {
     char *p_name;
@@ -1646,6 +1680,7 @@  static BlockDriver bdrv_vmdk = {
     .instance_size  = sizeof(BDRVVmdkState),
     .bdrv_probe     = vmdk_probe,
     .bdrv_open      = vmdk_open,
+    .bdrv_reopen_prepare = vmdk_reopen_prepare,
     .bdrv_read      = vmdk_co_read,
     .bdrv_write     = vmdk_co_write,
     .bdrv_close     = vmdk_close,