[Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Max Reitz]

The following changes since commit 9bb68d34dda9be60335e73e65c8fb61bca035362:

  Merge remote-tracking branch 'remotes/philmd-gitlab/tags/edk2-next-20190803' into staging (2019-08-05 11:05:36 +0100)

are available in the Git repository at:

  https://github.com/XanClic/qemu.git tags/pull-block-2019-08-05

for you to fetch changes up to 07b0851c592efe188a87259adbda26a63c61dc92:

  block/backup: disable copy_range for compressed backup (2019-08-05 18:05:05 +0200)

----------------------------------------------------------------
Block patches for 4.1.0-rc4:
- Fix the backup block job when using copy offloading
- Fix the mirror block job when using the write-blocking copy mode
- Fix incremental backups after the image has been grown with the
  respective bitmap attached to it

----------------------------------------------------------------
Max Reitz (5):
  backup: Copy only dirty areas
  iotests: Test backup job with two guest writes
  iotests: Test incremental backup after truncation
  mirror: Only mirror granularity-aligned chunks
  iotests: Test unaligned blocking mirror write

Vladimir Sementsov-Ogievskiy (2):
  util/hbitmap: update orig_size on truncate
  block/backup: disable copy_range for compressed backup

 block/backup.c             | 15 ++++++++++++---
 block/mirror.c             | 29 ++++++++++++++++++++++++++++
 util/hbitmap.c             |  6 +++++-
 tests/qemu-iotests/056     | 39 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/056.out |  4 ++--
 tests/qemu-iotests/124     | 38 +++++++++++++++++++++++++++++++++----
 tests/qemu-iotests/124.out |  4 ++--
 tests/qemu-iotests/151     | 25 ++++++++++++++++++++++++
 tests/qemu-iotests/151.out |  4 ++--
 9 files changed, 150 insertions(+), 14 deletions(-)

-- 
2.21.0

[Qemu-devel] [PULL 1/7] backup: Copy only dirty areas

[Max Reitz]

The backup job must only copy areas that the copy_bitmap reports as
dirty.  This is always the case when using traditional non-offloading
backup, because it copies each cluster separately.  When offloading the
copy operation, we sometimes copy more than one cluster at a time, but
we only check whether the first one is dirty.

Therefore, whenever copy offloading is possible, the backup job
currently produces wrong output when the guest writes to an area of
which an inner part has already been backed up, because that inner part
will be re-copied.

