Re: [Qemu-devel] [PATCH] loadvm for read-only snapshot files

[Eddie Kohler]

andrzej zaborowski wrote:
> On 03/02/2008, Eddie Kohler <address@hidden> wrote:
> > The following patch, against cvs, supports read-only snapshots on read-only
> > qcow2 image files.  Snapshots can be loaded, but not saved.  This is really
> > useful in my context, which is operating systems projects; a read-only memory
> > snapshot allows students to share a snapshot that skips the boot process,
> > allowing them to run their code right away.
> >
> > Please let me know if anything needs changing.
>
> Instead of including "block_int.h" in vl.c you should use the provided
> functions from "block.h" (bs->drv can be checked with _is_inserted).

Thanks for the comment.  The attached patch introduces a new block.h function, 
bdev_is_snapshot_capable.  Better?

Eddie

Index: block-qcow2.c
===================================================================
RCS file: /sources/qemu/qemu/block-qcow2.c,v
retrieving revision 1.10
diff -u -u -r1.10 block-qcow2.c
--- block-qcow2.c       11 Nov 2007 02:51:16 -0000      1.10
+++ block-qcow2.c       10 Feb 2008 23:54:34 -0000
@@ -429,7 +429,6 @@
     BDRVQcowState *s = bs->opaque;
     int new_l1_size, new_l1_size2, ret, i;
     uint64_t *new_l1_table;
-    uint64_t new_l1_table_offset;
     uint64_t data64;
     uint32_t data32;
 
@@ -450,28 +449,32 @@
     memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
 
     /* write new table (align to cluster) */
-    new_l1_table_offset = alloc_clusters(bs, new_l1_size2);
+    if (!bs->read_only) {
+       uint64_t new_l1_table_offset = alloc_clusters(bs, new_l1_size2);
 
-    for(i = 0; i < s->l1_size; i++)
-        new_l1_table[i] = cpu_to_be64(new_l1_table[i]);
-    ret = bdrv_pwrite(s->hd, new_l1_table_offset, new_l1_table, new_l1_size2);
-    if (ret != new_l1_size2)
-        goto fail;
-    for(i = 0; i < s->l1_size; i++)
-        new_l1_table[i] = be64_to_cpu(new_l1_table[i]);
-
-    /* set new table */
-    data64 = cpu_to_be64(new_l1_table_offset);
-    if (bdrv_pwrite(s->hd, offsetof(QCowHeader, l1_table_offset),
-                    &data64, sizeof(data64)) != sizeof(data64))
-        goto fail;
-    data32 = cpu_to_be32(new_l1_size);
-    if (bdrv_pwrite(s->hd, offsetof(QCowHeader, l1_size),
-                    &data32, sizeof(data32)) != sizeof(data32))
-        goto fail;
+       for(i = 0; i < s->l1_size; i++)
+           new_l1_table[i] = cpu_to_be64(new_l1_table[i]);
+       ret = bdrv_pwrite(s->hd, new_l1_table_offset, new_l1_table, 
new_l1_size2);
+       if (ret != new_l1_size2)
+           goto fail;
+       for(i = 0; i < s->l1_size; i++)
+           new_l1_table[i] = be64_to_cpu(new_l1_table[i]);
+
+       /* set new table */
+       data64 = cpu_to_be64(new_l1_table_offset);
+       if (bdrv_pwrite(s->hd, offsetof(QCowHeader, l1_table_offset),
+                       &data64, sizeof(data64)) != sizeof(data64))
+           goto fail;
+       data32 = cpu_to_be32(new_l1_size);
+       if (bdrv_pwrite(s->hd, offsetof(QCowHeader, l1_size),
+                       &data32, sizeof(data32)) != sizeof(data32))
+           goto fail;
+       free_clusters(bs, s->l1_table_offset, s->l1_size * sizeof(uint64_t));
+       
+       s->l1_table_offset = new_l1_table_offset;
+    }
+    
     qemu_free(s->l1_table);
-    free_clusters(bs, s->l1_table_offset, s->l1_size * sizeof(uint64_t));
-    s->l1_table_offset = new_l1_table_offset;
     s->l1_table = new_l1_table;
     s->l1_size = new_l1_size;
     return 0;
@@ -521,7 +524,8 @@
         /* update the L1 entry */
         s->l1_table[l1_index] = l2_offset | QCOW_OFLAG_COPIED;
         tmp = cpu_to_be64(l2_offset | QCOW_OFLAG_COPIED);
-        if (bdrv_pwrite(s->hd, s->l1_table_offset + l1_index * sizeof(tmp),
+        if (!bs->read_only &&
+           bdrv_pwrite(s->hd, s->l1_table_offset + l1_index * sizeof(tmp),
                         &tmp, sizeof(tmp)) != sizeof(tmp))
             return 0;
         min_index = l2_cache_new_entry(bs);
@@ -535,7 +539,8 @@
                 s->l2_size * sizeof(uint64_t))
                 return 0;
         }
