The iov_discard_front/back() operations are useful for parsing iovecs
but they modify the array elements. If the original array is needed
after parsing finishes there is currently no way to restore it.
Although g_memdup() can be used before performing destructive
iov_discard_front/back() operations, this is inefficient.
Introduce iov_discard_undo() to restore the array to the state prior to
an iov_discard_front/back() operation.
Seems there are some errors. See below
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/qemu/iov.h | 23 +++++++
tests/test-iov.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
util/iov.c | 50 ++++++++++++--
3 files changed, 234 insertions(+), 4 deletions(-)
diff --git a/include/qemu/iov.h b/include/qemu/iov.h
index bffc151282..b6b283a5e5 100644
--- a/include/qemu/iov.h
+++ b/include/qemu/iov.h
@@ -130,6 +130,29 @@ size_t iov_discard_front(struct iovec **iov, unsigned int *iov_cnt,
size_t iov_discard_back(struct iovec *iov, unsigned int *iov_cnt,
size_t bytes);
+/* Information needed to undo an iov_discard_*() operation */
+typedef struct {
+ struct iovec *modified_iov;
+ struct iovec orig;
+} IOVDiscardUndo;
+
+/*
+ * Undo an iov_discard_front_undoable() or iov_discard_back_undoable()
+ * operation. If multiple operations are made then each one needs a separate
+ * IOVDiscardUndo and iov_discard_undo() must be called in the reverse order
+ * that the operations were made.
+ */
+void iov_discard_undo(IOVDiscardUndo *undo);
+
+/*
+ * Undoable versions of iov_discard_front() and iov_discard_back(). Use
+ * iov_discard_undo() to reset to the state before the discard operations.
+ */
+size_t iov_discard_front_undoable(struct iovec **iov, unsigned int *iov_cnt,
+ size_t bytes, IOVDiscardUndo *undo);
+size_t iov_discard_back_undoable(struct iovec *iov, unsigned int *iov_cnt,
+ size_t bytes, IOVDiscardUndo *undo);
+
typedef struct QEMUIOVector {
struct iovec *iov;
int niov;
diff --git a/tests/test-iov.c b/tests/test-iov.c
index 458ca25099..9c415e2f1f 100644
--- a/tests/test-iov.c
+++ b/tests/test-iov.c
@@ -26,6 +26,12 @@ static void iov_free(struct iovec *iov, unsigned niov)
g_free(iov);
}
+static bool iov_equals(const struct iovec *a, const struct iovec *b,
+ unsigned niov)
+{
+ return memcmp(a, b, sizeof(a[0]) * niov) == 0;
+}
+
static void test_iov_bytes(struct iovec *iov, unsigned niov,
size_t offset, size_t bytes)
{
@@ -335,6 +341,87 @@ static void test_discard_front(void)
iov_free(iov, iov_cnt);
}
+static void test_discard_front_undo(void)
+{
+ IOVDiscardUndo undo;
+ struct iovec *iov;
+ struct iovec *iov_tmp;
+ struct iovec *iov_orig;
+ unsigned int iov_cnt;
+ unsigned int iov_cnt_tmp;
+ size_t size;
+
+ /* Discard zero bytes */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_tmp = iov;
+ iov_cnt_tmp = iov_cnt;
+ iov_discard_front_undoable(&iov_tmp, &iov_cnt_tmp, 0, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard more bytes than vector size */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_tmp = iov;
+ iov_cnt_tmp = iov_cnt;
+ size = iov_size(iov, iov_cnt);
+ iov_discard_front_undoable(&iov_tmp, &iov_cnt_tmp, size + 1, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
The 'iov_discard_front_undoable' will change the 'iov_tmp' it will not touch 'iov_orig'.
So the test will be always passed. The actually function will not be tested.
Also, maybe we could abstract a function to do these discard test?
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard entire vector */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_tmp = iov;
+ iov_cnt_tmp = iov_cnt;
+ size = iov_size(iov, iov_cnt);
+ iov_discard_front_undoable(&iov_tmp, &iov_cnt_tmp, size, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard within first element */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_tmp = iov;
+ iov_cnt_tmp = iov_cnt;
+ size = g_test_rand_int_range(1, iov->iov_len);
+ iov_discard_front_undoable(&iov_tmp, &iov_cnt_tmp, size, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard entire first element */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_tmp = iov;
+ iov_cnt_tmp = iov_cnt;
+ iov_discard_front_undoable(&iov_tmp, &iov_cnt_tmp, iov->iov_len, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard within second element */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_tmp = iov;
+ iov_cnt_tmp = iov_cnt;
+ size = iov->iov_len + g_test_rand_int_range(1, iov[1].iov_len);
+ iov_discard_front_undoable(&iov_tmp, &iov_cnt_tmp, size, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+}
+
static void test_discard_back(void)
{
struct iovec *iov;
@@ -404,6 +491,82 @@ static void test_discard_back(void)
iov_free(iov, iov_cnt);
}
+static void test_discard_back_undo(void)
+{
+ IOVDiscardUndo undo;
+ struct iovec *iov;
+ struct iovec *iov_orig;
+ unsigned int iov_cnt;
+ unsigned int iov_cnt_tmp;
+ size_t size;
+
+ /* Discard zero bytes */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_cnt_tmp = iov_cnt;
+ iov_discard_back_undoable(iov, &iov_cnt_tmp, 0, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard more bytes than vector size */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_cnt_tmp = iov_cnt;
+ size = iov_size(iov, iov_cnt);
+ iov_discard_back_undoable(iov, &iov_cnt_tmp, size + 1, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard entire vector */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_cnt_tmp = iov_cnt;
+ size = iov_size(iov, iov_cnt);
+ iov_discard_back_undoable(iov, &iov_cnt_tmp, size, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard within last element */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_cnt_tmp = iov_cnt;
+ size = g_test_rand_int_range(1, iov[iov_cnt - 1].iov_len);
+ iov_discard_back_undoable(iov, &iov_cnt_tmp, size, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard entire last element */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_cnt_tmp = iov_cnt;
+ size = iov[iov_cnt - 1].iov_len;
+ iov_discard_back_undoable(iov, &iov_cnt_tmp, size, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+
+ /* Discard within second-to-last element */
+ iov_random(&iov, &iov_cnt);
+ iov_orig = g_memdup(iov, sizeof(iov[0]) * iov_cnt);
+ iov_cnt_tmp = iov_cnt;
+ size = iov[iov_cnt - 1].iov_len +
+ g_test_rand_int_range(1, iov[iov_cnt - 2].iov_len);
+ iov_discard_back_undoable(iov, &iov_cnt_tmp, size, &undo);
+ iov_discard_undo(&undo);
+ assert(iov_equals(iov, iov_orig, iov_cnt));
+ g_free(iov_orig);
+ iov_free(iov, iov_cnt);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -412,5 +575,7 @@ int main(int argc, char **argv)
g_test_add_func("/basic/iov/io", test_io);
g_test_add_func("/basic/iov/discard-front", test_discard_front);
g_test_add_func("/basic/iov/discard-back", test_discard_back);
+ g_test_add_func("/basic/iov/discard-front-undo", test_discard_front_undo);
+ g_test_add_func("/basic/iov/discard-back-undo", test_discard_back_undo);
return g_test_run();
}
diff --git a/util/iov.c b/util/iov.c
index 45ef3043ee..efcf04b445 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -636,14 +636,33 @@ void qemu_iovec_clone(QEMUIOVector *dest, const QEMUIOVector *src, void *buf)
}
}
-size_t iov_discard_front(struct iovec **iov, unsigned int *iov_cnt,
- size_t bytes)
+void iov_discard_undo(IOVDiscardUndo *undo)
+{
+ /* Restore original iovec if it was modified */
+ if (undo->modified_iov) {
+ *undo->modified_iov = undo->orig;
+ }
+}
+
+size_t iov_discard_front_undoable(struct iovec **iov,
+ unsigned int *iov_cnt,
+ size_t bytes,
+ IOVDiscardUndo *undo)
{
size_t total = 0;
struct iovec *cur;
+ if (undo) {
+ undo->modified_iov = NULL;
+ }
+
for (cur = *iov; *iov_cnt > 0; cur++) {
if (cur->iov_len > bytes) {
+ if (undo) {
+ undo->modified_iov = cur;
+ undo->orig = *cur;
+ }
+
Why here we remember the 'cur'? 'cur' is the some of the 'iov'.
Maybe we remember the 'iov'. I think we need the undo structure like this:
struct IOVUndo {
iov **modified_iov;
iov *orig;
};
Then we can remeber the origin 'iov'.
Thanks,
Li Qiang
cur->iov_base += bytes;
cur->iov_len -= bytes;
total += bytes;
@@ -659,12 +678,24 @@ size_t iov_discard_front(struct iovec **iov, unsigned int *iov_cnt,
return total;
}
-size_t iov_discard_back(struct iovec *iov, unsigned int *iov_cnt,
- size_t bytes)
+size_t iov_discard_front(struct iovec **iov, unsigned int *iov_cnt,
+ size_t bytes)
+{
+ return iov_discard_front_undoable(iov, iov_cnt, bytes, NULL);
+}
+
+size_t iov_discard_back_undoable(struct iovec *iov,
+ unsigned int *iov_cnt,
+ size_t bytes,
+ IOVDiscardUndo *undo)
{
size_t total = 0;
struct iovec *cur;
+ if (undo) {
+ undo->modified_iov = NULL;
+ }
+
if (*iov_cnt == 0) {
return 0;
}
@@ -673,6 +704,11 @@ size_t iov_discard_back(struct iovec *iov, unsigned int *iov_cnt,
while (*iov_cnt > 0) {
if (cur->iov_len > bytes) {
+ if (undo) {
+ undo->modified_iov = cur;
+ undo->orig = *cur;
+ }
+
cur->iov_len -= bytes;
total += bytes;
break;
@@ -687,6 +723,12 @@ size_t iov_discard_back(struct iovec *iov, unsigned int *iov_cnt,
return total;
}
+size_t iov_discard_back(struct iovec *iov, unsigned int *iov_cnt,
+ size_t bytes)
+{
+ return iov_discard_back_undoable(iov, iov_cnt, bytes, NULL);
+}
+
void qemu_iovec_discard_back(QEMUIOVector *qiov, size_t bytes)
{
size_t total;
--
2.26.2