Fixes: 9ded4a0114968e98b41494fc035ba14f84cdf700
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190801173900.23851-2-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 715e1d3be8..1ee271f9f1 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -202,22 +202,31 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
     cow_request_begin(&cow_request, job, start, end);
 
     while (start < end) {
+        int64_t dirty_end;
+
         if (!hbitmap_get(job->copy_bitmap, start)) {
             trace_backup_do_cow_skip(job, start);
             start += job->cluster_size;
             continue; /* already copied */
         }
 
+        dirty_end = hbitmap_next_zero(job->copy_bitmap, start, (end - start));
+        if (dirty_end < 0) {
+            dirty_end = end;
+        }
+
         trace_backup_do_cow_process(job, start);
 
         if (job->use_copy_range) {
-            ret = backup_cow_with_offload(job, start, end, is_write_notifier);
+            ret = backup_cow_with_offload(job, start, dirty_end,
+                                          is_write_notifier);
             if (ret < 0) {
                 job->use_copy_range = false;
             }
         }
         if (!job->use_copy_range) {
-            ret = backup_cow_with_bounce_buffer(job, start, end, is_write_notifier,
+            ret = backup_cow_with_bounce_buffer(job, start, dirty_end,
+                                                is_write_notifier,
                                                 error_is_read, &bounce_buffer);
         }
         if (ret < 0) {
-- 
2.21.0

[Qemu-devel] [PULL 2/7] iotests: Test backup job with two guest writes

[Max Reitz]

Perform two guest writes to not yet backed up areas of an image, where
the former touches an inner area of the latter.

Before HEAD^, copy offloading broke this in two ways:
(1) The target image differs from the reference image (what the source
    was when the backup started).
(2) But you will not see that in the failing output, because the job
    offset is reported as being greater than the job length.  This is
    because one cluster is copied twice, and thus accounted for twice,
    but of course the job length does not increase.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190801173900.23851-3-mreitz@redhat.com
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/056     | 39 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/056.out |  4 ++--
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
index f40fc11a09..e761e465ae 100755
--- a/tests/qemu-iotests/056
+++ b/tests/qemu-iotests/056
@@ -133,6 +133,7 @@ class BackupTest(iotests.QMPTestCase):
         self.vm = iotests.VM()
         self.test_img = img_create('test')
         self.dest_img = img_create('dest')
+        self.ref_img = img_create('ref')
         self.vm.add_drive(self.test_img)
         self.vm.launch()
 
@@ -140,6 +141,7 @@ class BackupTest(iotests.QMPTestCase):
         self.vm.shutdown()
         try_remove(self.test_img)
         try_remove(self.dest_img)
+        try_remove(self.ref_img)
 
     def hmp_io_writes(self, drive, patterns):
         for pattern in patterns:
@@ -177,6 +179,43 @@ class BackupTest(iotests.QMPTestCase):
             self.assert_qmp(event, 'data/error', qerror)
             return False
 
+    def test_overlapping_writes(self):
+        # Write something to back up
+        self.hmp_io_writes('drive0', [('42', '0M', '2M')])
+
+        # Create a reference backup
+        self.qmp_backup_and_wait(device='drive0', format=iotests.imgfmt,
+                                 sync='full', target=self.ref_img,
+                                 auto_dismiss=False)
+        res = self.vm.qmp('block-job-dismiss', id='drive0')
+        self.assert_qmp(res, 'return', {})
+
+        # Now to the test backup: We simulate the following guest
+        # writes:
+        # (1) [1M + 64k, 1M + 128k): Afterwards, everything in that
+        #     area should be in the target image, and we must not copy
+        #     it again (because the source image has changed now)
+        #     (64k is the job's cluster size)
+        # (2) [1M, 2M): The backup job must not get overeager.  It
+        #     must copy [1M, 1M + 64k) and [1M + 128k, 2M) separately,
+        #     but not the area in between.
+
+        self.qmp_backup(device='drive0', format=iotests.imgfmt, sync='full',
+                        target=self.dest_img, speed=1, auto_dismiss=False)
+
+        self.hmp_io_writes('drive0', [('23', '%ik' % (1024 + 64), '64k'),
+                                      ('66', '1M', '1M')])
+
+        # Let the job complete
+        res = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
+        self.assert_qmp(res, 'return', {})
+        self.qmp_backup_wait('drive0')
+        res = self.vm.qmp('block-job-dismiss', id='drive0')
+        self.assert_qmp(res, 'return', {})
+
+        self.assertTrue(iotests.compare_images(self.ref_img, self.dest_img),
+                        'target image does not match reference image')
+
     def test_dismiss_false(self):
         res = self.vm.qmp('query-block-jobs')
         self.assert_qmp(res, 'return', [])
diff --git a/tests/qemu-iotests/056.out b/tests/qemu-iotests/056.out
index dae404e278..36376bed87 100644
--- a/tests/qemu-iotests/056.out
+++ b/tests/qemu-iotests/056.out
@@ -1,5 +1,5 @@
-.........
+..........
 ----------------------------------------------------------------------
-Ran 9 tests
+Ran 10 tests
 
 OK
-- 
2.21.0

[Qemu-devel] [PULL 3/7] util/hbitmap: update orig_size on truncate

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Without this, hbitmap_next_zero and hbitmap_next_dirty_area are broken
after truncate. So, orig_size is broken since it's introduction in
76d570dc495c56bb.

Fixes: 76d570dc495c56bb
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190805120120.23585-1-vsementsov@virtuozzo.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 util/hbitmap.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/util/hbitmap.c b/util/hbitmap.c
index 7905212a8b..bcc0acdc6a 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -53,7 +53,9 @@
  */
 
 struct HBitmap {
-    /* Size of the bitmap, as requested in hbitmap_alloc. */
+    /*
+     * Size of the bitmap, as requested in hbitmap_alloc or in hbitmap_truncate.
+     */
     uint64_t orig_size;
 
     /* Number of total bits in the bottom level.  */
@@ -732,6 +734,8 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size)
     uint64_t num_elements = size;
     uint64_t old;
 
+    hb->orig_size = size;
+
     /* Size comes in as logical elements, adjust for granularity. */
     size = (size + (1ULL << hb->granularity) - 1) >> hb->granularity;
     assert(size <= ((uint64_t)1 << HBITMAP_LOG_MAX_SIZE));
-- 
2.21.0

[Qemu-devel] [PULL 4/7] iotests: Test incremental backup after truncation

[Max Reitz]

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190805152840.32190-1-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/124     | 38 ++++++++++++++++++++++++++++++++++----
 tests/qemu-iotests/124.out |  4 ++--
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 80b356f7bb..3440f54781 100755
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -212,25 +212,28 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
         return bitmap
 
 
-    def prepare_backup(self, bitmap=None, parent=None):
+    def prepare_backup(self, bitmap=None, parent=None, **kwargs):
         if bitmap is None:
             bitmap = self.bitmaps[-1]
         if parent is None:
             parent, _ = bitmap.last_target()
 
         target, _ = bitmap.new_target()
-        self.img_create(target, bitmap.drive['fmt'], parent=parent)
+        self.img_create(target, bitmap.drive['fmt'], parent=parent,
+                        **kwargs)
         return target
 
 
     def create_incremental(self, bitmap=None, parent=None,
-                           parentFormat=None, validate=True):
+                           parentFormat=None, validate=True,
+                           target=None):
         if bitmap is None:
             bitmap = self.bitmaps[-1]
         if parent is None:
             parent, _ = bitmap.last_target()
 
-        target = self.prepare_backup(bitmap, parent)
+        if target is None:
+            target = self.prepare_backup(bitmap, parent)
         res = self.do_qmp_backup(job_id=bitmap.drive['id'],
                                  device=bitmap.drive['id'],
                                  sync='incremental', bitmap=bitmap.name,
@@ -572,6 +575,33 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
                           'bitmap0', self.drives[0],
                           granularity=64000)
 
+    def test_growing_before_backup(self):
+        '''
+        Test: Add a bitmap, truncate the image, write past the old
+              end, do a backup.
+
+        Incremental backup should not ignore dirty bits past the old
+        image end.
+        '''
+        self.assert_no_active_block_jobs()
+
+        self.create_anchor_backup()
+
+        self.add_bitmap('bitmap0', self.drives[0])
+
+        res = self.vm.qmp('block_resize', device=self.drives[0]['id'],
+                          size=(65 * 1048576))
+        self.assert_qmp(res, 'return', {})
+
+        # Dirty the image past the old end
+        self.vm.hmp_qemu_io(self.drives[0]['id'], 'write 64M 64k')
+
+        target = self.prepare_backup(size='65M')
+        self.create_incremental(target=target)
+
+        self.vm.shutdown()
+        self.check_backups()
+
 
 class TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
     '''Incremental backup tests that utilize a BlkDebug filter on drive0.'''
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 281b69efea..fa16b5ccef 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-............
+.............
 ----------------------------------------------------------------------
-Ran 12 tests
+Ran 13 tests
 
 OK
-- 
2.21.0

[Qemu-devel] [PULL 5/7] mirror: Only mirror granularity-aligned chunks

[Max Reitz]

In write-blocking mode, all writes to the top node directly go to the
target.  We must only mirror chunks of data that are aligned to the
job's granularity, because that is how the dirty bitmap works.
Therefore, the request alignment for writes must be the job's
granularity (in write-blocking mode).

Unfortunately, this forces all reads and writes to have the same
granularity (we only need this alignment for writes to the target, not
the source), but that is something to be fixed another time.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190805153308.2657-1-mreitz@redhat.com
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Fixes: d06107ade0ce74dc39739bac80de84b51ec18546
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/mirror.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 8cb75fb409..9f5c59ece1 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1481,6 +1481,15 @@ static void bdrv_mirror_top_child_perm(BlockDriverState *bs, BdrvChild *c,
     *nshared = BLK_PERM_ALL;
 }
 
+static void bdrv_mirror_top_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+    MirrorBDSOpaque *s = bs->opaque;
+
+    if (s && s->job && s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING) {
+        bs->bl.request_alignment = s->job->granularity;
+    }
+}
+
 /* Dummy node that provides consistent read to its users without requiring it
  * from its backing file and that allows writes on the backing file chain. */
 static BlockDriver bdrv_mirror_top = {
@@ -1493,6 +1502,7 @@ static BlockDriver bdrv_mirror_top = {
     .bdrv_co_block_status       = bdrv_co_block_status_from_backing,
     .bdrv_refresh_filename      = bdrv_mirror_top_refresh_filename,
     .bdrv_child_perm            = bdrv_mirror_top_child_perm,
+    .bdrv_refresh_limits        = bdrv_mirror_top_refresh_limits,
 };
 
 static BlockJob *mirror_start_job(
@@ -1637,6 +1647,25 @@ static BlockJob *mirror_start_job(
         s->should_complete = true;
     }
 
+    /*
+     * Must be called before we start tracking writes, but after
+     *
+     *     ((MirrorBlockJob *)
+     *         ((MirrorBDSOpaque *)
+     *             mirror_top_bs->opaque
+     *         )->job
+     *     )->copy_mode
+     *
+     * has the correct value.
+     * (We start tracking writes as of the following
+     * bdrv_create_dirty_bitmap() call.)
+     */
+    bdrv_refresh_limits(mirror_top_bs, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
     s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
     if (!s->dirty_bitmap) {
         goto fail;
-- 
2.21.0

[Qemu-devel] [PULL 6/7] iotests: Test unaligned blocking mirror write

[Max Reitz]

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190805113526.20319-1-mreitz@redhat.com
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/151     | 25 +++++++++++++++++++++++++
 tests/qemu-iotests/151.out |  4 ++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151
index 1bb74d67c4..ad7359fc8d 100755
--- a/tests/qemu-iotests/151
+++ b/tests/qemu-iotests/151
@@ -114,6 +114,31 @@ class TestActiveMirror(iotests.QMPTestCase):
     def testActiveIOFlushed(self):
         self.doActiveIO(True)
 
+    def testUnalignedActiveIO(self):
+        # Fill the source image
+        result = self.vm.hmp_qemu_io('source', 'write -P 1 0 2M')
+
+        # Start the block job (very slowly)
+        result = self.vm.qmp('blockdev-mirror',
+                             job_id='mirror',
+                             filter_node_name='mirror-node',
+                             device='source-node',
+                             target='target-node',
+                             sync='full',
+                             copy_mode='write-blocking',
+                             buf_size=(1048576 // 4),
+                             speed=1)
+        self.assert_qmp(result, 'return', {})
+
+        # Start an unaligned request to a dirty area
+        result = self.vm.hmp_qemu_io('source', 'write -P 2 %i 1' % (1048576 + 42))
+
+        # Let the job finish
+        result = self.vm.qmp('block-job-set-speed', device='mirror', speed=0)
+        self.assert_qmp(result, 'return', {})
+        self.complete_and_wait(drive='mirror')
+
+        self.potential_writes_in_flight = False
 
 
 if __name__ == '__main__':
diff --git a/tests/qemu-iotests/151.out b/tests/qemu-iotests/151.out
index fbc63e62f8..8d7e996700 100644
--- a/tests/qemu-iotests/151.out
+++ b/tests/qemu-iotests/151.out
@@ -1,5 +1,5 @@
-..
+...
 ----------------------------------------------------------------------
-Ran 2 tests
+Ran 3 tests
 
 OK
-- 
2.21.0

[Qemu-devel] [PULL 7/7] block/backup: disable copy_range for compressed backup

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Enabled by default copy_range ignores compress option. It's definitely
unexpected for user.

It's broken since introduction of copy_range usage in backup in
9ded4a011496.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190730163251.755248-3-vsementsov@virtuozzo.com
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/backup.c b/block/backup.c
index 1ee271f9f1..b26c22c4b8 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -657,7 +657,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     job->cluster_size = cluster_size;
     job->copy_bitmap = copy_bitmap;
     copy_bitmap = NULL;
-    job->use_copy_range = true;
+    job->use_copy_range = !compress; /* compression isn't supported for it */
     job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
                                         blk_get_max_transfer(job->target));
     job->copy_range_size = MAX(job->cluster_size,
-- 
2.21.0

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Vladimir Sementsov-Ogievskiy]

05.08.2019 19:37, Max Reitz wrote:
> The following changes since commit 9bb68d34dda9be60335e73e65c8fb61bca035362:
> 
>    Merge remote-tracking branch 'remotes/philmd-gitlab/tags/edk2-next-20190803' into staging (2019-08-05 11:05:36 +0100)
> 
> are available in the Git repository at:
> 
>    https://github.com/XanClic/qemu.git tags/pull-block-2019-08-05
> 
> for you to fetch changes up to 07b0851c592efe188a87259adbda26a63c61dc92:
> 
>    block/backup: disable copy_range for compressed backup (2019-08-05 18:05:05 +0200)
> 
> ----------------------------------------------------------------
> Block patches for 4.1.0-rc4:
> - Fix the backup block job when using copy offloading
> - Fix the mirror block job when using the write-blocking copy mode
> - Fix incremental backups after the image has been grown with the
>    respective bitmap attached to it
> 
> ----------------------------------------------------------------
> Max Reitz (5):
>    backup: Copy only dirty areas
>    iotests: Test backup job with two guest writes
>    iotests: Test incremental backup after truncation
>    mirror: Only mirror granularity-aligned chunks
>    iotests: Test unaligned blocking mirror write
> 
> Vladimir Sementsov-Ogievskiy (2):
>    util/hbitmap: update orig_size on truncate
>    block/backup: disable copy_range for compressed backup
> 

As I understand, this all should go to stable too? CC it.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 1544 bytes --]

On 05.08.19 18:59, Vladimir Sementsov-Ogievskiy wrote:
> 05.08.2019 19:37, Max Reitz wrote:
>> The following changes since commit 9bb68d34dda9be60335e73e65c8fb61bca035362:
>>
>>    Merge remote-tracking branch 'remotes/philmd-gitlab/tags/edk2-next-20190803' into staging (2019-08-05 11:05:36 +0100)
>>
>> are available in the Git repository at:
>>
>>    https://github.com/XanClic/qemu.git tags/pull-block-2019-08-05
>>
>> for you to fetch changes up to 07b0851c592efe188a87259adbda26a63c61dc92:
>>
>>    block/backup: disable copy_range for compressed backup (2019-08-05 18:05:05 +0200)
>>
>> ----------------------------------------------------------------
>> Block patches for 4.1.0-rc4:
>> - Fix the backup block job when using copy offloading
>> - Fix the mirror block job when using the write-blocking copy mode
>> - Fix incremental backups after the image has been grown with the
>>    respective bitmap attached to it
>>
>> ----------------------------------------------------------------
>> Max Reitz (5):
>>    backup: Copy only dirty areas
>>    iotests: Test backup job with two guest writes
>>    iotests: Test incremental backup after truncation
>>    mirror: Only mirror granularity-aligned chunks
>>    iotests: Test unaligned blocking mirror write
>>
>> Vladimir Sementsov-Ogievskiy (2):
>>    util/hbitmap: update orig_size on truncate
>>    block/backup: disable copy_range for compressed backup
>>
> 
> As I understand, this all should go to stable too? CC it.
Ah, yes.  Thanks.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Peter Maydell]

On Mon, 5 Aug 2019 at 17:37, Max Reitz <mreitz@redhat.com> wrote:
>
> The following changes since commit 9bb68d34dda9be60335e73e65c8fb61bca035362:
>
>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/edk2-next-20190803' into staging (2019-08-05 11:05:36 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/XanClic/qemu.git tags/pull-block-2019-08-05
>
> for you to fetch changes up to 07b0851c592efe188a87259adbda26a63c61dc92:
>
>   block/backup: disable copy_range for compressed backup (2019-08-05 18:05:05 +0200)
>
> ----------------------------------------------------------------
> Block patches for 4.1.0-rc4:
> - Fix the backup block job when using copy offloading
> - Fix the mirror block job when using the write-blocking copy mode
> - Fix incremental backups after the image has been grown with the
>   respective bitmap attached to it
>
> ----------------------------------------------------------------
> Max Reitz (5):
>   backup: Copy only dirty areas
>   iotests: Test backup job with two guest writes
>   iotests: Test incremental backup after truncation
>   mirror: Only mirror granularity-aligned chunks
>   iotests: Test unaligned blocking mirror write
>
> Vladimir Sementsov-Ogievskiy (2):
>   util/hbitmap: update orig_size on truncate
>   block/backup: disable copy_range for compressed backup
>
>  block/backup.c             | 15 ++++++++++++---
>  block/mirror.c             | 29 ++++++++++++++++++++++++++++
>  util/hbitmap.c             |  6 +++++-
>  tests/qemu-iotests/056     | 39 ++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/056.out |  4 ++--
>  tests/qemu-iotests/124     | 38 +++++++++++++++++++++++++++++++++----
>  tests/qemu-iotests/124.out |  4 ++--
>  tests/qemu-iotests/151     | 25 ++++++++++++++++++++++++
>  tests/qemu-iotests/151.out |  4 ++--
>  9 files changed, 150 insertions(+), 14 deletions(-)

This is quite a lot of changes for rc4 -- how confident are
you about them ? I suppose 3 out of 4 commits are updating
the test suite...

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Peter Maydell]

On Mon, 5 Aug 2019 at 19:05, Peter Maydell <peter.maydell@linaro.org> wrote:

> This is quite a lot of changes for rc4 -- how confident are
> you about them ? I suppose 3 out of 4 commits are updating
> the test suite...

3 out of 7, I meant :-)

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 3598 bytes --]

