Add a class that will unify block parents for blockdev-replace
functionality we are going to add.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block-parent.h | 32 +++++++++++++++++
block/block-parent.c | 66 ++++++++++++++++++++++++++++++++++++
block/meson.build | 1 +
3 files changed, 99 insertions(+)
create mode 100644 include/block/block-parent.h
create mode 100644 block/block-parent.c
diff --git a/include/block/block-parent.h b/include/block/block-parent.h
new file mode 100644
index 0000000000..bd9c9d91e6
--- /dev/null
+++ b/include/block/block-parent.h
@@ -0,0 +1,32 @@
+/*
+ * Block Parent class
+ *
+ * Copyright (c) 2021 Virtuozzo International GmbH.
+ *
+ * Authors:
+ * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef BLOCK_PARENT_H
+#define BLOCK_PARENT_H
+
+#include "block/block.h"
+
+typedef struct BlockParentClass {
+ const char *name;
+
+ int (*find_child)(const char *parent_id, const char *child_name,
+ BlockDriverState *child_bs, BdrvChild **child,
+ Error **errp);
+ QTAILQ_ENTRY(BlockParentClass) next;
+} BlockParentClass;
+
+void block_parent_class_register(BlockParentClass *cls);
+
+BdrvChild *block_find_child(const char *parent_id, const char *child_name,
+ BlockDriverState *child_bs, Error **errp);
+
+#endif /* BLOCK_PARENT_H */
diff --git a/block/block-parent.c b/block/block-parent.c
new file mode 100644
index 0000000000..73b6026c42
--- /dev/null
+++ b/block/block-parent.c
@@ -0,0 +1,66 @@
+/*
+ * Block Parent class
+ *
+ * Copyright (c) 2021 Virtuozzo International GmbH.
+ *
+ * Authors:
+ * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "block/block-parent.h"
+#include "qapi/error.h"
+
+static QTAILQ_HEAD(, BlockParentClass) block_parent_classes =
+ QTAILQ_HEAD_INITIALIZER(block_parent_classes);
+
+void block_parent_class_register(BlockParentClass *cls)
+{
+ QTAILQ_INSERT_HEAD(&block_parent_classes, cls, next);
+}
+
+BdrvChild *block_find_child(const char *parent_id, const char *child_name,
+ BlockDriverState *child_bs, Error **errp)
+{
+ BdrvChild *found_child = NULL;
+ BlockParentClass *found_cls = NULL, *cls;
+
+ QTAILQ_FOREACH(cls, &block_parent_classes, next) {
+ int ret;
+ BdrvChild *c;
+
+ /*
+ * Note that .find_child must fail if parent is found but doesn't have
+ * corresponding child.
+ */
+ ret = cls->find_child(parent_id, child_name, child_bs, &c, errp);
+ if (ret < 0) {
+ return NULL;
+ }
+ if (ret == 0) {
+ continue;
+ }
+
+ if (!found_child) {
+ found_cls = cls;
+ found_child = c;
+ continue;
+ }
+
+ error_setg(errp, "{%s, %s} parent-child pair is ambiguous: it match "
+ "both %s and %s", parent_id, child_name, found_cls->name,
+ cls->name);
+ return NULL;
+ }
+
+ if (!found_child) {
+ error_setg(errp, "{%s, %s} parent-child pair not found", parent_id,
+ child_name);
+ return NULL;
+ }
+
+ return found_child;
+}
diff --git a/block/meson.build b/block/meson.build
index 0450914c7a..5200e0ffce 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -10,6 +10,7 @@ block_ss.add(files(
'blkverify.c',
'block-backend.c',
'block-copy.c',
+ 'block-parent.c',
'commit.c',
'copy-on-read.c',
'preallocate.c',
--
2.29.2
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> Add a class that will unify block parents for blockdev-replace
> functionality we are going to add.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/block/block-parent.h | 32 +++++++++++++++++
> block/block-parent.c | 66 ++++++++++++++++++++++++++++++++++++
> block/meson.build | 1 +
> 3 files changed, 99 insertions(+)
> create mode 100644 include/block/block-parent.h
> create mode 100644 block/block-parent.c
>
> diff --git a/include/block/block-parent.h b/include/block/block-parent.h
> new file mode 100644
> index 0000000000..bd9c9d91e6
> --- /dev/null
> +++ b/include/block/block-parent.h
> @@ -0,0 +1,32 @@
> +/*
> + * Block Parent class
> + *
> + * Copyright (c) 2021 Virtuozzo International GmbH.
> + *
> + * Authors:
> + * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef BLOCK_PARENT_H
> +#define BLOCK_PARENT_H
> +
> +#include "block/block.h"
> +
> +typedef struct BlockParentClass {
> + const char *name;
> +
> + int (*find_child)(const char *parent_id, const char *child_name,
> + BlockDriverState *child_bs, BdrvChild **child,
> + Error **errp);
Callbacks should come with a contract.
> + QTAILQ_ENTRY(BlockParentClass) next;
> +} BlockParentClass;
> +
> +void block_parent_class_register(BlockParentClass *cls);
> +
> +BdrvChild *block_find_child(const char *parent_id, const char *child_name,
> + BlockDriverState *child_bs, Error **errp);
> +
> +#endif /* BLOCK_PARENT_H */
> diff --git a/block/block-parent.c b/block/block-parent.c
> new file mode 100644
> index 0000000000..73b6026c42
> --- /dev/null
> +++ b/block/block-parent.c
> @@ -0,0 +1,66 @@
> +/*
> + * Block Parent class
> + *
> + * Copyright (c) 2021 Virtuozzo International GmbH.
> + *
> + * Authors:
> + * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "block/block-parent.h"
> +#include "qapi/error.h"
> +
> +static QTAILQ_HEAD(, BlockParentClass) block_parent_classes =
> + QTAILQ_HEAD_INITIALIZER(block_parent_classes);
> +
> +void block_parent_class_register(BlockParentClass *cls)
> +{
> + QTAILQ_INSERT_HEAD(&block_parent_classes, cls, next);
> +}
> +
> +BdrvChild *block_find_child(const char *parent_id, const char *child_name,
> + BlockDriverState *child_bs, Error **errp)
> +{
> + BdrvChild *found_child = NULL;
> + BlockParentClass *found_cls = NULL, *cls;
> +
> + QTAILQ_FOREACH(cls, &block_parent_classes, next) {
> + int ret;
> + BdrvChild *c;
> +
> + /*
> + * Note that .find_child must fail if parent is found but doesn't have
> + * corresponding child.
> + */
> + ret = cls->find_child(parent_id, child_name, child_bs, &c, errp);
> + if (ret < 0) {
> + return NULL;
> + }
> + if (ret == 0) {
> + continue;
> + }
> +
> + if (!found_child) {
> + found_cls = cls;
> + found_child = c;
> + continue;
> + }
> +
> + error_setg(errp, "{%s, %s} parent-child pair is ambiguous: it match "
> + "both %s and %s", parent_id, child_name, found_cls->name,
> + cls->name);
> + return NULL;
> + }
> +
> + if (!found_child) {
> + error_setg(errp, "{%s, %s} parent-child pair not found", parent_id,
> + child_name);
> + return NULL;
> + }
> +
> + return found_child;
> +}
Especially when the callback can return NULL with and without setting an
error!
> diff --git a/block/meson.build b/block/meson.build
> index 0450914c7a..5200e0ffce 100644
> --- a/block/meson.build
> +++ b/block/meson.build
> @@ -10,6 +10,7 @@ block_ss.add(files(
> 'blkverify.c',
> 'block-backend.c',
> 'block-copy.c',
> + 'block-parent.c',
> 'commit.c',
> 'copy-on-read.c',
> 'preallocate.c',
16.09.2021 11:34, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>
>> Add a class that will unify block parents for blockdev-replace
>> functionality we are going to add.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> include/block/block-parent.h | 32 +++++++++++++++++
>> block/block-parent.c | 66 ++++++++++++++++++++++++++++++++++++
>> block/meson.build | 1 +
>> 3 files changed, 99 insertions(+)
>> create mode 100644 include/block/block-parent.h
>> create mode 100644 block/block-parent.c
>>
>> diff --git a/include/block/block-parent.h b/include/block/block-parent.h
>> new file mode 100644
>> index 0000000000..bd9c9d91e6
>> --- /dev/null
>> +++ b/include/block/block-parent.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * Block Parent class
>> + *
>> + * Copyright (c) 2021 Virtuozzo International GmbH.
>> + *
>> + * Authors:
>> + * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef BLOCK_PARENT_H
>> +#define BLOCK_PARENT_H
>> +
>> +#include "block/block.h"
>> +
>> +typedef struct BlockParentClass {
>> + const char *name;
>> +
>> + int (*find_child)(const char *parent_id, const char *child_name,
>> + BlockDriverState *child_bs, BdrvChild **child,
>> + Error **errp);
>
> Callbacks should come with a contract.
will add
>
>> + QTAILQ_ENTRY(BlockParentClass) next;
>> +} BlockParentClass;
>> +
>> +void block_parent_class_register(BlockParentClass *cls);
>> +
>> +BdrvChild *block_find_child(const char *parent_id, const char *child_name,
>> + BlockDriverState *child_bs, Error **errp);
>> +
>> +#endif /* BLOCK_PARENT_H */
>> diff --git a/block/block-parent.c b/block/block-parent.c
>> new file mode 100644
>> index 0000000000..73b6026c42
>> --- /dev/null
>> +++ b/block/block-parent.c
>> @@ -0,0 +1,66 @@
>> +/*
>> + * Block Parent class
>> + *
>> + * Copyright (c) 2021 Virtuozzo International GmbH.
>> + *
>> + * Authors:
>> + * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "block/block-parent.h"
>> +#include "qapi/error.h"
>> +
>> +static QTAILQ_HEAD(, BlockParentClass) block_parent_classes =
>> + QTAILQ_HEAD_INITIALIZER(block_parent_classes);
>> +
>> +void block_parent_class_register(BlockParentClass *cls)
>> +{
>> + QTAILQ_INSERT_HEAD(&block_parent_classes, cls, next);
>> +}
>> +
>> +BdrvChild *block_find_child(const char *parent_id, const char *child_name,
>> + BlockDriverState *child_bs, Error **errp)
>> +{
>> + BdrvChild *found_child = NULL;
>> + BlockParentClass *found_cls = NULL, *cls;
>> +
>> + QTAILQ_FOREACH(cls, &block_parent_classes, next) {
>> + int ret;
>> + BdrvChild *c;
>> +
>> + /*
>> + * Note that .find_child must fail if parent is found but doesn't have
>> + * corresponding child.
>> + */
>> + ret = cls->find_child(parent_id, child_name, child_bs, &c, errp);
>> + if (ret < 0) {
>> + return NULL;
>> + }
>> + if (ret == 0) {
>> + continue;
>> + }
>> +
>> + if (!found_child) {
>> + found_cls = cls;
>> + found_child = c;
>> + continue;
>> + }
>> +
>> + error_setg(errp, "{%s, %s} parent-child pair is ambiguous: it match "
>> + "both %s and %s", parent_id, child_name, found_cls->name,
>> + cls->name);
>> + return NULL;
>> + }
>> +
>> + if (!found_child) {
>> + error_setg(errp, "{%s, %s} parent-child pair not found", parent_id,
>> + child_name);
>> + return NULL;
>> + }
>> +
>> + return found_child;
>> +}
>
> Especially when the callback can return NULL with and without setting an
> error!
Hmm. Callback returns int.
And this block_find_child() function return NULL only together with errp set.
>
>> diff --git a/block/meson.build b/block/meson.build
>> index 0450914c7a..5200e0ffce 100644
>> --- a/block/meson.build
>> +++ b/block/meson.build
>> @@ -10,6 +10,7 @@ block_ss.add(files(
>> 'blkverify.c',
>> 'block-backend.c',
>> 'block-copy.c',
>> + 'block-parent.c',
>> 'commit.c',
>> 'copy-on-read.c',
>> 'preallocate.c',
>
--
Best regards,
Vladimir
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> Add a class that will unify block parents for blockdev-replace
> functionality we are going to add.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/block/block-parent.h | 32 +++++++++++++++++
> block/block-parent.c | 66 ++++++++++++++++++++++++++++++++++++
> block/meson.build | 1 +
> 3 files changed, 99 insertions(+)
> create mode 100644 include/block/block-parent.h
> create mode 100644 block/block-parent.c
>
> diff --git a/include/block/block-parent.h b/include/block/block-parent.h
> new file mode 100644
> index 0000000000..bd9c9d91e6
> --- /dev/null
> +++ b/include/block/block-parent.h
> @@ -0,0 +1,32 @@
> +/*
> + * Block Parent class
> + *
> + * Copyright (c) 2021 Virtuozzo International GmbH.
> + *
> + * Authors:
> + * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef BLOCK_PARENT_H
> +#define BLOCK_PARENT_H
> +
> +#include "block/block.h"
> +
> +typedef struct BlockParentClass {
> + const char *name;
> +
> + int (*find_child)(const char *parent_id, const char *child_name,
> + BlockDriverState *child_bs, BdrvChild **child,
> + Error **errp);
> + QTAILQ_ENTRY(BlockParentClass) next;
> +} BlockParentClass;
> +
> +void block_parent_class_register(BlockParentClass *cls);
> +
> +BdrvChild *block_find_child(const char *parent_id, const char *child_name,
> + BlockDriverState *child_bs, Error **errp);
> +
> +#endif /* BLOCK_PARENT_H */
> diff --git a/block/block-parent.c b/block/block-parent.c
> new file mode 100644
> index 0000000000..73b6026c42
> --- /dev/null
> +++ b/block/block-parent.c
> @@ -0,0 +1,66 @@
> +/*
> + * Block Parent class
> + *
> + * Copyright (c) 2021 Virtuozzo International GmbH.
> + *
> + * Authors:
> + * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "block/block-parent.h"
> +#include "qapi/error.h"
> +
> +static QTAILQ_HEAD(, BlockParentClass) block_parent_classes =
> + QTAILQ_HEAD_INITIALIZER(block_parent_classes);
> +
> +void block_parent_class_register(BlockParentClass *cls)
> +{
> + QTAILQ_INSERT_HEAD(&block_parent_classes, cls, next);
> +}
> +
> +BdrvChild *block_find_child(const char *parent_id, const char *child_name,
> + BlockDriverState *child_bs, Error **errp)
> +{
> + BdrvChild *found_child = NULL;
> + BlockParentClass *found_cls = NULL, *cls;
> +
> + QTAILQ_FOREACH(cls, &block_parent_classes, next) {
> + int ret;
> + BdrvChild *c;
> +
> + /*
> + * Note that .find_child must fail if parent is found but doesn't have
> + * corresponding child.
> + */
> + ret = cls->find_child(parent_id, child_name, child_bs, &c, errp);
> + if (ret < 0) {
> + return NULL;
Interesting: when one method fails, the entire function fails, even when
other methods succeed. The contract should probably spell this out.
> + }
> + if (ret == 0) {
> + continue;
> + }
> +
> + if (!found_child) {
> + found_cls = cls;
> + found_child = c;
> + continue;
> + }
> +
> + error_setg(errp, "{%s, %s} parent-child pair is ambiguous: it match "
> + "both %s and %s", parent_id, child_name, found_cls->name,
> + cls->name);
> + return NULL;
> + }
Style recommendation: if very much prefer
if bad
error out
normal
over
if ok
normal
bad
In this case:
if (found_child) {
error_setg(...);
return 0;
}
found_cls = cls;
found_child = c;
}
> +
> + if (!found_child) {
> + error_setg(errp, "{%s, %s} parent-child pair not found", parent_id,
> + child_name);
> + return NULL;
> + }
> +
> + return found_child;
> +}
> diff --git a/block/meson.build b/block/meson.build
> index 0450914c7a..5200e0ffce 100644
> --- a/block/meson.build
> +++ b/block/meson.build
> @@ -10,6 +10,7 @@ block_ss.add(files(
> 'blkverify.c',
> 'block-backend.c',
> 'block-copy.c',
> + 'block-parent.c',
> 'commit.c',
> 'copy-on-read.c',
> 'preallocate.c',
© 2016 - 2026 Red Hat, Inc.