Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging

Block pull request

# gpg: Signature made Fri 31 Jan 2014 21:16:43 GMT using RSA key ID 81AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>"
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>"
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35  775A 9CA4 ABB3 81AB 73C8

* remotes/stefanha/tags/block-pull-request:
  qemu-iotests: only run 071 on qcow2
  dataplane: Comment fix
  block/vhdx: Error checking fixes
  qemu-iotests: Drop assert_no_active_commit in case 040
  block/vmdk: add basic .bdrv_check support
  block: remove qcow2 .bdrv_make_empty implementation
  block: remove QED .bdrv_make_empty implementation
  Describe flaws in qcow/qcow2 encryption in the docs
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>

block/qcow2.c

@@ -1689,26 +1689,6 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
     return ret;
 }
 
-static int qcow2_make_empty(BlockDriverState *bs)
-{
-#if 0
-    /* XXX: not correct */
-    BDRVQcowState *s = bs->opaque;
-    uint32_t l1_length = s->l1_size * sizeof(uint64_t);
-    int ret;
-
-    memset(s->l1_table, 0, l1_length);
-    if (bdrv_pwrite(bs->file, s->l1_table_offset, s->l1_table, l1_length) < 0)
-        return -1;
-    ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length);
-    if (ret < 0)
-        return ret;
-
-    l2_cache_reset(bs);
-#endif
-    return 0;
-}
-
 static coroutine_fn int qcow2_co_write_zeroes(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, BdrvRequestFlags flags)
 {
@@ -2252,7 +2232,6 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = qcow2_co_get_block_status,
     .bdrv_set_key       = qcow2_set_key,
-    .bdrv_make_empty    = qcow2_make_empty,
 
     .bdrv_co_readv          = qcow2_co_readv,
     .bdrv_co_writev         = qcow2_co_writev,

block/qed.c

@@ -731,11 +731,6 @@ static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
     return cb.status;
 }
 