On 05.08.19 20:05, Peter Maydell wrote:
> On Mon, 5 Aug 2019 at 17:37, Max Reitz <mreitz@redhat.com> wrote:
>>
>> The following changes since commit 9bb68d34dda9be60335e73e65c8fb61bca035362:
>>
>>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/edk2-next-20190803' into staging (2019-08-05 11:05:36 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/XanClic/qemu.git tags/pull-block-2019-08-05
>>
>> for you to fetch changes up to 07b0851c592efe188a87259adbda26a63c61dc92:
>>
>>   block/backup: disable copy_range for compressed backup (2019-08-05 18:05:05 +0200)
>>
>> ----------------------------------------------------------------
>> Block patches for 4.1.0-rc4:
>> - Fix the backup block job when using copy offloading
>> - Fix the mirror block job when using the write-blocking copy mode
>> - Fix incremental backups after the image has been grown with the
>>   respective bitmap attached to it
>>
>> ----------------------------------------------------------------
>> Max Reitz (5):
>>   backup: Copy only dirty areas
>>   iotests: Test backup job with two guest writes
>>   iotests: Test incremental backup after truncation
>>   mirror: Only mirror granularity-aligned chunks
>>   iotests: Test unaligned blocking mirror write
>>
>> Vladimir Sementsov-Ogievskiy (2):
>>   util/hbitmap: update orig_size on truncate
>>   block/backup: disable copy_range for compressed backup
>>
>>  block/backup.c             | 15 ++++++++++++---
>>  block/mirror.c             | 29 ++++++++++++++++++++++++++++
>>  util/hbitmap.c             |  6 +++++-
>>  tests/qemu-iotests/056     | 39 ++++++++++++++++++++++++++++++++++++++
>>  tests/qemu-iotests/056.out |  4 ++--
>>  tests/qemu-iotests/124     | 38 +++++++++++++++++++++++++++++++++----
>>  tests/qemu-iotests/124.out |  4 ++--
>>  tests/qemu-iotests/151     | 25 ++++++++++++++++++++++++
>>  tests/qemu-iotests/151.out |  4 ++--
>>  9 files changed, 150 insertions(+), 14 deletions(-)
> 
> This is quite a lot of changes for rc4 -- how confident are
> you about them ? I suppose 3 out of 4 commits are updating
> the test suite...

