Re: [Qemu-devel] [PATCH v3 19/23] blockdev: Drop DriveInfo member enable_auto_del
On 16.09.2014 20:12, Markus Armbruster wrote:
Commit 2d246f0 introduced DriveInfo member enable_auto_del to
distinguish DriveInfo created via drive_new() from DriveInfo created
via qmp_blockdev_add(). The latter no longer exist. Drop
enable_auto_del.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
blockdev.c| 11 +++
include/sysemu/blockdev.h | 1 -
2 files changed, 3 insertions(+), 9 deletions(-)
I would've liked some comment somewhere about DriveInfo's presence
corresponding with the drive having been created through drive_new(),
but I can live without, too.
diff --git a/blockdev.c b/blockdev.c
index 0d99be0..e218c59 100644
--- a/blockdev.c
+++ b/blockdev.c
[snip]
@@ -1727,8 +1723,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
QObject **ret_data)
}
bs = blk_bs(blk);
-dinfo = blk_legacy_dinfo(blk);
-if (dinfo !dinfo-enable_auto_del) {
+if (!blk_legacy_dinfo(blk)) {
error_report(Deleting device added with blockdev-add
is not supported);
return -1;
This doesn't look like a 1-to-1 correspondence. Before this patch, if
DriveInfo was not present, the condition was false (actually, it was
always false, which is the reason for this patch). Now it's true. It
seems like the behavior is now correct but wasn't before... I guess this
means patch 18 should be fixed?
However, for this patch:
Reviewed-by: Max Reitz mre...@redhat.com
Re: [Qemu-devel] [PATCH v3 19/23] blockdev: Drop DriveInfo member enable_auto_del
Max Reitz mre...@redhat.com writes:
On 16.09.2014 20:12, Markus Armbruster wrote:
Commit 2d246f0 introduced DriveInfo member enable_auto_del to
distinguish DriveInfo created via drive_new() from DriveInfo created
via qmp_blockdev_add(). The latter no longer exist. Drop
enable_auto_del.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
blockdev.c| 11 +++
include/sysemu/blockdev.h | 1 -
2 files changed, 3 insertions(+), 9 deletions(-)
I would've liked some comment somewhere about DriveInfo's presence
corresponding with the drive having been created through drive_new(),
but I can live without, too.
Fair request. I could squash the following into PATCH 18:
diff --git a/block/block-backend.c b/block/block-backend.c
index 5646628..5e4ecd7 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -22,7 +22,7 @@ struct BlockBackend {
char *name;
int refcnt;
BlockDriverState *bs;
-DriveInfo *legacy_dinfo;
+DriveInfo *legacy_dinfo;/* null unless created by drive_new() */
QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */
void *dev; /* attached device model, if any */
diff --git a/blockdev.c b/blockdev.c
index 0d99be0..e218c59 100644
--- a/blockdev.c
+++ b/blockdev.c
[snip]
@@ -1727,8 +1723,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
QObject **ret_data)
}
bs = blk_bs(blk);
-dinfo = blk_legacy_dinfo(blk);
-if (dinfo !dinfo-enable_auto_del) {
+if (!blk_legacy_dinfo(blk)) {
error_report(Deleting device added with blockdev-add
is not supported);
return -1;
This doesn't look like a 1-to-1 correspondence. Before this patch, if
DriveInfo was not present, the condition was false (actually, it was
always false, which is the reason for this patch). Now it's true. It
seems like the behavior is now correct but wasn't before... I guess
this means patch 18 should be fixed?
PATCH 18 is broken. See also my reply to Fam on v2 of this patch.
However, for this patch:
Reviewed-by: Max Reitz mre...@redhat.com
Thanks!
Re: [Qemu-devel] [PATCH v3 19/23] blockdev: Drop DriveInfo member enable_auto_del
On 22.09.2014 17:06, Markus Armbruster wrote:
Max Reitz mre...@redhat.com writes:
On 16.09.2014 20:12, Markus Armbruster wrote:
Commit 2d246f0 introduced DriveInfo member enable_auto_del to
distinguish DriveInfo created via drive_new() from DriveInfo created
via qmp_blockdev_add(). The latter no longer exist. Drop
enable_auto_del.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
blockdev.c| 11 +++
include/sysemu/blockdev.h | 1 -
2 files changed, 3 insertions(+), 9 deletions(-)
I would've liked some comment somewhere about DriveInfo's presence
corresponding with the drive having been created through drive_new(),
but I can live without, too.
Fair request. I could squash the following into PATCH 18:
diff --git a/block/block-backend.c b/block/block-backend.c
index 5646628..5e4ecd7 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -22,7 +22,7 @@ struct BlockBackend {
char *name;
int refcnt;
BlockDriverState *bs;
-DriveInfo *legacy_dinfo;
+DriveInfo *legacy_dinfo;/* null unless created by drive_new() */
QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */
void *dev; /* attached device model, if any */
Yes, that would be nice, thank you!
diff --git a/blockdev.c b/blockdev.c
index 0d99be0..e218c59 100644
--- a/blockdev.c
+++ b/blockdev.c
[snip]
@@ -1727,8 +1723,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
QObject **ret_data)
}
bs = blk_bs(blk);
-dinfo = blk_legacy_dinfo(blk);
-if (dinfo !dinfo-enable_auto_del) {
+if (!blk_legacy_dinfo(blk)) {
error_report(Deleting device added with blockdev-add
is not supported);
return -1;
This doesn't look like a 1-to-1 correspondence. Before this patch, if
DriveInfo was not present, the condition was false (actually, it was
always false, which is the reason for this patch). Now it's true. It
seems like the behavior is now correct but wasn't before... I guess
this means patch 18 should be fixed?
PATCH 18 is broken. See also my reply to Fam on v2 of this patch.
Hm, okay. Fine with me. I don't object to squashing 19 into 18, either.
Max
However, for this patch:
Reviewed-by: Max Reitz mre...@redhat.com
Thanks!
Re: [Qemu-devel] [PATCH v3 19/23] blockdev: Drop DriveInfo member enable_auto_del
1.9.3 Reviewed-by: Benoit Canet benoit.ca...@nodalink.com
[Qemu-devel] [PATCH v3 19/23] blockdev: Drop DriveInfo member enable_auto_del
Commit 2d246f0 introduced DriveInfo member enable_auto_del to
distinguish DriveInfo created via drive_new() from DriveInfo created
via qmp_blockdev_add(). The latter no longer exist. Drop
enable_auto_del.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
blockdev.c| 11 +++
include/sysemu/blockdev.h | 1 -
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 0d99be0..e218c59 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -90,16 +90,14 @@ void blockdev_mark_auto_del(BlockBackend *blk)
DriveInfo *dinfo = blk_legacy_dinfo(blk);
BlockDriverState *bs = blk_bs(blk);
-if (dinfo !dinfo-enable_auto_del) {
+if (!dinfo) {
return;
}
if (bs-job) {
block_job_cancel(bs-job);
}
-if (dinfo) {
-dinfo-auto_del = 1;
-}
+dinfo-auto_del = 1;
}
void blockdev_auto_del(BlockBackend *blk)
@@ -900,7 +898,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
/* Set legacy DriveInfo fields */
dinfo = g_malloc0(sizeof(*dinfo));
-dinfo-enable_auto_del = true;
dinfo-opts = all_opts;
dinfo-cyls = cyls;
dinfo-heads = heads;
@@ -1716,7 +1713,6 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
QObject **ret_data)
const char *id = qdict_get_str(qdict, id);
BlockBackend *blk;
BlockDriverState *bs;
-DriveInfo *dinfo;
AioContext *aio_context;
Error *local_err = NULL;
@@ -1727,8 +1723,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
QObject **ret_data)
}
bs = blk_bs(blk);
-dinfo = blk_legacy_dinfo(blk);
-if (dinfo !dinfo-enable_auto_del) {
+if (!blk_legacy_dinfo(blk)) {
error_report(Deleting device added with blockdev-add
is not supported);
return -1;
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 27a40d5..2129d81 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -35,7 +35,6 @@ struct DriveInfo {
int bus;
int unit;
int auto_del; /* see blockdev_mark_auto_del() */
-bool enable_auto_del; /* Only for legacy drive_new() */
int media_cd;
int cyls, heads, secs, trans;
QemuOpts *opts;
--
1.9.3
