[Qemu-devel] [PATCH V2 0/2] Implement sync modes for drive-backup.

[Qemu-devel] [PATCH V2 0/2] Implement sync modes for drive-backup.

[Ian Main]

This patch adds sync modes on top of the work that Stefan Hajnoczi has done.

These patches apply on kevin/block with 
'[PATCH] block: add drive_backup HMP command' also applied.

Hopefully all is in order as this is my first QEMU patch.  Many thanks to 
Stephan and Fam Zheng for their help.

V2:

- No longer poll, instead use qemu_coroutine_yield().
- Use bdrv_co_is_allocated().
- Much better SYNC_MODE_NONE test.

Ian Main (2):
  Implement sync modes for drive-backup.
  Add tests for sync modes 'TOP' and 'NONE'

 block/backup.c                | 90 ++++++++++++++++++++++++++++---------------
 blockdev.c                    | 25 ++++++++----
 include/block/block_int.h     |  4 +-
 qapi-schema.json              |  4 ++
 qmp-commands.hx               |  1 +
 tests/qemu-iotests/055        | 67 ++++++++++++++++++++++++++++++--
 tests/qemu-iotests/055.out    |  4 +-
 tests/qemu-iotests/group      |  2 +-
 tests/qemu-iotests/iotests.py |  5 +++
 9 files changed, 156 insertions(+), 46 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH V2 1/2] Implement sync modes for drive-backup.

[Ian Main]

This patch adds sync-modes to the drive-backup interface and
implements the FULL, NONE and TOP modes of synchronization.

FULL performs as before copying the entire contents of the drive
while preserving the point-in-time using CoW.
NONE only copies new writes to the target drive.
TOP copies changes to the topmost drive image and preserves the
point-in-time using CoW.