Would dropping the test patches make it better? :-)

I am reasonably (i.e., rc4-levels of) confident that the patches don’t
break anything that wasn’t broken before.

(I’m least confident about the test patches working for everyone and
everywhere, as with all new test cases.  But it was my impression that
it’s always fine to include test case additions.)

Patch 1 is very important.  I’m very confident about it.
It fixes a silent corruption in the backup job, so I’m not too surprised
people haven’t noticed.  I would be surprised if really noone was
affected so far.

Patch 3 is not that important, but it is sufficiently simple, so I think
we should take it, even into rc4.

Patch 5 is very important for a specific mirror copying mode.  It can be
argued that nobody really uses this mode because otherwise somebody
should have noticed the corruption, because if you hit it, you will
simply lose data (as opposed to the backup case, where you will simply
get the wrong version of the data in the output image).
But that is why it’s so important.  I really don’t want anyone to hit it.
It is probably the most complicated patch here, but at any other point,
it would still be considered a simple patch.  (Just not quite trivial.)
I think it is worth taking it.

Patch 7 is actually not important.  But it’s an obvious trivial
one-liner.  I thought I might as well.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Peter Maydell]

On Mon, 5 Aug 2019 at 19:21, Max Reitz <mreitz@redhat.com> wrote:
> Would dropping the test patches make it better? :-)
>
> I am reasonably (i.e., rc4-levels of) confident that the patches don’t
> break anything that wasn’t broken before.
>
> (I’m least confident about the test patches working for everyone and
> everywhere, as with all new test cases.  But it was my impression that
> it’s always fine to include test case additions.)
>
> Patch 1 is very important.  I’m very confident about it.
> It fixes a silent corruption in the backup job, so I’m not too surprised
> people haven’t noticed.  I would be surprised if really noone was
> affected so far.
>
> Patch 3 is not that important, but it is sufficiently simple, so I think
> we should take it, even into rc4.
>
> Patch 5 is very important for a specific mirror copying mode.  It can be
> argued that nobody really uses this mode because otherwise somebody
> should have noticed the corruption, because if you hit it, you will
> simply lose data (as opposed to the backup case, where you will simply
> get the wrong version of the data in the output image).
> But that is why it’s so important.  I really don’t want anyone to hit it.
> It is probably the most complicated patch here, but at any other point,
> it would still be considered a simple patch.  (Just not quite trivial.)
> I think it is worth taking it.
>
> Patch 7 is actually not important.  But it’s an obvious trivial
> one-liner.  I thought I might as well.