-static int bdrv_qed_make_empty(BlockDriverState *bs)
-{
-    return -ENOTSUP;
-}
-
 static BDRVQEDState *acb_to_s(QEDAIOCB *acb)
 {
     return acb->common.bs->opaque;
@@ -1617,7 +1612,6 @@ static BlockDriver bdrv_qed = {
     .bdrv_create              = bdrv_qed_create,
     .bdrv_has_zero_init       = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = bdrv_qed_co_get_block_status,
-    .bdrv_make_empty          = bdrv_qed_make_empty,
     .bdrv_aio_readv           = bdrv_qed_aio_readv,
     .bdrv_aio_writev          = bdrv_qed_aio_writev,
     .bdrv_co_write_zeroes     = bdrv_qed_co_write_zeroes,

block/vhdx-log.c

@@ -965,7 +965,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s,
     cpu_to_le32s((uint32_t *)(buffer + 4));
 
     /* now write to the log */
-    vhdx_log_write_sectors(bs, &s->log, &sectors_written, buffer,
+    ret = vhdx_log_write_sectors(bs, &s->log, &sectors_written, buffer,
                                  desc_sectors + sectors);
     if (ret < 0) {
         goto exit;

block/vhdx.c

@@ -374,7 +374,7 @@ static int vhdx_update_header(BlockDriverState *bs, BDRVVHDXState *s,
         inactive_header->log_guid = *log_guid;
     }
 
-    vhdx_write_header(bs->file, inactive_header, header_offset, true);
+    ret = vhdx_write_header(bs->file, inactive_header, header_offset, true);
     if (ret < 0) {
         goto exit;
     }
@@ -1810,13 +1810,13 @@ static int vhdx_create(const char *filename, QEMUOptionParameter *options,
     creator = g_utf8_to_utf16("QEMU v" QEMU_VERSION, -1, NULL,
                               &creator_items, NULL);
     signature = cpu_to_le64(VHDX_FILE_SIGNATURE);
-    bdrv_pwrite(bs, VHDX_FILE_ID_OFFSET, &signature, sizeof(signature));
+    ret = bdrv_pwrite(bs, VHDX_FILE_ID_OFFSET, &signature, sizeof(signature));
     if (ret < 0) {
         goto delete_and_exit;
     }
     if (creator) {
-        bdrv_pwrite(bs, VHDX_FILE_ID_OFFSET + sizeof(signature), creator,
-                    creator_items * sizeof(gunichar2));
+        ret = bdrv_pwrite(bs, VHDX_FILE_ID_OFFSET + sizeof(signature),
+                          creator, creator_items * sizeof(gunichar2));
         if (ret < 0) {
             goto delete_and_exit;
         }

block/vmdk.c

@@ -1942,6 +1942,53 @@ static ImageInfo *vmdk_get_extent_info(VmdkExtent *extent)
     return info;
 }
 
+static int vmdk_check(BlockDriverState *bs, BdrvCheckResult *result,
+                      BdrvCheckMode fix)
+{
+    BDRVVmdkState *s = bs->opaque;
+    VmdkExtent *extent = NULL;
+    int64_t sector_num = 0;
+    int64_t total_sectors = bdrv_getlength(bs) / BDRV_SECTOR_SIZE;
+    int ret;
+    uint64_t cluster_offset;
+
+    if (fix) {
+        return -ENOTSUP;
+    }
+
+    for (;;) {
+        if (sector_num >= total_sectors) {
+            return 0;
+        }
+        extent = find_extent(s, sector_num, extent);
+        if (!extent) {
+            fprintf(stderr,
+                    "ERROR: could not find extent for sector %" PRId64 "\n",
+                    sector_num);
+            break;
+        }
+        ret = get_cluster_offset(bs, extent, NULL,
+                                 sector_num << BDRV_SECTOR_BITS,
+                                 0, &cluster_offset);
+        if (ret == VMDK_ERROR) {
+            fprintf(stderr,
+                    "ERROR: could not get cluster_offset for sector %"
+                    PRId64 "\n", sector_num);
+            break;
+        }
+        if (ret == VMDK_OK && cluster_offset >= bdrv_getlength(extent->file)) {
+            fprintf(stderr,
+                    "ERROR: cluster offset for sector %"
+                    PRId64 " points after EOF\n", sector_num);
+            break;
+        }
+        sector_num += extent->cluster_sectors;
+    }
+
+    result->corruptions++;
+    return 0;
+}
+
 static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs)
 {
     int i;
@@ -2015,6 +2062,7 @@ static BlockDriver bdrv_vmdk = {
     .instance_size                = sizeof(BDRVVmdkState),
     .bdrv_probe                   = vmdk_probe,
     .bdrv_open                    = vmdk_open,
+    .bdrv_check                   = vmdk_check,
     .bdrv_reopen_prepare          = vmdk_reopen_prepare,
     .bdrv_read                    = vmdk_co_read,
     .bdrv_write                   = vmdk_co_write,

hw/block/dataplane/virtio-blk.c

@@ -145,7 +145,7 @@ static void do_get_id_cmd(VirtIOBlockDataPlane *s,
 {
     char id[VIRTIO_BLK_ID_BYTES];
 
-    /* Serial number not NUL-terminated when shorter than buffer */
+    /* Serial number not NUL-terminated when longer than buffer */
     strncpy(id, s->blk->serial ? s->blk->serial : "", sizeof(id));
     iov_from_buf(iov, iov_cnt, 0, id, sizeof(id));
     complete_request_early(s, elem, inhdr, VIRTIO_BLK_S_OK);

qemu-doc.texi

@@ -547,10 +547,27 @@ File name of a base image (see @option{create} subcommand)
 @item backing_fmt
 Image format of the base image
 @item encryption
-If this option is set to @code{on}, the image is encrypted.
+If this option is set to @code{on}, the image is encrypted with 128-bit AES-CBC.
+
+The use of encryption in qcow and qcow2 images is considered to be flawed by
+modern cryptography standards, suffering from a number of design problems:
+
+@itemize @minus
+@item The AES-CBC cipher is used with predictable initialization vectors based
+on the sector number. This makes it vulnerable to chosen plaintext attacks
+which can reveal the existence of encrypted data.
+@item The user passphrase is directly used as the encryption key. A poorly
+chosen or short passphrase will compromise the security of the encryption.
+@item In the event of the passphrase being compromised there is no way to
+change the passphrase to protect data in any qcow images. The files must
+be cloned, using a different encryption passphrase in the new file. The
+original file must then be securely erased using a program like shred,
+though even this is ineffective with many modern storage technologies.
+@end itemize
 
-Encryption uses the AES format which is very secure (128 bit keys). Use
-a long password (16 characters) to get maximum protection.
+Use of qcow / qcow2 encryption is thus strongly discouraged. Users are
+recommended to use an alternative encryption technology such as the
+Linux dm-crypt / LUKS system.
 
 @item cluster_size
 Changes the qcow2 cluster size (must be between 512 and 2M). Smaller cluster

qemu-img.texi

@@ -409,10 +409,27 @@ File name of a base image (see @option{create} subcommand)
 @item backing_fmt
 Image format of the base image
 @item encryption
-If this option is set to @code{on}, the image is encrypted.
+If this option is set to @code{on}, the image is encrypted with 128-bit AES-CBC.
 
-Encryption uses the AES format which is very secure (128 bit keys). Use
-a long password (16 characters) to get maximum protection.
+The use of encryption in qcow and qcow2 images is considered to be flawed by
+modern cryptography standards, suffering from a number of design problems:
+
+@itemize @minus
+@item The AES-CBC cipher is used with predictable initialization vectors based
+on the sector number. This makes it vulnerable to chosen plaintext attacks
+which can reveal the existence of encrypted data.
+@item The user passphrase is directly used as the encryption key. A poorly
+chosen or short passphrase will compromise the security of the encryption.
+@item In the event of the passphrase being compromised there is no way to
+change the passphrase to protect data in any qcow images. The files must
+be cloned, using a different encryption passphrase in the new file. The
+original file must then be securely erased using a program like shred,
+though even this is ineffective with many modern storage technologies.
+@end itemize
+
+Use of qcow / qcow2 encryption is thus strongly discouraged. Users are
+recommended to use an alternative encryption technology such as the
+Linux dm-crypt / LUKS system.
 
 @item cluster_size
 Changes the qcow2 cluster size (must be between 512 and 2M). Smaller cluster

tests/qemu-iotests/040

@@ -35,12 +35,8 @@ test_img = os.path.join(iotests.test_dir, 'test.img')
 class ImageCommitTestCase(iotests.QMPTestCase):
     '''Abstract base class for image commit test cases'''
 
-    def assert_no_active_commit(self):
-        result = self.vm.qmp('query-block-jobs')
-        self.assert_qmp(result, 'return', [])
-
     def run_commit_test(self, top, base):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top=top, base=base)
         self.assert_qmp(result, 'return', {})
 
@@ -59,7 +55,7 @@ class ImageCommitTestCase(iotests.QMPTestCase):
                     self.assert_qmp(event, 'data/len', self.image_len)
                     self.vm.qmp('block-job-complete', device='drive0')
 
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         self.vm.shutdown()
 
 class TestSingleDrive(ImageCommitTestCase):
@@ -91,19 +87,19 @@ class TestSingleDrive(ImageCommitTestCase):
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
     def test_top_same_base(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % backing_img, base='%s' % backing_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % backing_img)
 
     def test_top_invalid(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='badfile', base='%s' % backing_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', 'Top image file badfile not found')
 
     def test_base_invalid(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % mid_img, base='badfile')
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found')
@@ -114,13 +110,13 @@ class TestSingleDrive(ImageCommitTestCase):
         self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
 
     def test_top_and_base_reversed(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % backing_img, base='%s' % mid_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % mid_img)
 
     def test_top_omitted(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0')
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', "Parameter 'top' is missing")
@@ -181,19 +177,19 @@ class TestRelativePaths(ImageCommitTestCase):
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
     def test_top_same_base(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.mid_img, base='%s' % self.mid_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % self.mid_img)
 
     def test_top_invalid(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='badfile', base='%s' % self.backing_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', 'Top image file badfile not found')
 
     def test_base_invalid(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.mid_img, base='badfile')
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found')
@@ -204,7 +200,7 @@ class TestRelativePaths(ImageCommitTestCase):
         self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
 
     def test_top_and_base_reversed(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.backing_img, base='%s' % self.mid_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % self.mid_img)
@@ -229,7 +225,7 @@ class TestSetSpeed(ImageCommitTestCase):
         os.remove(backing_img)
 
     def test_set_speed(self):
-        self.assert_no_active_commit()
+        self.assert_no_active_block_jobs()
 
         self.vm.pause_drive('drive0')
         result = self.vm.qmp('block-commit', device='drive0', top=mid_img, speed=1024 * 1024)

tests/qemu-iotests/071

@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.rc
 . ./common.filter
 
-_supported_fmt generic
+_supported_fmt qcow2
 _supported_proto generic
 _supported_os Linux