Re: [Qemu-devel] [PATCH v6 11/20] block: vhdx write support
On Wed, Sep 25, 2013 at 05:02:56PM -0400, Jeff Cody wrote:
@@ -1070,7 +1070,43 @@ exit:
return ret;
}
+/*
+ * Allocate a new payload block at the end of the file.
+ *
+ * Allocation will happen at 1MB alignment inside the file
+ *
+ * Returns the file offset start of the new payload block
+ */
+static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
+uint64_t *new_offset)
+{
+*new_offset = bdrv_getlength(bs-file);
+
+/* per the spec, the address for a block is in units of 1MB */
+*new_offset = ROUND_UP(*new_offset, 1024*1024);
+
+return bdrv_truncate(bs-file, *new_offset + s-block_size);
+}
+
+/*
+ * Update the BAT tablet entry with the new file offset, and the new entry
s/tablet/table/
+ * state */
+static void vhdx_update_bat_table_entry(BlockDriverState *bs, BDRVVHDXState
*s,
+ VHDXSectorInfo *sinfo,
+ uint64_t *bat_entry,
+ uint64_t *bat_offset, int state)
+{
+/* The BAT entry is a uint64, with 44 bits for the file offset in units
of
+ * 1MB, and 3 bits for the block state. */
+s-bat[sinfo-bat_idx] = ((sinfo-file_offset20)
QEMU coding style requires space around the ''. Please run
scripts/checkpatch.pl.
+ VHDX_BAT_FILE_OFF_BITS);
+s-bat[sinfo-bat_idx] |= state VHDX_BAT_STATE_BIT_MASK;
+
+*bat_entry = cpu_to_le64(s-bat[sinfo-bat_idx]);
Call this argument bat_entry_le so callers know now to manipulate the
little-endian value directly?
+*bat_offset = s-bat_offset + sinfo-bat_idx * sizeof(VHDXBatEntry);
+
+}
/* Per the spec, on the first write of guest-visible data to the file the
* data write guid must be updated in the header */
@@ -1087,7 +1123,117 @@ int vhdx_user_visible_write(BlockDriverState *bs,
BDRVVHDXState *s)
static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t
sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
-return -ENOTSUP;
+int ret = -ENOTSUP;
+BDRVVHDXState *s = bs-opaque;
+VHDXSectorInfo sinfo;
+uint64_t bytes_done = 0;
+uint64_t bat_entry = 0;
+uint64_t bat_entry_offset = 0;
+bool bat_update;
+QEMUIOVector hd_qiov;
+
+qemu_iovec_init(hd_qiov, qiov-niov);
+
+qemu_co_mutex_lock(s-lock);
+
+ret = vhdx_user_visible_write(bs, s);
+if (ret 0) {
+goto exit;
+}
+
+while (nb_sectors 0) {
+if (s-params.data_bits VHDX_PARAMS_HAS_PARENT) {
+/* not supported yet */
+ret = -ENOTSUP;
+goto exit;
+} else {
+bat_update = false;
+vhdx_block_translate(s, sector_num, nb_sectors, sinfo);
+
+qemu_iovec_reset(hd_qiov);
+qemu_iovec_concat(hd_qiov, qiov, bytes_done,
sinfo.bytes_avail);
+/* check the payload block state */
+switch (s-bat[sinfo.bat_idx] VHDX_BAT_STATE_BIT_MASK) {
+case PAYLOAD_BLOCK_ZERO:
+/* in this case, we need to preserve zero writes for
+ * data that is not part of this write, so we must pad
+ * the rest of the buffer to zeroes */
+
+/* if we are on a posix system with ftruncate() that extends
+ * a file, then it is zero-filled for us. On Win32, the raw
+ * layer uses SetFilePointer and SetFileEnd, which does not
+ * zero fill AFAIK */
+
+/* TODO: queue another write of zero buffers if the host OS
does
+ * not zero-fill on file extension */
This is never addressed in the series.
+
+/* fall through */
+case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
+case PAYLOAD_BLOCK_UNMAPPED:/* fall through */
+case PAYLOAD_BLOCK_UNDEFINED: /* fall through */
+ret = vhdx_allocate_block(bs, s, sinfo.file_offset);
+if (ret 0) {
+goto exit;
+}
+/* once we support differencing files, this may also be
+ * partially present */
+/* update block state to the newly specified state */
+vhdx_update_bat_table_entry(bs, s, sinfo, bat_entry,
+bat_entry_offset,
+PAYLOAD_BLOCK_FULL_PRESENT);
In the specification the constant is called PAYLOAD_BLOCK_FULLY_PRESENT.
Is s/FULLY/FULL/ intentional and can you match the spec for consistency?
+bat_update = true;
+/* since we just allocated a block, file_offset is the
+ * beginning of the payload block. It needs to be the
+ * write address, which includes
Re: [Qemu-devel] [PATCH v6 11/20] block: vhdx write support
On Tue, Oct 01, 2013 at 03:29:17PM +0200, Stefan Hajnoczi wrote:
On Wed, Sep 25, 2013 at 05:02:56PM -0400, Jeff Cody wrote:
@@ -1070,7 +1070,43 @@ exit:
return ret;
}
+/*
+ * Allocate a new payload block at the end of the file.
+ *
+ * Allocation will happen at 1MB alignment inside the file
+ *
+ * Returns the file offset start of the new payload block
+ */
+static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
+uint64_t *new_offset)
+{
+*new_offset = bdrv_getlength(bs-file);
+
+/* per the spec, the address for a block is in units of 1MB */
+*new_offset = ROUND_UP(*new_offset, 1024*1024);
+
+return bdrv_truncate(bs-file, *new_offset + s-block_size);
+}
+
+/*
+ * Update the BAT tablet entry with the new file offset, and the new entry
s/tablet/table/
+ * state */
+static void vhdx_update_bat_table_entry(BlockDriverState *bs,
BDRVVHDXState *s,
+ VHDXSectorInfo *sinfo,
+ uint64_t *bat_entry,
+ uint64_t *bat_offset, int state)
+{
+/* The BAT entry is a uint64, with 44 bits for the file offset in
units of
+ * 1MB, and 3 bits for the block state. */
+s-bat[sinfo-bat_idx] = ((sinfo-file_offset20)
QEMU coding style requires space around the ''. Please run
scripts/checkpatch.pl.
I missed that - but I did run checkpatch.pl, it did not catch it:
vhdx-v6/0011-block-vhdx-write-support.patch has no obvious style problems and
is ready for submission.
total: 0 errors, 0 warnings, 31 lines checked
I think the parenthesis hid it.
+ VHDX_BAT_FILE_OFF_BITS);
+s-bat[sinfo-bat_idx] |= state VHDX_BAT_STATE_BIT_MASK;
+
+*bat_entry = cpu_to_le64(s-bat[sinfo-bat_idx]);
Call this argument bat_entry_le so callers know now to manipulate the
little-endian value directly?
Yes, good idea.
+*bat_offset = s-bat_offset + sinfo-bat_idx * sizeof(VHDXBatEntry);
+
+}
/* Per the spec, on the first write of guest-visible data to the file the
* data write guid must be updated in the header */
@@ -1087,7 +1123,117 @@ int vhdx_user_visible_write(BlockDriverState *bs,
BDRVVHDXState *s)
static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t
sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
-return -ENOTSUP;
+int ret = -ENOTSUP;
+BDRVVHDXState *s = bs-opaque;
+VHDXSectorInfo sinfo;
+uint64_t bytes_done = 0;
+uint64_t bat_entry = 0;
+uint64_t bat_entry_offset = 0;
+bool bat_update;
+QEMUIOVector hd_qiov;
+
+qemu_iovec_init(hd_qiov, qiov-niov);
+
+qemu_co_mutex_lock(s-lock);
+
+ret = vhdx_user_visible_write(bs, s);
+if (ret 0) {
+goto exit;
+}
+
+while (nb_sectors 0) {
+if (s-params.data_bits VHDX_PARAMS_HAS_PARENT) {
+/* not supported yet */
+ret = -ENOTSUP;
+goto exit;
+} else {
+bat_update = false;
+vhdx_block_translate(s, sector_num, nb_sectors, sinfo);
+
+qemu_iovec_reset(hd_qiov);
+qemu_iovec_concat(hd_qiov, qiov, bytes_done,
sinfo.bytes_avail);
+/* check the payload block state */
+switch (s-bat[sinfo.bat_idx] VHDX_BAT_STATE_BIT_MASK) {
+case PAYLOAD_BLOCK_ZERO:
+/* in this case, we need to preserve zero writes for
+ * data that is not part of this write, so we must pad
+ * the rest of the buffer to zeroes */
+
+/* if we are on a posix system with ftruncate() that
extends
+ * a file, then it is zero-filled for us. On Win32, the
raw
+ * layer uses SetFilePointer and SetFileEnd, which does not
+ * zero fill AFAIK */
+
+/* TODO: queue another write of zero buffers if the host
OS does
+ * not zero-fill on file extension */
This is never addressed in the series.
I will address it in v7
+
+/* fall through */
+case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
+case PAYLOAD_BLOCK_UNMAPPED:/* fall through */
+case PAYLOAD_BLOCK_UNDEFINED: /* fall through */
+ret = vhdx_allocate_block(bs, s, sinfo.file_offset);
+if (ret 0) {
+goto exit;
+}
+/* once we support differencing files, this may also be
+ * partially present */
+/* update block state to the newly specified state */
+vhdx_update_bat_table_entry(bs, s, sinfo, bat_entry,
+