Thanks for the clarifications -- these all sound worth taking.
The thing about rc4 is that we don't really have much chance
to find any problems with patches we put in at this point,
so I like to be pretty cautious.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Peter Maydell]

On Mon, 5 Aug 2019 at 18:01, Max Reitz <mreitz@redhat.com> wrote:
>
> On 05.08.19 18:59, Vladimir Sementsov-Ogievskiy wrote:
> > 05.08.2019 19:37, Max Reitz wrote:
> >> The following changes since commit 9bb68d34dda9be60335e73e65c8fb61bca035362:
> >>
> >>    Merge remote-tracking branch 'remotes/philmd-gitlab/tags/edk2-next-20190803' into staging (2019-08-05 11:05:36 +0100)
> >>
> >> are available in the Git repository at:
> >>
> >>    https://github.com/XanClic/qemu.git tags/pull-block-2019-08-05
> >>
> >> for you to fetch changes up to 07b0851c592efe188a87259adbda26a63c61dc92:
> >>
> >>    block/backup: disable copy_range for compressed backup (2019-08-05 18:05:05 +0200)
> >>
> >> ----------------------------------------------------------------
> >> Block patches for 4.1.0-rc4:
> >> - Fix the backup block job when using copy offloading
> >> - Fix the mirror block job when using the write-blocking copy mode
> >> - Fix incremental backups after the image has been grown with the
> >>    respective bitmap attached to it
> >>
> >> ----------------------------------------------------------------
> >> Max Reitz (5):
> >>    backup: Copy only dirty areas
> >>    iotests: Test backup job with two guest writes
> >>    iotests: Test incremental backup after truncation
> >>    mirror: Only mirror granularity-aligned chunks
> >>    iotests: Test unaligned blocking mirror write
> >>
> >> Vladimir Sementsov-Ogievskiy (2):
> >>    util/hbitmap: update orig_size on truncate
> >>    block/backup: disable copy_range for compressed backup
> >>
> >
> > As I understand, this all should go to stable too? CC it.
> Ah, yes.  Thanks.

