[Qemu-devel] Re: [PATCH 07/13] blockdev: Means to destroy blockdev only if made with drive_init()
Am 02.06.2010 18:55, schrieb Markus Armbruster:
All drives are still made that way. They get destroyed along with
their device. That's inappropriate for the alternative way to make
blockdevs that will appear later in this series. These won't have a
DriveInfo.
blockdev_detach() destroys the blockdev only if it has a DriveInfo.
blockdev_attach() does nothing for now. It'll be fleshed out later.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
blockdev.c | 35 +++
blockdev.h |7 +++
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index ace74e4..f90d4fc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1,8 +1,12 @@
/*
* QEMU host block devices
*
+ * Copyright (C) 2010 Red Hat Inc.
* Copyright (c) 2003-2008 Fabrice Bellard
*
+ * Authors:
+ * Markus Armbruster arm...@redhat.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.
*/
@@ -17,6 +21,37 @@
static QTAILQ_HEAD(drivelist, DriveInfo) drives =
QTAILQ_HEAD_INITIALIZER(drives);
+static int blockdev_del_dinfo(BlockDriverState *bs)
+{
+DriveInfo *dinfo, *next_dinfo;
+int res = 0;
+
+QTAILQ_FOREACH_SAFE(dinfo, drives, next, next_dinfo) {
+if (dinfo-bdrv == bs) {
+qemu_opts_del(dinfo-opts);
+QTAILQ_REMOVE(drives, dinfo, next);
+qemu_free(dinfo);
+res = 1;
+}
+}
+
+return res;
Can it happen that a BlockDriverState belongs to multiple DriveInfos? If
no, why not returning in the loop? Wouldn't need a FOREACH_SAFE then, too.
It's not worth respinning because of this one, but there were more
comments and I think you'll send a v2 for the actual -blockdev option
anyway once we have decided how to do it.
I have applied patches 1 to 6 now, and I think I could safely go on
until patch 9 if the minor improvements that were mentioned in comments
are made. I'd ignore patch 10 to 13 for now.
Is this what you would have expected or should I handle anything in a
different way?
Kevin
[Qemu-devel] Re: [PATCH 07/13] blockdev: Means to destroy blockdev only if made with drive_init()
Kevin Wolf kw...@redhat.com writes:
Am 02.06.2010 18:55, schrieb Markus Armbruster:
All drives are still made that way. They get destroyed along with
their device. That's inappropriate for the alternative way to make
blockdevs that will appear later in this series. These won't have a
DriveInfo.
blockdev_detach() destroys the blockdev only if it has a DriveInfo.
blockdev_attach() does nothing for now. It'll be fleshed out later.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
blockdev.c | 35 +++
blockdev.h |7 +++
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index ace74e4..f90d4fc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1,8 +1,12 @@
/*
* QEMU host block devices
*
+ * Copyright (C) 2010 Red Hat Inc.
* Copyright (c) 2003-2008 Fabrice Bellard
*
+ * Authors:
+ * Markus Armbruster arm...@redhat.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.
*/
@@ -17,6 +21,37 @@
static QTAILQ_HEAD(drivelist, DriveInfo) drives =
QTAILQ_HEAD_INITIALIZER(drives);
+static int blockdev_del_dinfo(BlockDriverState *bs)
+{
+DriveInfo *dinfo, *next_dinfo;
+int res = 0;
+
+QTAILQ_FOREACH_SAFE(dinfo, drives, next, next_dinfo) {
+if (dinfo-bdrv == bs) {
+qemu_opts_del(dinfo-opts);
+QTAILQ_REMOVE(drives, dinfo, next);
+qemu_free(dinfo);
+res = 1;
+}
+}
+
+return res;
Can it happen that a BlockDriverState belongs to multiple DriveInfos? If
no, why not returning in the loop? Wouldn't need a FOREACH_SAFE then, too.
No, that shouldn't happen. Defensive coding, I don't want to leave
dinfos with dangling dinfo-bdrv around. Maybe I should put an
assert(!res) before the qemu_opts_del(). Or just forget about it, and
simplify like you suggest.
It's not worth respinning because of this one, but there were more
comments and I think you'll send a v2 for the actual -blockdev option
anyway once we have decided how to do it.
I have applied patches 1 to 6 now, and I think I could safely go on
until patch 9 if the minor improvements that were mentioned in comments
are made. I'd ignore patch 10 to 13 for now.
Is this what you would have expected or should I handle anything in a
different way?
No, that suits me fine. I definitely need to respin from part 8 on
(commit message too terse).