-        if (bdrv_pwrite(s->hd, l2_offset,
+        if (!bs->read_only &&
+           bdrv_pwrite(s->hd, l2_offset,
                         l2_table, s->l2_size * sizeof(uint64_t)) !=
             s->l2_size * sizeof(uint64_t))
             return 0;
@@ -624,7 +629,8 @@
     }
     /* update L2 table */
     l2_table[l2_index] = tmp;
-    if (bdrv_pwrite(s->hd,
+    if (!bs->read_only &&
+       bdrv_pwrite(s->hd,
                     l2_offset + l2_index * sizeof(tmp), &tmp, sizeof(tmp)) != 
sizeof(tmp))
         return 0;
     return cluster_offset;
@@ -1322,7 +1328,7 @@
                     }
                 }
             }
-            if (l2_modified) {
+            if (l2_modified && !bs->read_only) {
                 if (bdrv_pwrite(s->hd,
                                 l2_offset, l2_table, l2_size) != l2_size)
                     goto fail;
@@ -1342,7 +1348,7 @@
             }
         }
     }
-    if (l1_modified) {
+    if (l1_modified && !bs->read_only) {
         for(i = 0; i < l1_size; i++)
             cpu_to_be64s(&l1_table[i]);
         if (bdrv_pwrite(s->hd, l1_table_offset, l1_table,
@@ -1553,6 +1559,9 @@
     int i, ret;
     uint64_t *l1_table = NULL;
 
+    if (bs->read_only)
+       return -EACCES;
+    
     memset(sn, 0, sizeof(*sn));
 
     if (sn_info->id_str[0] == '\0') {
@@ -1628,7 +1637,8 @@
         return -ENOENT;
     sn = &s->snapshots[snapshot_index];
 
-    if (update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, -1) < 0)
+    if (!bs->read_only &&
+       update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, -1) < 0)
         goto fail;
 
     if (grow_l1_table(bs, sn->l1_size) < 0)
@@ -1640,14 +1650,17 @@
     if (bdrv_pread(s->hd, sn->l1_table_offset,
                    s->l1_table, l1_size2) != l1_size2)
         goto fail;
-    if (bdrv_pwrite(s->hd, s->l1_table_offset,
-                    s->l1_table, l1_size2) != l1_size2)
+    if (bs->read_only)
+       s->l1_table_offset = sn->l1_table_offset;
+    else if (bdrv_pwrite(s->hd, s->l1_table_offset,
+                        s->l1_table, l1_size2) != l1_size2)
         goto fail;
     for(i = 0;i < s->l1_size; i++) {
         be64_to_cpus(&s->l1_table[i]);
     }
 
-    if (update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 1) < 0)
+    if (!bs->read_only &&
+       update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 1) < 0)
         goto fail;
 
 #ifdef DEBUG_ALLOC
@@ -1664,6 +1677,9 @@
     QCowSnapshot *sn;
     int snapshot_index, ret;
 
+    if (bs->read_only)
+       return -EACCES;
+
     snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
     if (snapshot_index < 0)
         return -ENOENT;
@@ -1959,6 +1975,9 @@
     int ret, refcount_table_index, block_index, refcount;
     uint64_t data64;
 
+    if (bs->read_only)
+       return 2;
+
     refcount_table_index = cluster_index >> (s->cluster_bits - REFCOUNT_SHIFT);
     if (refcount_table_index >= s->refcount_table_size) {
         if (addend < 0)
Index: block.c
===================================================================
RCS file: /sources/qemu/qemu/block.c,v
retrieving revision 1.53
diff -u -u -r1.53 block.c
--- block.c     24 Dec 2007 16:10:43 -0000      1.53
+++ block.c     10 Feb 2008 23:54:34 -0000
@@ -968,6 +968,19 @@
 /**************************************************************/
 /* handling of snapshots */
 
+/**
+ * Return TRUE if the driver can go to a snapshot
+ */
+int bdrv_is_snapshot_capable(BlockDriverState *bs)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv)
+        return 0;
+    if (!drv->bdrv_snapshot_goto)
+        return 0;
+    return 1;
+}
+
 int bdrv_snapshot_create(BlockDriverState *bs,
                          QEMUSnapshotInfo *sn_info)
 {
Index: block.h
===================================================================
RCS file: /sources/qemu/qemu/block.h,v
retrieving revision 1.6
diff -u -u -r1.6 block.h
--- block.h     24 Dec 2007 16:10:43 -0000      1.6
+++ block.h     10 Feb 2008 23:54:34 -0000
@@ -139,6 +139,7 @@
                           const uint8_t *buf, int nb_sectors);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
 
+int bdrv_is_snapshot_capable(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size);
 int bdrv_snapshot_create(BlockDriverState *bs,
Index: vl.c
===================================================================
RCS file: /sources/qemu/qemu/vl.c,v
retrieving revision 1.405
diff -u -u -r1.405 vl.c
--- vl.c        10 Feb 2008 16:33:13 -0000      1.405
+++ vl.c        10 Feb 2008 23:54:34 -0000
@@ -5970,7 +5970,7 @@
 {
     return (bs &&
             !bdrv_is_removable(bs) &&
-            !bdrv_is_read_only(bs));
+           bdrv_is_snapshot_capable(bs));
 }
 
 /* device must be snapshots in order to have a reliable snapshot */
@@ -5978,7 +5978,7 @@
 {
     return (bs &&
             !bdrv_is_removable(bs) &&
-            !bdrv_is_read_only(bs));
+           bdrv_is_snapshot_capable(bs));
 }
 
 static BlockDriverState *get_bs_snapshots(void)