Are you planning to send a respin with the CC:stable tags?
(I did a test merge of this version which all passed OK.)

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 1983 bytes --]

On 06.08.19 12:12, Peter Maydell wrote:
> On Mon, 5 Aug 2019 at 18:01, Max Reitz <mreitz@redhat.com> wrote:
>>
>> On 05.08.19 18:59, Vladimir Sementsov-Ogievskiy wrote:
>>> 05.08.2019 19:37, Max Reitz wrote:
>>>> The following changes since commit 9bb68d34dda9be60335e73e65c8fb61bca035362:
>>>>
>>>>    Merge remote-tracking branch 'remotes/philmd-gitlab/tags/edk2-next-20190803' into staging (2019-08-05 11:05:36 +0100)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>    https://github.com/XanClic/qemu.git tags/pull-block-2019-08-05
>>>>
>>>> for you to fetch changes up to 07b0851c592efe188a87259adbda26a63c61dc92:
>>>>
>>>>    block/backup: disable copy_range for compressed backup (2019-08-05 18:05:05 +0200)
>>>>
>>>> ----------------------------------------------------------------
>>>> Block patches for 4.1.0-rc4:
>>>> - Fix the backup block job when using copy offloading
>>>> - Fix the mirror block job when using the write-blocking copy mode
>>>> - Fix incremental backups after the image has been grown with the
>>>>    respective bitmap attached to it
>>>>
>>>> ----------------------------------------------------------------
>>>> Max Reitz (5):
>>>>    backup: Copy only dirty areas
>>>>    iotests: Test backup job with two guest writes
>>>>    iotests: Test incremental backup after truncation
>>>>    mirror: Only mirror granularity-aligned chunks
>>>>    iotests: Test unaligned blocking mirror write
>>>>
>>>> Vladimir Sementsov-Ogievskiy (2):
>>>>    util/hbitmap: update orig_size on truncate
>>>>    block/backup: disable copy_range for compressed backup
>>>>
>>>
>>> As I understand, this all should go to stable too? CC it.
>> Ah, yes.  Thanks.
> 
> Are you planning to send a respin with the CC:stable tags?
> (I did a test merge of this version which all passed OK.)

