* [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
@ 2018-12-18 16:11 ` Thomas Huth
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2018-12-18 16:11 UTC (permalink / raw)
To: qemu-block, Kevin Wolf, Max Reitz, Stefano Stabellini, Anthony Perard
Cc: qemu-devel, xen-devel
The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
is derived from XenDevice which in turn is derived from DeviceState
since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
Thus the code can also simply use blk_attach_dev() with a pointer
to the DeviceState instead.
So we can finally remove all code related to the "legacy_dev" flag, too,
and turn the related "void *" in block-backend.c into "DeviceState *"
to fix some of the remaining TODOs there.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
Note: I haven't tested the Xen code since I don't have a working Xen
installation at hand. I'd appreciate if someone could check it...
block/block-backend.c | 54 +++++++-----------------------------------
hw/block/xen_disk.c | 6 +++--
include/sysemu/block-backend.h | 5 ++--
3 files changed, 15 insertions(+), 50 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 60d37a0..3c3118f 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -47,9 +47,7 @@ struct BlockBackend {
QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
BlockBackendPublic public;
- void *dev; /* attached device model, if any */
- bool legacy_dev; /* true if dev is not a DeviceState */
- /* TODO change to DeviceState when all users are qdevified */
+ DeviceState *dev; /* attached device model, if any */
const BlockDevOps *dev_ops;
void *dev_opaque;
@@ -836,7 +834,11 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm)
*shared_perm = blk->shared_perm;
}
-static int blk_do_attach_dev(BlockBackend *blk, void *dev)
+/*
+ * Attach device model @dev to @blk.
+ * Return 0 on success, -EBUSY when a device model is attached already.
+ */
+int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
{
if (blk->dev) {
return -EBUSY;
@@ -851,40 +853,16 @@ static int blk_do_attach_dev(BlockBackend *blk, void *dev)
blk_ref(blk);
blk->dev = dev;
- blk->legacy_dev = false;
blk_iostatus_reset(blk);
return 0;
}
/*
- * Attach device model @dev to @blk.
- * Return 0 on success, -EBUSY when a device model is attached already.
- */
-int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
-{
- return blk_do_attach_dev(blk, dev);
-}
-
-/*
- * Attach device model @dev to @blk.
- * @blk must not have a device model attached already.
- * TODO qdevified devices don't use this, remove when devices are qdevified
- */
-void blk_attach_dev_legacy(BlockBackend *blk, void *dev)
-{
- if (blk_do_attach_dev(blk, dev) < 0) {
- abort();
- }
- blk->legacy_dev = true;
-}
-
-/*
* Detach device model @dev from @blk.
* @dev must be currently attached to @blk.
*/
-void blk_detach_dev(BlockBackend *blk, void *dev)
-/* TODO change to DeviceState *dev when all users are qdevified */
+void blk_detach_dev(BlockBackend *blk, DeviceState *dev)
{
assert(blk->dev == dev);
blk->dev = NULL;
@@ -898,8 +876,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
/*
* Return the device model attached to @blk if any, else null.
*/
-void *blk_get_attached_dev(BlockBackend *blk)
-/* TODO change to return DeviceState * when all users are qdevified */
+DeviceState *blk_get_attached_dev(BlockBackend *blk)
{
return blk->dev;
}
@@ -908,10 +885,7 @@ void *blk_get_attached_dev(BlockBackend *blk)
* device attached to the BlockBackend. */
char *blk_get_attached_dev_id(BlockBackend *blk)
{
- DeviceState *dev;
-
- assert(!blk->legacy_dev);
- dev = blk->dev;
+ DeviceState *dev = blk->dev;
if (!dev) {
return g_strdup("");
@@ -949,11 +923,6 @@ BlockBackend *blk_by_dev(void *dev)
void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops,
void *opaque)
{
- /* All drivers that use blk_set_dev_ops() are qdevified and we want to keep
- * it that way, so we can assume blk->dev, if present, is a DeviceState if
- * blk->dev_ops is set. Non-device users may use dev_ops without device. */
- assert(!blk->legacy_dev);
-
blk->dev_ops = ops;
blk->dev_opaque = opaque;
@@ -979,8 +948,6 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp)
bool tray_was_open, tray_is_open;
Error *local_err = NULL;
- assert(!blk->legacy_dev);
-
tray_was_open = blk_dev_is_tray_open(blk);
blk->dev_ops->change_media_cb(blk->dev_opaque, load, &local_err);
if (local_err) {
@@ -1779,9 +1746,6 @@ void blk_eject(BlockBackend *blk, bool eject_flag)
BlockDriverState *bs = blk_bs(blk);
char *id;
- /* blk_eject is only called by qdevified devices */
- assert(!blk->legacy_dev);
-
if (bs) {
bdrv_eject(bs, eject_flag);
}
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 36eff94..9605caf 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
* so we can blk_unref() unconditionally */
blk_ref(blkdev->blk);
}
- blk_attach_dev_legacy(blkdev->blk, blkdev);
+ if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
+ return -1;
+ }
blkdev->file_size = blk_getlength(blkdev->blk);
if (blkdev->file_size < 0) {
BlockDriverState *bs = blk_bs(blkdev->blk);
@@ -951,7 +953,7 @@ static void blk_disconnect(struct XenDevice *xendev)
if (blkdev->blk) {
blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
- blk_detach_dev(blkdev->blk, blkdev);
+ blk_detach_dev(blkdev->blk, DEVICE(blkdev));
blk_unref(blkdev->blk);
blkdev->blk = NULL;
}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index c96bcde..39507d6 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -110,9 +110,8 @@ void blk_iostatus_disable(BlockBackend *blk);
void blk_iostatus_reset(BlockBackend *blk);
void blk_iostatus_set_err(BlockBackend *blk, int error);
int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
-void blk_attach_dev_legacy(BlockBackend *blk, void *dev);
-void blk_detach_dev(BlockBackend *blk, void *dev);
-void *blk_get_attached_dev(BlockBackend *blk);
+void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
+DeviceState *blk_get_attached_dev(BlockBackend *blk);
char *blk_get_attached_dev_id(BlockBackend *blk);
BlockBackend *blk_by_dev(void *dev);
BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
@ 2018-12-18 16:11 ` Thomas Huth
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2018-12-18 16:11 UTC (permalink / raw)
To: qemu-block, Kevin Wolf, Max Reitz, Stefano Stabellini, Anthony Perard
Cc: xen-devel, qemu-devel
The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
is derived from XenDevice which in turn is derived from DeviceState
since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
Thus the code can also simply use blk_attach_dev() with a pointer
to the DeviceState instead.
So we can finally remove all code related to the "legacy_dev" flag, too,
and turn the related "void *" in block-backend.c into "DeviceState *"
to fix some of the remaining TODOs there.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
Note: I haven't tested the Xen code since I don't have a working Xen
installation at hand. I'd appreciate if someone could check it...
block/block-backend.c | 54 +++++++-----------------------------------
hw/block/xen_disk.c | 6 +++--
include/sysemu/block-backend.h | 5 ++--
3 files changed, 15 insertions(+), 50 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 60d37a0..3c3118f 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -47,9 +47,7 @@ struct BlockBackend {
QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
BlockBackendPublic public;
- void *dev; /* attached device model, if any */
- bool legacy_dev; /* true if dev is not a DeviceState */
- /* TODO change to DeviceState when all users are qdevified */
+ DeviceState *dev; /* attached device model, if any */
const BlockDevOps *dev_ops;
void *dev_opaque;
@@ -836,7 +834,11 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm)
*shared_perm = blk->shared_perm;
}
-static int blk_do_attach_dev(BlockBackend *blk, void *dev)
+/*
+ * Attach device model @dev to @blk.
+ * Return 0 on success, -EBUSY when a device model is attached already.
+ */
+int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
{
if (blk->dev) {
return -EBUSY;
@@ -851,40 +853,16 @@ static int blk_do_attach_dev(BlockBackend *blk, void *dev)
blk_ref(blk);
blk->dev = dev;
- blk->legacy_dev = false;
blk_iostatus_reset(blk);
return 0;
}
/*
- * Attach device model @dev to @blk.
- * Return 0 on success, -EBUSY when a device model is attached already.
- */
-int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
-{
- return blk_do_attach_dev(blk, dev);
-}
-
-/*
- * Attach device model @dev to @blk.
- * @blk must not have a device model attached already.
- * TODO qdevified devices don't use this, remove when devices are qdevified
- */
-void blk_attach_dev_legacy(BlockBackend *blk, void *dev)
-{
- if (blk_do_attach_dev(blk, dev) < 0) {
- abort();
- }
- blk->legacy_dev = true;
-}
-
-/*
* Detach device model @dev from @blk.
* @dev must be currently attached to @blk.
*/
-void blk_detach_dev(BlockBackend *blk, void *dev)
-/* TODO change to DeviceState *dev when all users are qdevified */
+void blk_detach_dev(BlockBackend *blk, DeviceState *dev)
{
assert(blk->dev == dev);
blk->dev = NULL;
@@ -898,8 +876,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
/*
* Return the device model attached to @blk if any, else null.
*/
-void *blk_get_attached_dev(BlockBackend *blk)
-/* TODO change to return DeviceState * when all users are qdevified */
+DeviceState *blk_get_attached_dev(BlockBackend *blk)
{
return blk->dev;
}
@@ -908,10 +885,7 @@ void *blk_get_attached_dev(BlockBackend *blk)
* device attached to the BlockBackend. */
char *blk_get_attached_dev_id(BlockBackend *blk)
{
- DeviceState *dev;
-
- assert(!blk->legacy_dev);
- dev = blk->dev;
+ DeviceState *dev = blk->dev;
if (!dev) {
return g_strdup("");
@@ -949,11 +923,6 @@ BlockBackend *blk_by_dev(void *dev)
void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops,
void *opaque)
{
- /* All drivers that use blk_set_dev_ops() are qdevified and we want to keep
- * it that way, so we can assume blk->dev, if present, is a DeviceState if
- * blk->dev_ops is set. Non-device users may use dev_ops without device. */
- assert(!blk->legacy_dev);
-
blk->dev_ops = ops;
blk->dev_opaque = opaque;
@@ -979,8 +948,6 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp)
bool tray_was_open, tray_is_open;
Error *local_err = NULL;
- assert(!blk->legacy_dev);
-
tray_was_open = blk_dev_is_tray_open(blk);
blk->dev_ops->change_media_cb(blk->dev_opaque, load, &local_err);
if (local_err) {
@@ -1779,9 +1746,6 @@ void blk_eject(BlockBackend *blk, bool eject_flag)
BlockDriverState *bs = blk_bs(blk);
char *id;
- /* blk_eject is only called by qdevified devices */
- assert(!blk->legacy_dev);
-
if (bs) {
bdrv_eject(bs, eject_flag);
}
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 36eff94..9605caf 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
* so we can blk_unref() unconditionally */
blk_ref(blkdev->blk);
}
- blk_attach_dev_legacy(blkdev->blk, blkdev);
+ if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
+ return -1;
+ }
blkdev->file_size = blk_getlength(blkdev->blk);
if (blkdev->file_size < 0) {
BlockDriverState *bs = blk_bs(blkdev->blk);
@@ -951,7 +953,7 @@ static void blk_disconnect(struct XenDevice *xendev)
if (blkdev->blk) {
blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
- blk_detach_dev(blkdev->blk, blkdev);
+ blk_detach_dev(blkdev->blk, DEVICE(blkdev));
blk_unref(blkdev->blk);
blkdev->blk = NULL;
}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index c96bcde..39507d6 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -110,9 +110,8 @@ void blk_iostatus_disable(BlockBackend *blk);
void blk_iostatus_reset(BlockBackend *blk);
void blk_iostatus_set_err(BlockBackend *blk, int error);
int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
-void blk_attach_dev_legacy(BlockBackend *blk, void *dev);
-void blk_detach_dev(BlockBackend *blk, void *dev);
-void *blk_get_attached_dev(BlockBackend *blk);
+void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
+DeviceState *blk_get_attached_dev(BlockBackend *blk);
char *blk_get_attached_dev_id(BlockBackend *blk);
BlockBackend *blk_by_dev(void *dev);
BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
2018-12-18 16:11 ` Thomas Huth
@ 2018-12-18 18:33 ` Markus Armbruster
-1 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2018-12-18 18:33 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-block, Kevin Wolf, Max Reitz, Stefano Stabellini,
Anthony Perard, xen-devel, qemu-devel
Thomas Huth <thuth@redhat.com> writes:
> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
> is derived from XenDevice which in turn is derived from DeviceState
> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
> Thus the code can also simply use blk_attach_dev() with a pointer
> to the DeviceState instead.
> So we can finally remove all code related to the "legacy_dev" flag, too,
> and turn the related "void *" in block-backend.c into "DeviceState *"
> to fix some of the remaining TODOs there.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> Note: I haven't tested the Xen code since I don't have a working Xen
> installation at hand. I'd appreciate if someone could check it...
Same here. All I can do is review.
> block/block-backend.c | 54 +++++++-----------------------------------
> hw/block/xen_disk.c | 6 +++--
> include/sysemu/block-backend.h | 5 ++--
> 3 files changed, 15 insertions(+), 50 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 60d37a0..3c3118f 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -47,9 +47,7 @@ struct BlockBackend {
> QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
> BlockBackendPublic public;
>
> - void *dev; /* attached device model, if any */
> - bool legacy_dev; /* true if dev is not a DeviceState */
> - /* TODO change to DeviceState when all users are qdevified */
> + DeviceState *dev; /* attached device model, if any */
> const BlockDevOps *dev_ops;
> void *dev_opaque;
>
> @@ -836,7 +834,11 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm)
> *shared_perm = blk->shared_perm;
> }
>
> -static int blk_do_attach_dev(BlockBackend *blk, void *dev)
> +/*
> + * Attach device model @dev to @blk.
> + * Return 0 on success, -EBUSY when a device model is attached already.
> + */
> +int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> {
> if (blk->dev) {
> return -EBUSY;
> @@ -851,40 +853,16 @@ static int blk_do_attach_dev(BlockBackend *blk, void *dev)
>
> blk_ref(blk);
> blk->dev = dev;
> - blk->legacy_dev = false;
> blk_iostatus_reset(blk);
>
> return 0;
> }
>
> /*
> - * Attach device model @dev to @blk.
> - * Return 0 on success, -EBUSY when a device model is attached already.
> - */
> -int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> -{
> - return blk_do_attach_dev(blk, dev);
> -}
> -
> -/*
> - * Attach device model @dev to @blk.
> - * @blk must not have a device model attached already.
> - * TODO qdevified devices don't use this, remove when devices are qdevified
> - */
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev)
> -{
> - if (blk_do_attach_dev(blk, dev) < 0) {
> - abort();
> - }
> - blk->legacy_dev = true;
> -}
> -
> -/*
> * Detach device model @dev from @blk.
> * @dev must be currently attached to @blk.
> */
> -void blk_detach_dev(BlockBackend *blk, void *dev)
> -/* TODO change to DeviceState *dev when all users are qdevified */
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev)
> {
> assert(blk->dev == dev);
> blk->dev = NULL;
> @@ -898,8 +876,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
> /*
> * Return the device model attached to @blk if any, else null.
> */
> -void *blk_get_attached_dev(BlockBackend *blk)
> -/* TODO change to return DeviceState * when all users are qdevified */
> +DeviceState *blk_get_attached_dev(BlockBackend *blk)
> {
> return blk->dev;
> }
> @@ -908,10 +885,7 @@ void *blk_get_attached_dev(BlockBackend *blk)
> * device attached to the BlockBackend. */
> char *blk_get_attached_dev_id(BlockBackend *blk)
> {
> - DeviceState *dev;
> -
> - assert(!blk->legacy_dev);
> - dev = blk->dev;
> + DeviceState *dev = blk->dev;
>
> if (!dev) {
> return g_strdup("");
> @@ -949,11 +923,6 @@ BlockBackend *blk_by_dev(void *dev)
> void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops,
> void *opaque)
> {
> - /* All drivers that use blk_set_dev_ops() are qdevified and we want to keep
> - * it that way, so we can assume blk->dev, if present, is a DeviceState if
> - * blk->dev_ops is set. Non-device users may use dev_ops without device. */
> - assert(!blk->legacy_dev);
> -
> blk->dev_ops = ops;
> blk->dev_opaque = opaque;
>
> @@ -979,8 +948,6 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp)
> bool tray_was_open, tray_is_open;
> Error *local_err = NULL;
>
> - assert(!blk->legacy_dev);
> -
> tray_was_open = blk_dev_is_tray_open(blk);
> blk->dev_ops->change_media_cb(blk->dev_opaque, load, &local_err);
> if (local_err) {
> @@ -1779,9 +1746,6 @@ void blk_eject(BlockBackend *blk, bool eject_flag)
> BlockDriverState *bs = blk_bs(blk);
> char *id;
>
> - /* blk_eject is only called by qdevified devices */
> - assert(!blk->legacy_dev);
> -
> if (bs) {
> bdrv_eject(bs, eject_flag);
> }
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 36eff94..9605caf 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
> * so we can blk_unref() unconditionally */
> blk_ref(blkdev->blk);
> }
> - blk_attach_dev_legacy(blkdev->blk, blkdev);
> + if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
> + return -1;
> + }
Other error returns in this function call xen_pv_printf() first. Should
this one, too?
> blkdev->file_size = blk_getlength(blkdev->blk);
> if (blkdev->file_size < 0) {
> BlockDriverState *bs = blk_bs(blkdev->blk);
> @@ -951,7 +953,7 @@ static void blk_disconnect(struct XenDevice *xendev)
>
> if (blkdev->blk) {
> blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
> - blk_detach_dev(blkdev->blk, blkdev);
> + blk_detach_dev(blkdev->blk, DEVICE(blkdev));
> blk_unref(blkdev->blk);
> blkdev->blk = NULL;
> }
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index c96bcde..39507d6 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -110,9 +110,8 @@ void blk_iostatus_disable(BlockBackend *blk);
> void blk_iostatus_reset(BlockBackend *blk);
> void blk_iostatus_set_err(BlockBackend *blk, int error);
> int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev);
> -void blk_detach_dev(BlockBackend *blk, void *dev);
> -void *blk_get_attached_dev(BlockBackend *blk);
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
> +DeviceState *blk_get_attached_dev(BlockBackend *blk);
> char *blk_get_attached_dev_id(BlockBackend *blk);
> BlockBackend *blk_by_dev(void *dev);
> BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
I didn't verify your claim that DEVICE(blkdev) is okay. Apart from
that, looks good to me.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
@ 2018-12-18 18:33 ` Markus Armbruster
0 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2018-12-18 18:33 UTC (permalink / raw)
To: Thomas Huth
Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
Max Reitz, Anthony Perard, xen-devel
Thomas Huth <thuth@redhat.com> writes:
> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
> is derived from XenDevice which in turn is derived from DeviceState
> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
> Thus the code can also simply use blk_attach_dev() with a pointer
> to the DeviceState instead.
> So we can finally remove all code related to the "legacy_dev" flag, too,
> and turn the related "void *" in block-backend.c into "DeviceState *"
> to fix some of the remaining TODOs there.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> Note: I haven't tested the Xen code since I don't have a working Xen
> installation at hand. I'd appreciate if someone could check it...
Same here. All I can do is review.
> block/block-backend.c | 54 +++++++-----------------------------------
> hw/block/xen_disk.c | 6 +++--
> include/sysemu/block-backend.h | 5 ++--
> 3 files changed, 15 insertions(+), 50 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 60d37a0..3c3118f 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -47,9 +47,7 @@ struct BlockBackend {
> QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
> BlockBackendPublic public;
>
> - void *dev; /* attached device model, if any */
> - bool legacy_dev; /* true if dev is not a DeviceState */
> - /* TODO change to DeviceState when all users are qdevified */
> + DeviceState *dev; /* attached device model, if any */
> const BlockDevOps *dev_ops;
> void *dev_opaque;
>
> @@ -836,7 +834,11 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm)
> *shared_perm = blk->shared_perm;
> }
>
> -static int blk_do_attach_dev(BlockBackend *blk, void *dev)
> +/*
> + * Attach device model @dev to @blk.
> + * Return 0 on success, -EBUSY when a device model is attached already.
> + */
> +int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> {
> if (blk->dev) {
> return -EBUSY;
> @@ -851,40 +853,16 @@ static int blk_do_attach_dev(BlockBackend *blk, void *dev)
>
> blk_ref(blk);
> blk->dev = dev;
> - blk->legacy_dev = false;
> blk_iostatus_reset(blk);
>
> return 0;
> }
>
> /*
> - * Attach device model @dev to @blk.
> - * Return 0 on success, -EBUSY when a device model is attached already.
> - */
> -int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> -{
> - return blk_do_attach_dev(blk, dev);
> -}
> -
> -/*
> - * Attach device model @dev to @blk.
> - * @blk must not have a device model attached already.
> - * TODO qdevified devices don't use this, remove when devices are qdevified
> - */
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev)
> -{
> - if (blk_do_attach_dev(blk, dev) < 0) {
> - abort();
> - }
> - blk->legacy_dev = true;
> -}
> -
> -/*
> * Detach device model @dev from @blk.
> * @dev must be currently attached to @blk.
> */
> -void blk_detach_dev(BlockBackend *blk, void *dev)
> -/* TODO change to DeviceState *dev when all users are qdevified */
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev)
> {
> assert(blk->dev == dev);
> blk->dev = NULL;
> @@ -898,8 +876,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
> /*
> * Return the device model attached to @blk if any, else null.
> */
> -void *blk_get_attached_dev(BlockBackend *blk)
> -/* TODO change to return DeviceState * when all users are qdevified */
> +DeviceState *blk_get_attached_dev(BlockBackend *blk)
> {
> return blk->dev;
> }
> @@ -908,10 +885,7 @@ void *blk_get_attached_dev(BlockBackend *blk)
> * device attached to the BlockBackend. */
> char *blk_get_attached_dev_id(BlockBackend *blk)
> {
> - DeviceState *dev;
> -
> - assert(!blk->legacy_dev);
> - dev = blk->dev;
> + DeviceState *dev = blk->dev;
>
> if (!dev) {
> return g_strdup("");
> @@ -949,11 +923,6 @@ BlockBackend *blk_by_dev(void *dev)
> void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops,
> void *opaque)
> {
> - /* All drivers that use blk_set_dev_ops() are qdevified and we want to keep
> - * it that way, so we can assume blk->dev, if present, is a DeviceState if
> - * blk->dev_ops is set. Non-device users may use dev_ops without device. */
> - assert(!blk->legacy_dev);
> -
> blk->dev_ops = ops;
> blk->dev_opaque = opaque;
>
> @@ -979,8 +948,6 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp)
> bool tray_was_open, tray_is_open;
> Error *local_err = NULL;
>
> - assert(!blk->legacy_dev);
> -
> tray_was_open = blk_dev_is_tray_open(blk);
> blk->dev_ops->change_media_cb(blk->dev_opaque, load, &local_err);
> if (local_err) {
> @@ -1779,9 +1746,6 @@ void blk_eject(BlockBackend *blk, bool eject_flag)
> BlockDriverState *bs = blk_bs(blk);
> char *id;
>
> - /* blk_eject is only called by qdevified devices */
> - assert(!blk->legacy_dev);
> -
> if (bs) {
> bdrv_eject(bs, eject_flag);
> }
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 36eff94..9605caf 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
> * so we can blk_unref() unconditionally */
> blk_ref(blkdev->blk);
> }
> - blk_attach_dev_legacy(blkdev->blk, blkdev);
> + if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
> + return -1;
> + }
Other error returns in this function call xen_pv_printf() first. Should
this one, too?
> blkdev->file_size = blk_getlength(blkdev->blk);
> if (blkdev->file_size < 0) {
> BlockDriverState *bs = blk_bs(blkdev->blk);
> @@ -951,7 +953,7 @@ static void blk_disconnect(struct XenDevice *xendev)
>
> if (blkdev->blk) {
> blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
> - blk_detach_dev(blkdev->blk, blkdev);
> + blk_detach_dev(blkdev->blk, DEVICE(blkdev));
> blk_unref(blkdev->blk);
> blkdev->blk = NULL;
> }
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index c96bcde..39507d6 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -110,9 +110,8 @@ void blk_iostatus_disable(BlockBackend *blk);
> void blk_iostatus_reset(BlockBackend *blk);
> void blk_iostatus_set_err(BlockBackend *blk, int error);
> int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev);
> -void blk_detach_dev(BlockBackend *blk, void *dev);
> -void *blk_get_attached_dev(BlockBackend *blk);
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
> +DeviceState *blk_get_attached_dev(BlockBackend *blk);
> char *blk_get_attached_dev_id(BlockBackend *blk);
> BlockBackend *blk_by_dev(void *dev);
> BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
I didn't verify your claim that DEVICE(blkdev) is okay. Apart from
that, looks good to me.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
2018-12-18 18:33 ` Markus Armbruster
(?)
(?)
@ 2018-12-19 12:00 ` Thomas Huth
-1 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2018-12-19 12:00 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-block, Kevin Wolf, Max Reitz, Stefano Stabellini,
Anthony Perard, xen-devel, qemu-devel
On 2018-12-18 19:33, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
>> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
>> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
>> is derived from XenDevice which in turn is derived from DeviceState
>> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
>> Thus the code can also simply use blk_attach_dev() with a pointer
>> to the DeviceState instead.
>> So we can finally remove all code related to the "legacy_dev" flag, too,
>> and turn the related "void *" in block-backend.c into "DeviceState *"
>> to fix some of the remaining TODOs there.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> Note: I haven't tested the Xen code since I don't have a working Xen
>> installation at hand. I'd appreciate if someone could check it...
[...]
>> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
>> index 36eff94..9605caf 100644
>> --- a/hw/block/xen_disk.c
>> +++ b/hw/block/xen_disk.c
>> @@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
>> * so we can blk_unref() unconditionally */
>> blk_ref(blkdev->blk);
>> }
>> - blk_attach_dev_legacy(blkdev->blk, blkdev);
>> + if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
>> + return -1;
>> + }
>
> Other error returns in this function call xen_pv_printf() first. Should
> this one, too?
Only some of them do a xen_pv_printf() first, there are also many that
don't. blk_attach_dev() currently only returns an error if a device has
already been attach - which should simply never happen here, so a printf
currently does not seem to be justified to me. Alternatively, we could
also abort() here instead, I think. I'll leave that decision up to the
Xen maintainers...
Thomas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
2018-12-18 18:33 ` Markus Armbruster
(?)
@ 2018-12-19 12:00 ` Thomas Huth
-1 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2018-12-19 12:00 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
Max Reitz, Anthony Perard, xen-devel
On 2018-12-18 19:33, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
>> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
>> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
>> is derived from XenDevice which in turn is derived from DeviceState
>> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
>> Thus the code can also simply use blk_attach_dev() with a pointer
>> to the DeviceState instead.
>> So we can finally remove all code related to the "legacy_dev" flag, too,
>> and turn the related "void *" in block-backend.c into "DeviceState *"
>> to fix some of the remaining TODOs there.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> Note: I haven't tested the Xen code since I don't have a working Xen
>> installation at hand. I'd appreciate if someone could check it...
[...]
>> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
>> index 36eff94..9605caf 100644
>> --- a/hw/block/xen_disk.c
>> +++ b/hw/block/xen_disk.c
>> @@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
>> * so we can blk_unref() unconditionally */
>> blk_ref(blkdev->blk);
>> }
>> - blk_attach_dev_legacy(blkdev->blk, blkdev);
>> + if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
>> + return -1;
>> + }
>
> Other error returns in this function call xen_pv_printf() first. Should
> this one, too?
Only some of them do a xen_pv_printf() first, there are also many that
don't. blk_attach_dev() currently only returns an error if a device has
already been attach - which should simply never happen here, so a printf
currently does not seem to be justified to me. Alternatively, we could
also abort() here instead, I think. I'll leave that decision up to the
Xen maintainers...
Thomas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
2018-12-18 16:11 ` Thomas Huth
@ 2019-01-22 14:19 ` Thomas Huth
-1 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2019-01-22 14:19 UTC (permalink / raw)
To: qemu-block, Kevin Wolf, Max Reitz, Stefano Stabellini, Anthony Perard
Cc: xen-devel, qemu-devel, paul.durrant
On 2018-12-18 17:11, Thomas Huth wrote:
> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
> is derived from XenDevice which in turn is derived from DeviceState
> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
> Thus the code can also simply use blk_attach_dev() with a pointer
> to the DeviceState instead.
> So we can finally remove all code related to the "legacy_dev" flag, too,
> and turn the related "void *" in block-backend.c into "DeviceState *"
> to fix some of the remaining TODOs there.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> Note: I haven't tested the Xen code since I don't have a working Xen
> installation at hand. I'd appreciate if someone could check it...
Ping?
> block/block-backend.c | 54 +++++++-----------------------------------
> hw/block/xen_disk.c | 6 +++--
> include/sysemu/block-backend.h | 5 ++--
> 3 files changed, 15 insertions(+), 50 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 60d37a0..3c3118f 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -47,9 +47,7 @@ struct BlockBackend {
> QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
> BlockBackendPublic public;
>
> - void *dev; /* attached device model, if any */
> - bool legacy_dev; /* true if dev is not a DeviceState */
> - /* TODO change to DeviceState when all users are qdevified */
> + DeviceState *dev; /* attached device model, if any */
> const BlockDevOps *dev_ops;
> void *dev_opaque;
>
> @@ -836,7 +834,11 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm)
> *shared_perm = blk->shared_perm;
> }
>
> -static int blk_do_attach_dev(BlockBackend *blk, void *dev)
> +/*
> + * Attach device model @dev to @blk.
> + * Return 0 on success, -EBUSY when a device model is attached already.
> + */
> +int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> {
> if (blk->dev) {
> return -EBUSY;
> @@ -851,40 +853,16 @@ static int blk_do_attach_dev(BlockBackend *blk, void *dev)
>
> blk_ref(blk);
> blk->dev = dev;
> - blk->legacy_dev = false;
> blk_iostatus_reset(blk);
>
> return 0;
> }
>
> /*
> - * Attach device model @dev to @blk.
> - * Return 0 on success, -EBUSY when a device model is attached already.
> - */
> -int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> -{
> - return blk_do_attach_dev(blk, dev);
> -}
> -
> -/*
> - * Attach device model @dev to @blk.
> - * @blk must not have a device model attached already.
> - * TODO qdevified devices don't use this, remove when devices are qdevified
> - */
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev)
> -{
> - if (blk_do_attach_dev(blk, dev) < 0) {
> - abort();
> - }
> - blk->legacy_dev = true;
> -}
> -
> -/*
> * Detach device model @dev from @blk.
> * @dev must be currently attached to @blk.
> */
> -void blk_detach_dev(BlockBackend *blk, void *dev)
> -/* TODO change to DeviceState *dev when all users are qdevified */
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev)
> {
> assert(blk->dev == dev);
> blk->dev = NULL;
> @@ -898,8 +876,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
> /*
> * Return the device model attached to @blk if any, else null.
> */
> -void *blk_get_attached_dev(BlockBackend *blk)
> -/* TODO change to return DeviceState * when all users are qdevified */
> +DeviceState *blk_get_attached_dev(BlockBackend *blk)
> {
> return blk->dev;
> }
> @@ -908,10 +885,7 @@ void *blk_get_attached_dev(BlockBackend *blk)
> * device attached to the BlockBackend. */
> char *blk_get_attached_dev_id(BlockBackend *blk)
> {
> - DeviceState *dev;
> -
> - assert(!blk->legacy_dev);
> - dev = blk->dev;
> + DeviceState *dev = blk->dev;
>
> if (!dev) {
> return g_strdup("");
> @@ -949,11 +923,6 @@ BlockBackend *blk_by_dev(void *dev)
> void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops,
> void *opaque)
> {
> - /* All drivers that use blk_set_dev_ops() are qdevified and we want to keep
> - * it that way, so we can assume blk->dev, if present, is a DeviceState if
> - * blk->dev_ops is set. Non-device users may use dev_ops without device. */
> - assert(!blk->legacy_dev);
> -
> blk->dev_ops = ops;
> blk->dev_opaque = opaque;
>
> @@ -979,8 +948,6 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp)
> bool tray_was_open, tray_is_open;
> Error *local_err = NULL;
>
> - assert(!blk->legacy_dev);
> -
> tray_was_open = blk_dev_is_tray_open(blk);
> blk->dev_ops->change_media_cb(blk->dev_opaque, load, &local_err);
> if (local_err) {
> @@ -1779,9 +1746,6 @@ void blk_eject(BlockBackend *blk, bool eject_flag)
> BlockDriverState *bs = blk_bs(blk);
> char *id;
>
> - /* blk_eject is only called by qdevified devices */
> - assert(!blk->legacy_dev);
> -
> if (bs) {
> bdrv_eject(bs, eject_flag);
> }
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 36eff94..9605caf 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
> * so we can blk_unref() unconditionally */
> blk_ref(blkdev->blk);
> }
> - blk_attach_dev_legacy(blkdev->blk, blkdev);
> + if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
> + return -1;
> + }
> blkdev->file_size = blk_getlength(blkdev->blk);
> if (blkdev->file_size < 0) {
> BlockDriverState *bs = blk_bs(blkdev->blk);
> @@ -951,7 +953,7 @@ static void blk_disconnect(struct XenDevice *xendev)
>
> if (blkdev->blk) {
> blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
> - blk_detach_dev(blkdev->blk, blkdev);
> + blk_detach_dev(blkdev->blk, DEVICE(blkdev));
> blk_unref(blkdev->blk);
> blkdev->blk = NULL;
> }
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index c96bcde..39507d6 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -110,9 +110,8 @@ void blk_iostatus_disable(BlockBackend *blk);
> void blk_iostatus_reset(BlockBackend *blk);
> void blk_iostatus_set_err(BlockBackend *blk, int error);
> int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev);
> -void blk_detach_dev(BlockBackend *blk, void *dev);
> -void *blk_get_attached_dev(BlockBackend *blk);
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
> +DeviceState *blk_get_attached_dev(BlockBackend *blk);
> char *blk_get_attached_dev_id(BlockBackend *blk);
> BlockBackend *blk_by_dev(void *dev);
> BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
@ 2019-01-22 14:19 ` Thomas Huth
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2019-01-22 14:19 UTC (permalink / raw)
To: qemu-block, Kevin Wolf, Max Reitz, Stefano Stabellini, Anthony Perard
Cc: xen-devel, paul.durrant, qemu-devel
On 2018-12-18 17:11, Thomas Huth wrote:
> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
> is derived from XenDevice which in turn is derived from DeviceState
> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
> Thus the code can also simply use blk_attach_dev() with a pointer
> to the DeviceState instead.
> So we can finally remove all code related to the "legacy_dev" flag, too,
> and turn the related "void *" in block-backend.c into "DeviceState *"
> to fix some of the remaining TODOs there.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> Note: I haven't tested the Xen code since I don't have a working Xen
> installation at hand. I'd appreciate if someone could check it...
Ping?
> block/block-backend.c | 54 +++++++-----------------------------------
> hw/block/xen_disk.c | 6 +++--
> include/sysemu/block-backend.h | 5 ++--
> 3 files changed, 15 insertions(+), 50 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 60d37a0..3c3118f 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -47,9 +47,7 @@ struct BlockBackend {
> QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
> BlockBackendPublic public;
>
> - void *dev; /* attached device model, if any */
> - bool legacy_dev; /* true if dev is not a DeviceState */
> - /* TODO change to DeviceState when all users are qdevified */
> + DeviceState *dev; /* attached device model, if any */
> const BlockDevOps *dev_ops;
> void *dev_opaque;
>
> @@ -836,7 +834,11 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm)
> *shared_perm = blk->shared_perm;
> }
>
> -static int blk_do_attach_dev(BlockBackend *blk, void *dev)
> +/*
> + * Attach device model @dev to @blk.
> + * Return 0 on success, -EBUSY when a device model is attached already.
> + */
> +int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> {
> if (blk->dev) {
> return -EBUSY;
> @@ -851,40 +853,16 @@ static int blk_do_attach_dev(BlockBackend *blk, void *dev)
>
> blk_ref(blk);
> blk->dev = dev;
> - blk->legacy_dev = false;
> blk_iostatus_reset(blk);
>
> return 0;
> }
>
> /*
> - * Attach device model @dev to @blk.
> - * Return 0 on success, -EBUSY when a device model is attached already.
> - */
> -int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> -{
> - return blk_do_attach_dev(blk, dev);
> -}
> -
> -/*
> - * Attach device model @dev to @blk.
> - * @blk must not have a device model attached already.
> - * TODO qdevified devices don't use this, remove when devices are qdevified
> - */
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev)
> -{
> - if (blk_do_attach_dev(blk, dev) < 0) {
> - abort();
> - }
> - blk->legacy_dev = true;
> -}
> -
> -/*
> * Detach device model @dev from @blk.
> * @dev must be currently attached to @blk.
> */
> -void blk_detach_dev(BlockBackend *blk, void *dev)
> -/* TODO change to DeviceState *dev when all users are qdevified */
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev)
> {
> assert(blk->dev == dev);
> blk->dev = NULL;
> @@ -898,8 +876,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
> /*
> * Return the device model attached to @blk if any, else null.
> */
> -void *blk_get_attached_dev(BlockBackend *blk)
> -/* TODO change to return DeviceState * when all users are qdevified */
> +DeviceState *blk_get_attached_dev(BlockBackend *blk)
> {
> return blk->dev;
> }
> @@ -908,10 +885,7 @@ void *blk_get_attached_dev(BlockBackend *blk)
> * device attached to the BlockBackend. */
> char *blk_get_attached_dev_id(BlockBackend *blk)
> {
> - DeviceState *dev;
> -
> - assert(!blk->legacy_dev);
> - dev = blk->dev;
> + DeviceState *dev = blk->dev;
>
> if (!dev) {
> return g_strdup("");
> @@ -949,11 +923,6 @@ BlockBackend *blk_by_dev(void *dev)
> void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops,
> void *opaque)
> {
> - /* All drivers that use blk_set_dev_ops() are qdevified and we want to keep
> - * it that way, so we can assume blk->dev, if present, is a DeviceState if
> - * blk->dev_ops is set. Non-device users may use dev_ops without device. */
> - assert(!blk->legacy_dev);
> -
> blk->dev_ops = ops;
> blk->dev_opaque = opaque;
>
> @@ -979,8 +948,6 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp)
> bool tray_was_open, tray_is_open;
> Error *local_err = NULL;
>
> - assert(!blk->legacy_dev);
> -
> tray_was_open = blk_dev_is_tray_open(blk);
> blk->dev_ops->change_media_cb(blk->dev_opaque, load, &local_err);
> if (local_err) {
> @@ -1779,9 +1746,6 @@ void blk_eject(BlockBackend *blk, bool eject_flag)
> BlockDriverState *bs = blk_bs(blk);
> char *id;
>
> - /* blk_eject is only called by qdevified devices */
> - assert(!blk->legacy_dev);
> -
> if (bs) {
> bdrv_eject(bs, eject_flag);
> }
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 36eff94..9605caf 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
> * so we can blk_unref() unconditionally */
> blk_ref(blkdev->blk);
> }
> - blk_attach_dev_legacy(blkdev->blk, blkdev);
> + if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
> + return -1;
> + }
> blkdev->file_size = blk_getlength(blkdev->blk);
> if (blkdev->file_size < 0) {
> BlockDriverState *bs = blk_bs(blkdev->blk);
> @@ -951,7 +953,7 @@ static void blk_disconnect(struct XenDevice *xendev)
>
> if (blkdev->blk) {
> blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
> - blk_detach_dev(blkdev->blk, blkdev);
> + blk_detach_dev(blkdev->blk, DEVICE(blkdev));
> blk_unref(blkdev->blk);
> blkdev->blk = NULL;
> }
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index c96bcde..39507d6 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -110,9 +110,8 @@ void blk_iostatus_disable(BlockBackend *blk);
> void blk_iostatus_reset(BlockBackend *blk);
> void blk_iostatus_set_err(BlockBackend *blk, int error);
> int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev);
> -void blk_detach_dev(BlockBackend *blk, void *dev);
> -void *blk_get_attached_dev(BlockBackend *blk);
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
> +DeviceState *blk_get_attached_dev(BlockBackend *blk);
> char *blk_get_attached_dev_id(BlockBackend *blk);
> BlockBackend *blk_by_dev(void *dev);
> BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
2019-01-22 14:19 ` Thomas Huth
@ 2019-01-22 14:46 ` Kevin Wolf
-1 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-01-22 14:46 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-block, Max Reitz, Stefano Stabellini, Anthony Perard,
xen-devel, qemu-devel, paul.durrant
Am 22.01.2019 um 15:19 hat Thomas Huth geschrieben:
> On 2018-12-18 17:11, Thomas Huth wrote:
> > The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
> > It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
> > is derived from XenDevice which in turn is derived from DeviceState
> > since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
> > Thus the code can also simply use blk_attach_dev() with a pointer
> > to the DeviceState instead.
> > So we can finally remove all code related to the "legacy_dev" flag, too,
> > and turn the related "void *" in block-backend.c into "DeviceState *"
> > to fix some of the remaining TODOs there.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> > Note: I haven't tested the Xen code since I don't have a working Xen
> > installation at hand. I'd appreciate if someone could check it...
>
> Ping?
This needs a rebase. xen_disk.c doesn't even exist any more and
blk_attach_dev_legacy() is really dead code now.
The work on the Xen block device is the reason why I didn't merge this
patch yet. I thought I had sent a reply to that effect, but it seems I
hadn't. Sorry for that!
Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
@ 2019-01-22 14:46 ` Kevin Wolf
0 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-01-22 14:46 UTC (permalink / raw)
To: Thomas Huth
Cc: Stefano Stabellini, qemu-block, qemu-devel, Max Reitz,
paul.durrant, Anthony Perard, xen-devel
Am 22.01.2019 um 15:19 hat Thomas Huth geschrieben:
> On 2018-12-18 17:11, Thomas Huth wrote:
> > The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
> > It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
> > is derived from XenDevice which in turn is derived from DeviceState
> > since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
> > Thus the code can also simply use blk_attach_dev() with a pointer
> > to the DeviceState instead.
> > So we can finally remove all code related to the "legacy_dev" flag, too,
> > and turn the related "void *" in block-backend.c into "DeviceState *"
> > to fix some of the remaining TODOs there.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> > Note: I haven't tested the Xen code since I don't have a working Xen
> > installation at hand. I'd appreciate if someone could check it...
>
> Ping?
This needs a rebase. xen_disk.c doesn't even exist any more and
blk_attach_dev_legacy() is really dead code now.
The work on the Xen block device is the reason why I didn't merge this
patch yet. I thought I had sent a reply to that effect, but it seems I
hadn't. Sorry for that!
Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
2019-01-22 14:46 ` Kevin Wolf
@ 2019-01-22 14:57 ` Thomas Huth
-1 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2019-01-22 14:57 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-block, Max Reitz, Stefano Stabellini, Anthony Perard,
xen-devel, qemu-devel, paul.durrant
On 2019-01-22 15:46, Kevin Wolf wrote:
> Am 22.01.2019 um 15:19 hat Thomas Huth geschrieben:
>> On 2018-12-18 17:11, Thomas Huth wrote:
>>> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
>>> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
>>> is derived from XenDevice which in turn is derived from DeviceState
>>> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
>>> Thus the code can also simply use blk_attach_dev() with a pointer
>>> to the DeviceState instead.
>>> So we can finally remove all code related to the "legacy_dev" flag, too,
>>> and turn the related "void *" in block-backend.c into "DeviceState *"
>>> to fix some of the remaining TODOs there.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> Note: I haven't tested the Xen code since I don't have a working Xen
>>> installation at hand. I'd appreciate if someone could check it...
>>
>> Ping?
>
> This needs a rebase. xen_disk.c doesn't even exist any more and
> blk_attach_dev_legacy() is really dead code now.
Ah, great, that makes it even easier! I'll send a v2 to remove the
remainders...
Thomas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code
@ 2019-01-22 14:57 ` Thomas Huth
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2019-01-22 14:57 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefano Stabellini, qemu-block, qemu-devel, Max Reitz,
paul.durrant, Anthony Perard, xen-devel
On 2019-01-22 15:46, Kevin Wolf wrote:
> Am 22.01.2019 um 15:19 hat Thomas Huth geschrieben:
>> On 2018-12-18 17:11, Thomas Huth wrote:
>>> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
>>> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
>>> is derived from XenDevice which in turn is derived from DeviceState
>>> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
>>> Thus the code can also simply use blk_attach_dev() with a pointer
>>> to the DeviceState instead.
>>> So we can finally remove all code related to the "legacy_dev" flag, too,
>>> and turn the related "void *" in block-backend.c into "DeviceState *"
>>> to fix some of the remaining TODOs there.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> Note: I haven't tested the Xen code since I don't have a working Xen
>>> installation at hand. I'd appreciate if someone could check it...
>>
>> Ping?
>
> This needs a rebase. xen_disk.c doesn't even exist any more and
> blk_attach_dev_legacy() is really dead code now.
Ah, great, that makes it even easier! I'll send a v2 to remove the
remainders...
Thomas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-01-22 19:44 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-18 16:11 [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code Thomas Huth
2018-12-18 16:11 ` Thomas Huth
2018-12-18 18:33 ` [Qemu-devel] " Markus Armbruster
2018-12-18 18:33 ` Markus Armbruster
2018-12-19 12:00 ` Thomas Huth
2018-12-19 12:00 ` Thomas Huth
2019-01-22 14:19 ` Thomas Huth
2019-01-22 14:19 ` Thomas Huth
2019-01-22 14:46 ` Kevin Wolf
2019-01-22 14:46 ` Kevin Wolf
2019-01-22 14:57 ` Thomas Huth
2019-01-22 14:57 ` Thomas Huth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.