qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 10/36] util: add transactions.c


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v2 10/36] util: add transactions.c
Date: Mon, 18 Jan 2021 20:41:43 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1

18.01.2021 19:50, Kevin Wolf wrote:
Am 27.11.2020 um 15:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
Add simple transaction API to use in further update of block graph
operations.

Supposed usage is:

- "prepare" is main function of the action and it should make the main
   effect of the action to be visible for the following actions, keeping
   possibility of roll-back, saving necessary things in action state,
   which is prepended to the list. So, driver struct doesn't include
   "prepare" field, as it is supposed to be called directly.

So the convention is that tran_prepend() should be called by the
function that does the preparation?

yes.

Or would we call tran_prepend() and
do the actual action in different places?

- commit/rollback is supposed to be called for the list of action
   states, to commit/rollback all the actions in reverse order

- When possible "commit" should not make visible effect for other
   actions, which make possible transparent logical interaction between
   actions.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
  include/qemu/transactions.h | 46 +++++++++++++++++++++
  util/transactions.c         | 81 +++++++++++++++++++++++++++++++++++++
  util/meson.build            |  1 +
  3 files changed, 128 insertions(+)
  create mode 100644 include/qemu/transactions.h
  create mode 100644 util/transactions.c

diff --git a/include/qemu/transactions.h b/include/qemu/transactions.h
new file mode 100644
index 0000000000..a5b15f46ab
--- /dev/null
+++ b/include/qemu/transactions.h
@@ -0,0 +1,46 @@
+/*
+ * Simple transactions API
+ *
+ * Copyright (c) 2020 Virtuozzo International GmbH.
+ *
+ * Author:
+ *  Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_TRANSACTIONS_H
+#define QEMU_TRANSACTIONS_H
+
+#include <gmodule.h>
+
+typedef struct TransactionActionDrv {
+    void (*abort)(void *opeque);
+    void (*commit)(void *opeque);
+    void (*clean)(void *opeque);
+} TransactionActionDrv;

s/opeque/opaque/

+void tran_prepend(GSList **list, TransactionActionDrv *drv, void *opaque);
+void tran_abort(GSList *backup);
+void tran_commit(GSList *backup);

I'd add an empty line before a full function definition.

+static inline void tran_finalize(GSList *backup, int ret)
+{
+    if (ret < 0) {
+        tran_abort(backup);
+    } else {
+        tran_commit(backup);
+    }
+}

Let's use an opaque struct instead of GSList here and...

+#endif /* QEMU_TRANSACTIONS_H */
diff --git a/util/transactions.c b/util/transactions.c
new file mode 100644
index 0000000000..ef1b9a36a4
--- /dev/null
+++ b/util/transactions.c
@@ -0,0 +1,81 @@
+/*
+ * Simple transactions API
+ *
+ * Copyright (c) 2020 Virtuozzo International GmbH.
+ *
+ * Author:
+ *  Sementsov-Ogievskiy Vladimir <vsementsov@virtuozzo.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+
+#include "qemu/transactions.h"
+
+typedef struct BdrvAction {
+    TransactionActionDrv *drv;
+    void *opaque;
+} BdrvAction;

...add a QSLIST_ENTRY (or similar) here to make it a type-safe list.

The missing type safety of GSList means that we should avoid it whenever
it's easily possible (i.e. we know the number of lists in which an
element will be). Here, each BdrvAction will only be in a single list,
so typed lists should be simple enough.


OK


--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]