I thought Vladimir just meant to physically CC qemu-stable on the series
(which he did).  Should I respin with the tags?

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PULL 0/7] Block patches for 4.1.0-rc4

[Peter Maydell]

On Tue, 6 Aug 2019 at 12:12, Max Reitz <mreitz@redhat.com> wrote:
>
> On 06.08.19 12:12, Peter Maydell wrote:
> > On Mon, 5 Aug 2019 at 18:01, Max Reitz <mreitz@redhat.com> wrote:
> >>
> >> On 05.08.19 18:59, Vladimir Sementsov-Ogievskiy wrote:
> >>> 05.08.2019 19:37, Max Reitz wrote:
> >>>> The following changes since commit 9bb68d34dda9be60335e73e65c8fb61bca035362:
> >>>>
> >>>>    Merge remote-tracking branch 'remotes/philmd-gitlab/tags/edk2-next-20190803' into staging (2019-08-05 11:05:36 +0100)
> >>>>
> >>>> are available in the Git repository at:
> >>>>
> >>>>    https://github.com/XanClic/qemu.git tags/pull-block-2019-08-05
> >>>>
> >>>> for you to fetch changes up to 07b0851c592efe188a87259adbda26a63c61dc92:
> >>>>
> >>>>    block/backup: disable copy_range for compressed backup (2019-08-05 18:05:05 +0200)
> >>>>
> >>>> ----------------------------------------------------------------
> >>>> Block patches for 4.1.0-rc4:
> >>>> - Fix the backup block job when using copy offloading
> >>>> - Fix the mirror block job when using the write-blocking copy mode
> >>>> - Fix incremental backups after the image has been grown with the
> >>>>    respective bitmap attached to it
> >>>>
> >>>> ----------------------------------------------------------------
> >>>> Max Reitz (5):
> >>>>    backup: Copy only dirty areas
> >>>>    iotests: Test backup job with two guest writes
> >>>>    iotests: Test incremental backup after truncation
> >>>>    mirror: Only mirror granularity-aligned chunks
> >>>>    iotests: Test unaligned blocking mirror write
> >>>>
> >>>> Vladimir Sementsov-Ogievskiy (2):
> >>>>    util/hbitmap: update orig_size on truncate
> >>>>    block/backup: disable copy_range for compressed backup
> >>>>
> >>>
> >>> As I understand, this all should go to stable too? CC it.
> >> Ah, yes.  Thanks.
> >
> > Are you planning to send a respin with the CC:stable tags?
> > (I did a test merge of this version which all passed OK.)
>
> I thought Vladimir just meant to physically CC qemu-stable on the series
> (which he did).  Should I respin with the tags?

If you could do a quick respin that's probably most reliable --
I'm not sure exactly how the qemu-stable process works, though.

thanks
-- PMM