Signed-off-by: Ian Main <imain@redhat.com>
---
 block/backup.c            | 90 +++++++++++++++++++++++++++++++----------------
 blockdev.c                | 25 ++++++++-----
 include/block/block_int.h |  4 ++-
 qapi-schema.json          |  4 +++
 qmp-commands.hx           |  1 +
 5 files changed, 85 insertions(+), 39 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 16105d4..e72a5af 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,6 +37,7 @@ typedef struct CowRequest {
 typedef struct BackupBlockJob {
     BlockJob common;
     BlockDriverState *target;
+    MirrorSyncMode sync_mode;
     RateLimit limit;
     BlockdevOnError on_source_error;
     BlockdevOnError on_target_error;
@@ -247,40 +248,68 @@ static void coroutine_fn backup_run(void *opaque)
 
     bdrv_add_before_write_notifier(bs, &before_write);
 
-    for (; start < end; start++) {
-        bool error_is_read;
-
-        if (block_job_is_cancelled(&job->common)) {
-            break;
+    if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
+        while (!block_job_is_cancelled(&job->common)) {
+            /* Yield until the job is cancelled.  We just let our before_write
+             * notify callback service CoW requests. */
+            job->common.busy = false;
+            qemu_coroutine_yield();
+            job->common.busy = true;
         }
+    } else {
+        /* Both FULL and TOP SYNC_MODE's require copying.. */
+        for (; start < end; start++) {
+            bool error_is_read;
 
-        /* we need to yield so that qemu_aio_flush() returns.
-         * (without, VM does not reboot)
-         */
-        if (job->common.speed) {
-            uint64_t delay_ns = ratelimit_calculate_delay(
-                &job->limit, job->sectors_read);
-            job->sectors_read = 0;
-            block_job_sleep_ns(&job->common, rt_clock, delay_ns);
-        } else {
-            block_job_sleep_ns(&job->common, rt_clock, 0);
-        }
+            if (block_job_is_cancelled(&job->common)) {
+                break;
+            }
 
-        if (block_job_is_cancelled(&job->common)) {
-            break;
-        }
+            /* we need to yield so that qemu_aio_flush() returns.
+             * (without, VM does not reboot)
+             */
+            if (job->common.speed) {
+                uint64_t delay_ns = ratelimit_calculate_delay(
+                        &job->limit, job->sectors_read);
+                job->sectors_read = 0;
+                block_job_sleep_ns(&job->common, rt_clock, delay_ns);
+            } else {
+                block_job_sleep_ns(&job->common, rt_clock, 0);
+            }
 
-        ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
-                            BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
-        if (ret < 0) {
-            /* Depending on error action, fail now or retry cluster */
-            BlockErrorAction action =
-                backup_error_action(job, error_is_read, -ret);
-            if (action == BDRV_ACTION_REPORT) {
+            if (block_job_is_cancelled(&job->common)) {
                 break;
-            } else {
-                start--;
-                continue;
+            }
+
+            if (job->sync_mode == MIRROR_SYNC_MODE_TOP) {
+                int n, alloced;
+
+                /* Check to see if these blocks are already in the backing file. */
+
+                alloced =
+                    bdrv_co_is_allocated(bs, start * BACKUP_SECTORS_PER_CLUSTER,
+                                         BACKUP_SECTORS_PER_CLUSTER, &n);
+                /* The above call returns true if the given sector is in the
+                 * topmost image.  If that is the case then we must copy it as
+                 * it has been modified from the original backing file. 
+                 * Otherwise we skip it. */
+                if (alloced == 0) {
+                    continue;
+                }
+            }
+            /* FULL sync mode we copy the whole drive. */
+            ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
+                    BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
+            if (ret < 0) {
+                /* Depending on error action, fail now or retry cluster */
+                BlockErrorAction action =
+                    backup_error_action(job, error_is_read, -ret);
+                if (action == BDRV_ACTION_REPORT) {
+                    break;
+                } else {
+                    start--;
+                    continue;
+                }
             }
         }
     }
@@ -300,7 +329,7 @@ static void coroutine_fn backup_run(void *opaque)
 }
 
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
-                  int64_t speed,
+                  int64_t speed, MirrorSyncMode sync_mode,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   BlockDriverCompletionFunc *cb, void *opaque,
@@ -335,6 +364,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
     job->on_source_error = on_source_error;
     job->on_target_error = on_target_error;
     job->target = target;
+    job->sync_mode = sync_mode;
     job->common.len = len;
     job->common.co = qemu_coroutine_create(backup_run);
     qemu_coroutine_enter(job->common.co, job);
diff --git a/blockdev.c b/blockdev.c
index c5abd65..000dea6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1430,17 +1430,13 @@ void qmp_drive_backup(const char *device, const char *target,
                       Error **errp)
 {
     BlockDriverState *bs;
-    BlockDriverState *target_bs;
+    BlockDriverState *target_bs, *source;
     BlockDriver *drv = NULL;
     Error *local_err = NULL;
     int flags;
     int64_t size;
     int ret;
 
-    if (sync != MIRROR_SYNC_MODE_FULL) {
-        error_setg(errp, "only sync mode 'full' is currently supported");
-        return;
-    }
     if (!has_speed) {
         speed = 0;
     }
@@ -1483,6 +1479,13 @@ void qmp_drive_backup(const char *device, const char *target,
 
     flags = bs->open_flags | BDRV_O_RDWR;
 
+    /* See if we have a backing HD we can use to create our new image
+     * on top of. */
+    source = bs->backing_hd;
+    if (!source && sync == MIRROR_SYNC_MODE_TOP) {
+        sync = MIRROR_SYNC_MODE_FULL;
+    }
+
     size = bdrv_getlength(bs);
     if (size < 0) {
         error_setg_errno(errp, -size, "bdrv_getlength failed");
@@ -1491,8 +1494,14 @@ void qmp_drive_backup(const char *device, const char *target,
 
     if (mode != NEW_IMAGE_MODE_EXISTING) {
         assert(format && drv);
-        bdrv_img_create(target, format,
-                        NULL, NULL, NULL, size, flags, &local_err, false);
+        if (sync == MIRROR_SYNC_MODE_TOP) {
+            bdrv_img_create(target, format, source->filename,
+                            source->drv->format_name, NULL,
+                            size, flags, &local_err, false);
+        } else {
+            bdrv_img_create(target, format, NULL, NULL, NULL,
+                            size, flags, &local_err, false);
+        }
     }
 
     if (error_is_set(&local_err)) {
@@ -1508,7 +1517,7 @@ void qmp_drive_backup(const char *device, const char *target,
         return;
     }
 
-    backup_start(bs, target_bs, speed, on_source_error, on_target_error,
+    backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
                  block_job_cb, bs, &local_err);
     if (local_err != NULL) {
         bdrv_delete(target_bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index c6ac871..e45f2a0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -404,6 +404,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
  * @bs: Block device to operate on.
  * @target: Block device to write to.
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
+ * @sync_mode: What parts of the disk image should be copied to the destination.
  * @on_source_error: The action to take upon error reading from the source.
  * @on_target_error: The action to take upon error writing to the target.
  * @cb: Completion function for the job.
@@ -413,7 +414,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
  * until the job is cancelled or manually completed.
  */
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
-                  int64_t speed, BlockdevOnError on_source_error,
+                  int64_t speed, MirrorSyncMode sync_mode,
+                  BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   BlockDriverCompletionFunc *cb, void *opaque,
                   Error **errp);
diff --git a/qapi-schema.json b/qapi-schema.json
index cdd2d6b..b3f6c2a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1807,6 +1807,10 @@
 # @format: #optional the format of the new destination, default is to
 #          probe if @mode is 'existing', else the format of the source
 #
+# @sync: what parts of the disk image should be copied to the destination
+#        (all the disk, only the sectors allocated in the topmost image, or
+#        only new I/O).
+#
 # @mode: #optional whether and how QEMU should create a new image, default is
 #        'absolute-paths'.
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e075df4..f3f6b3d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -957,6 +957,7 @@ Arguments:
 
 Example:
 -> { "execute": "drive-backup", "arguments": { "device": "drive0",
+                                               "sync": "full",
                                                "target": "backup.img" } }
 <- { "return": {} }
 EQMP
-- 
1.8.1.4

[Qemu-devel] [PATCH V2 2/2] Add tests for sync modes 'TOP' and 'NONE'

[Ian Main]

This patch adds tests for sync modes top and none.

Signed-off-by: Ian Main <imain@redhat.com>
---
 tests/qemu-iotests/055        | 67 ++++++++++++++++++++++++++++++++++++++++---
 tests/qemu-iotests/055.out    |  4 +--
 tests/qemu-iotests/group      |  2 +-
 tests/qemu-iotests/iotests.py |  5 ++++
 4 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index c66f8db..06d1f77 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -23,8 +23,9 @@
 import time
 import os
 import iotests
-from iotests import qemu_img, qemu_io
+from iotests import qemu_img, qemu_io, create_image
 
+backing_img = os.path.join(iotests.test_dir, 'backing.img')
 test_img = os.path.join(iotests.test_dir, 'test.img')
 target_img = os.path.join(iotests.test_dir, 'target.img')
 
@@ -34,7 +35,7 @@ class TestSingleDrive(iotests.QMPTestCase):
     def setUp(self):
         # Write data to the image so we can compare later
         qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSingleDrive.image_len))
-        qemu_io('-c', 'write -P0x5d 0 64k', test_img)
+        qemu_io('-c', 'write -P0x41 0 512', test_img)
         qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
         qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
         qemu_io('-c', 'write -P0xdc 67043328 64k', test_img)
@@ -60,6 +61,22 @@ class TestSingleDrive(iotests.QMPTestCase):
         event = self.cancel_and_wait()
         self.assert_qmp(event, 'data/type', 'backup')
 
+    def test_cancel_sync_none(self):
+        self.assert_no_active_block_jobs()
+
+        result = self.vm.qmp('drive-backup', device='drive0',
+                             sync='none', format='raw', target=target_img)
+        self.assert_qmp(result, 'return', {})
+        time.sleep(1)
+        self.vm.hmp_qemu_io('drive0', 'write -P0x5e 0 512')
+        self.vm.hmp_qemu_io('drive0', 'aio_flush')
+        time.sleep(1)
+        # Verify that the original contents exist in the target image.
+        self.assertEqual(-1, qemu_io('-c', 'read -P0x41 0 512', target_img).find("verification failed"))
+
+        event = self.cancel_and_wait()
+        self.assert_qmp(event, 'data/type', 'backup')
+
     def test_pause(self):
         self.assert_no_active_block_jobs()
 
@@ -102,6 +119,47 @@ class TestSingleDrive(iotests.QMPTestCase):
                              target=target_img, sync='full')
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
+class TestSyncModeTop(iotests.QMPTestCase):
+    image_len = 2 * 1024 * 1024 # MB
+
+    def setUp(self):
+        create_image(backing_img, TestSyncModeTop.image_len)
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
+        qemu_io('-c', 'write -P0x5d 0 64k', test_img)
+        qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
+        qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        os.remove(backing_img)
+        try:
+            os.remove(target_img)
+        except OSError:
+            pass
+
+    def test_complete_top(self):
+        self.assert_no_active_block_jobs()
+        result = self.vm.qmp('drive-backup', device='drive0', sync='top',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        # Custom completed check as we are not copying all data.
+        completed = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp_absent(event, 'data/error')
+                    completed = True
+
+        self.assert_no_active_block_jobs()
+        self.vm.shutdown()
+        self.assertTrue(iotests.compare_images(test_img, target_img),
+                        'target image does not match source after backup')
+
 class TestSetSpeed(iotests.QMPTestCase):
     image_len = 80 * 1024 * 1024 # MB
 
@@ -127,7 +185,8 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return[0]/device', 'drive0')
         self.assert_qmp(result, 'return[0]/speed', 0)
 
-        result = self.vm.qmp('block-job-set-speed', device='drive0', speed=8 * 1024 * 1024)
+        result = self.vm.qmp('block-job-set-speed', device='drive0',
+                             speed=8 * 1024 * 1024)
         self.assert_qmp(result, 'return', {})
 
         # Ensure the speed we set was accepted
@@ -285,4 +344,4 @@ class TestSingleTransaction(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
 if __name__ == '__main__':
-    iotests.main(supported_fmts=['raw', 'qcow2'])
+    iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/055.out
index fa16b5c..96961ed 100644
--- a/tests/qemu-iotests/055.out
+++ b/tests/qemu-iotests/055.out
@@ -1,5 +1,5 @@
-.............
+...............
 ----------------------------------------------------------------------
-Ran 13 tests
+Ran 15 tests
 
 OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index fdc6ed1..d3c3f61 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -61,4 +61,4 @@
 052 rw auto backing
 053 rw auto
 054 rw auto
-055 rw auto
+055 rw auto backing
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b028a89..33ad0ec 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -95,6 +95,11 @@ class VM(object):
         self._num_drives += 1
         return self
 
+    def hmp_qemu_io(self, drive, cmd):
+        '''Write to a given drive using an HMP command'''
+        return self.qmp('human-monitor-command',
+                        command_line='qemu-io %s "%s"' % (drive, cmd))
+
     def add_fd(self, fd, fdset, opaque, opts=''):
         '''Pass a file descriptor to the VM'''
         options = ['fd=%d' % fd,
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH V2 0/2] Implement sync modes for drive-backup.

[Ian Main]

On Fri, Jul 05, 2013 at 06:35:26PM -0700, Ian Main wrote:
> This patch adds sync modes on top of the work that Stefan Hajnoczi has done.
> 
> These patches apply on kevin/block with 
> '[PATCH] block: add drive_backup HMP command' also applied.
> 
> Hopefully all is in order as this is my first QEMU patch.  Many thanks to 
> Stephan and Fam Zheng for their help.
> 
> V2:
> 
> - No longer poll, instead use qemu_coroutine_yield().
> - Use bdrv_co_is_allocated().
> - Much better SYNC_MODE_NONE test.

FYI I'll be off camping for all next week so I won't get back to you
until the following week.  Thanks in advance for any reviews!

	Ian

Re: [Qemu-devel] [PATCH V2 1/2] Implement sync modes for drive-backup.

[Fam Zheng]

Great work! The patch looks good for me, I made two trivial style
comments (if you respin, you can check your patches for style problems
by running scripts/checkpatch.pl).

On Fri, 07/05 18:35, Ian Main wrote:
> This patch adds sync-modes to the drive-backup interface and
> implements the FULL, NONE and TOP modes of synchronization.
> 
> FULL performs as before copying the entire contents of the drive
> while preserving the point-in-time using CoW.
> NONE only copies new writes to the target drive.
> TOP copies changes to the topmost drive image and preserves the
> point-in-time using CoW.
> 
> Signed-off-by: Ian Main <imain@redhat.com>
> ---
>  block/backup.c            | 90 +++++++++++++++++++++++++++++++----------------
>  blockdev.c                | 25 ++++++++-----
>  include/block/block_int.h |  4 ++-
>  qapi-schema.json          |  4 +++
>  qmp-commands.hx           |  1 +
>  5 files changed, 85 insertions(+), 39 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index 16105d4..e72a5af 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -37,6 +37,7 @@ typedef struct CowRequest {
>  typedef struct BackupBlockJob {
>      BlockJob common;
>      BlockDriverState *target;
> +    MirrorSyncMode sync_mode;
>      RateLimit limit;
>      BlockdevOnError on_source_error;
>      BlockdevOnError on_target_error;
> @@ -247,40 +248,68 @@ static void coroutine_fn backup_run(void *opaque)
>  
>      bdrv_add_before_write_notifier(bs, &before_write);
>  
> -    for (; start < end; start++) {
> -        bool error_is_read;
> -
> -        if (block_job_is_cancelled(&job->common)) {
> -            break;
> +    if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
> +        while (!block_job_is_cancelled(&job->common)) {
> +            /* Yield until the job is cancelled.  We just let our before_write
> +             * notify callback service CoW requests. */
> +            job->common.busy = false;
> +            qemu_coroutine_yield();
> +            job->common.busy = true;
>          }
> +    } else {
> +        /* Both FULL and TOP SYNC_MODE's require copying.. */
> +        for (; start < end; start++) {
> +            bool error_is_read;
>  
> -        /* we need to yield so that qemu_aio_flush() returns.
> -         * (without, VM does not reboot)
> -         */
> -        if (job->common.speed) {
> -            uint64_t delay_ns = ratelimit_calculate_delay(
> -                &job->limit, job->sectors_read);
> -            job->sectors_read = 0;
> -            block_job_sleep_ns(&job->common, rt_clock, delay_ns);
> -        } else {
> -            block_job_sleep_ns(&job->common, rt_clock, 0);
> -        }
> +            if (block_job_is_cancelled(&job->common)) {
> +                break;
> +            }
>  
> -        if (block_job_is_cancelled(&job->common)) {
> -            break;
> -        }
> +            /* we need to yield so that qemu_aio_flush() returns.
> +             * (without, VM does not reboot)
> +             */
> +            if (job->common.speed) {
> +                uint64_t delay_ns = ratelimit_calculate_delay(
> +                        &job->limit, job->sectors_read);
> +                job->sectors_read = 0;
> +                block_job_sleep_ns(&job->common, rt_clock, delay_ns);
> +            } else {
> +                block_job_sleep_ns(&job->common, rt_clock, 0);
> +            }
>  
> -        ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
> -                            BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
> -        if (ret < 0) {
> -            /* Depending on error action, fail now or retry cluster */
> -            BlockErrorAction action =
> -                backup_error_action(job, error_is_read, -ret);
> -            if (action == BDRV_ACTION_REPORT) {
> +            if (block_job_is_cancelled(&job->common)) {
>                  break;
> -            } else {
> -                start--;
> -                continue;
> +            }
> +
> +            if (job->sync_mode == MIRROR_SYNC_MODE_TOP) {
> +                int n, alloced;
> +
> +                /* Check to see if these blocks are already in the backing file. */
line over 80 characters
> +
> +                alloced =
> +                    bdrv_co_is_allocated(bs, start * BACKUP_SECTORS_PER_CLUSTER,
> +                                         BACKUP_SECTORS_PER_CLUSTER, &n);
> +                /* The above call returns true if the given sector is in the
> +                 * topmost image.  If that is the case then we must copy it as
> +                 * it has been modified from the original backing file. 
Trailing whitespace.

Thanks

-- 
Fam

Re: [Qemu-devel] [PATCH V2 0/2] Implement sync modes for drive-backup.

[Paolo Bonzini]

Il 06/07/2013 03:38, Ian Main ha scritto:
>> > This patch adds sync modes on top of the work that Stefan Hajnoczi has done.
>> > 
>> > These patches apply on kevin/block with 
>> > '[PATCH] block: add drive_backup HMP command' also applied.
>> > 
>> > Hopefully all is in order as this is my first QEMU patch.  Many thanks to 
>> > Stephan and Fam Zheng for their help.
>> > 
>> > V2:
>> > 
>> > - No longer poll, instead use qemu_coroutine_yield().
>> > - Use bdrv_co_is_allocated().
>> > - Much better SYNC_MODE_NONE test.
> FYI I'll be off camping for all next week so I won't get back to you
> until the following week.  Thanks in advance for any reviews!

I'm still not sure how "none" works (see my discussion with Fam in the
v1 thread).

Paolo